Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 61 additions & 1 deletion src/hooks/task.hook.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -32,6 +32,48 @@ import type { Hook, HookContext } from '@objectstack/spec/data';
* holding exactly what the caller supplied, so a value derived here
* replaces a caller-supplied one instead of being dropped with it.
*
* ── The shared-payload path, and why this hook REFUSES it ────────────────
* A predicate (`multi: true`) write carries ONE payload for all N matched rows
* — `driver.updateMany` takes a single SET clause — and ADR-0058 Addendum II
* governs what a hook may do with it. D1: a `before*` event dispatches once per
* matched row. D3: every per-row context carries THAT one payload, so "a
* rewrite takes effect on the WHOLE batch, whichever row's dispatch made it",
* and therefore "a rewrite CONDITIONED on the row (`ctx.previous`,
* `ctx.input.id`) is outside this contract". D3 names the sanctioned route in
* as many words: per-row `previous` is supplied "so a guard can REFUSE the
* write, not so a rewrite can be aimed at one row".
*
* `completed_at` is precisely such a rewrite: it is read off THIS row's
* pre-image. Measured against a booted engine — a batch of one `open` row and
* one already-`done` row, both written `status: 'done'` — the open row's
* dispatch stamps `completed_at = now` and the done row's completion instant,
* days old, is silently overwritten. Nothing errors; the history just moves.
*
* So on this path the hook throws. The guard is decided from the ROW ALONE —
* its own pre-image plus the payload — never from what an earlier dispatch left
* in the shared payload, because dispatch order is not the caller's to control
* and an accumulator would only catch the orders in which the done row happens
* to come second.
*
* Three boundaries this guard deliberately holds:
*
* - It turns on `status` being IN THE PAYLOAD. A predicate write that does not
* write `status` computes no stamp, so there is nothing to leak — an
* administrative bulk backfill over done rows, and the seed's `mode:
* 'update'` backdating pass, are untouched.
* - Only the stamping direction is guarded. A batch moving rows OUT of done
* writes `completed_at = null`, and null is the correct value for every row
* being moved out of done, including one that was never completed. That
* rewrite is genuinely row-invariant, so it is allowed.
* - A batch in which EVERY row is already done is refused too, even though
* nothing would leak. The hook cannot see the batch — `dispatch.index` is a
* position, not a total — and a rule stated on the row is one a caller can
* predict and a test can pin. Re-completing a done task in bulk is a caller
* mistake either way, and the answer is now loud instead of silent.
*
* The single-record path (`dispatch.mode === 'record'`) has a payload of its
* own, so the row-conditional stamp is sound there and is unchanged.
*
* ── 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
Expand DownExpand Up@@ -76,6 +118,22 @@ const stampTaskLifecycle = (ctx: HookContext): void => {
const wasDone = previous.status === 'done';
const isDone = nextStatus === 'done';

// ── The shared-payload guard — refuse, never aim a rewrite at one row ──
// `ctx.dispatch` is the engine's own dispatch marker: `'record'` for a
// single-record write, `'per-row'` for one dispatch of a predicate write.
// Conditioned on `status` being in the payload because that is the only
// shape that computes a stamp at all — see the module header.
if (ctx.dispatch?.mode === 'per-row' && 'status' in input && wasDone && isDone) {
throw Object.assign(
new Error(
`Task ${String(input.id ?? previous.id ?? '')} is already done. A bulk status write carries one `
+ 'payload for every matched row, so completing this batch would overwrite that task\'s original '
+ 'completion timestamp. Leave the done rows out of the selection, or write them one at a time.',
),
{ code: 'DULY_TASK_BULK_ALREADY_DONE', status: 409 },
);
}

if (!wasDone && isDone) {
input.completed_at = now;
} else if (wasDone && !isDone) {
Expand DownExpand Up@@ -125,7 +183,9 @@ export const TaskLifecycleHook: Hook = {
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.',
+ 'never on an administrative or bulk write, which would reset the stagnation signal. '
+ 'A predicate write that would re-stamp an already-done row is refused outright '
+ '(ADR-0058 Addendum II D3: one payload for the whole batch, so a guard throws).',
// 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
Expand Down
31 changes: 21 additions & 10 deletions src/views/task.view.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -27,17 +27,28 @@ const columns = [
* 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 ────────────────────────
* ── `visible` here is the OUTER of two layers ─────────────────────────────
* 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.
* that pass, which is what keeps an already-`done` row out of the batch. That
* matters 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 would write that timestamp to the whole batch —
* silently re-dating a task completed days ago.
*
* A view predicate is a client-side hide, though, and the write it guards is
* server-side: an import, a backfill, the dispatcher or an MCP caller
* reassembles the same batch without ever reading this file. So the authority
* lives at the write. `task.hook.ts` REFUSES a predicate write that would
* re-stamp an already-done row (`DULY_TASK_BULK_ALREADY_DONE`, 409) — the one
* route ADR-0058 Addendum II D3 sanctions for a row-conditional decision on a
* batch-scoped payload.
*
* This predicate is kept because it is still the right UX: it stops the
* console from assembling a batch the server would refuse, so a user gets an
* unavailable action rather than an error they did not cause.
* `test/task-hook.test.ts` pins the refusal; `test/task-actions.test.ts` pins
* both layers.
*
* 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
Expand Down
36 changes: 26 additions & 10 deletions test/task-actions.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -531,26 +531,42 @@ describe('bulk', () => {
}
});

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.
it('an already-done row in the batch is refused by the SERVER, not merely hidden', async () => {
// The layer that actually decides. 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 would be
// written to every row in the batch — including one completed days ago.
// ADR-0058 Addendum II D3 hands a `before*` hook exactly one way out of
// that, and `task.hook.ts` takes it: refuse the write.
//
// This is the assertion that makes the `visible` predicate below a
// convenience rather than the only thing between a caller and moved
// history — an import, a backfill, the dispatcher or an MCP caller never
// goes near a view predicate.
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] } } });
const { code, status } = await refusal(
data.update('duly_task', { ...COMPLETE_PATCH }, { multi: true, where: { id: { $in: [open, alreadyDone] } } }),
);
expect(code).toBe('DULY_TASK_BULK_ALREADY_DONE');
expect(status).toBe(409);

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);
'the original completion instant must survive a batch that tried to re-stamp it',
).toBe(original);
expect((await read(open)).status, 'and the refusal writes nothing at all').toBe('open');
});

// So the declaration has to exclude it, and does.
it('and the visible predicate still keeps such a batch from being assembled', async () => {
// The outer layer, kept. `visible` is evaluated once per selected record
// and the run covers only the passing rows, so the console cannot build
// the batch the server now refuses. Defence in depth: the user gets a
// greyed-out row instead of an error they did not cause.
const complete = allBulkDefs().find((d) => d.name === BULK_COMPLETE);
const source = String(complete.visible?.source ?? '');
expect(source).not.toContain('"done"');
Expand Down
162 changes: 162 additions & 0 deletions test/task-hook.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -323,3 +323,165 @@ describe('last_update_at — the stagnation signal', () => {
expect(edited.last_update_at as string < forged).toBe(true);
});
});

// ── The shared-payload path: a predicate (bulk) write ──────────────────────
//
// A `multi: true` update carries ONE payload for all N matched rows —
// `driver.updateMany` takes a single SET clause — and ADR-0058 Addendum II D3
// says what that means for a hook: every per-row `beforeUpdate` context
// carries THAT payload, so "a rewrite takes effect on the WHOLE batch,
// whichever row's dispatch made it". D3 names the consequence outright: a
// rewrite CONDITIONED on the row is *expressible and wrong*, and the sanctioned
// route for row-specific work in a `before*` hook is to THROW.
//
// `completed_at` is exactly such a rewrite — it is stamped on the TRANSITION,
// read off this row's own pre-image. So on this path the hook refuses instead
// of stamping. These assertions run against the real engine, and the dispatch
// mode they turn on is the engine's own (`ctx.dispatch.mode`), measured here
// rather than inferred.
describe('completed_at on a predicate write — one payload, N rows', () => {
/** Assert a refusal by its ENVELOPE (ADR-0112), never by the bare fact that it threw. */
const refusal = async (promise: Promise<unknown>) => {
try {
await promise;
} catch (error: any) {
return { code: error?.code, status: error?.status, message: String(error?.message ?? '') };
}
throw new Error('expected the predicate write to be refused, but it resolved');
};

const complete = async (id: string) => data.update('duly_task', { id, status: 'done' });

it('refuses a done batch that contains an already-done row, and does not move its clock', async () => {
// THE assertion this guard exists for. Without it the open row's dispatch
// stamps `completed_at = now` into the shared payload and the already-done
// row — completed days ago — is silently re-dated.
const open = (await newTask({ subject: 'still open' })).id;
const alreadyDone = (await newTask({ subject: 'done last week' })).id;
await complete(alreadyDone);
const original = (await read(alreadyDone)).completed_at;
expect(original).toBeTruthy();

await tick();
const { code, status, message } = await refusal(
data.update('duly_task', { status: 'done' }, { multi: true, where: { id: { $in: [open, alreadyDone] } } }),
);

expect(code).toBe('DULY_TASK_BULK_ALREADY_DONE');
expect(status).toBe(409);
expect(message, 'the refusal must name the row a caller has to remove').toContain(alreadyDone);

expect(
(await read(alreadyDone)).completed_at,
'the original completion instant must survive the refused batch',
).toBe(original);
});

it('writes nothing at all — the refusal is not a partial batch', async () => {
const open = (await newTask({ subject: 'untouched by a refused batch' })).id;
const alreadyDone = (await newTask({ subject: 'already done' })).id;
await complete(alreadyDone);

await refusal(
data.update('duly_task', { status: 'done' }, { multi: true, where: { id: { $in: [open, alreadyDone] } } }),
);

expect((await read(open)).status, 'the transitioning row must not commit either').toBe('open');
expect((await read(open)).completed_at ?? null).toBeNull();
});

it('refuses whichever dispatch order the batch arrives in', async () => {
// The guard is decided from the ROW alone — its own pre-image and the
// payload — never from what an earlier dispatch happened to leave behind.
// An accumulator would only catch the order in which the done row is
// dispatched second.
for (const doneFirst of [true, false]) {
const openRow = (await newTask({ subject: `order open ${doneFirst}` })).id;
const doneRow = (await newTask({ subject: `order done ${doneFirst}` })).id;
await complete(doneRow);
const original = (await read(doneRow)).completed_at;
await tick();

const ids = doneFirst ? [doneRow, openRow] : [openRow, doneRow];
const { code } = await refusal(
data.update('duly_task', { status: 'done' }, { multi: true, where: { id: { $in: ids } } }),
);

expect(code, `done-first=${doneFirst} must refuse`).toBe('DULY_TASK_BULK_ALREADY_DONE');
expect((await read(doneRow)).completed_at).toBe(original);
}
});

it('still completes a homogeneous batch — every row stamped, in one write', async () => {
// The negative control. The guard must refuse the mixed batch WITHOUT
// taking bulk complete away: a week of ticks in one gesture is the feature.
const ids: string[] = [];
for (let i = 0; i < 5; i += 1) ids.push((await newTask({ subject: `homogeneous ${i}` })).id);

const affected = await data.update('duly_task', { status: 'done' }, {
multi: true,
where: { id: { $in: ids } },
});
expect(affected).toBe(5);

for (const id of ids) {
const row = await read(id);
expect(row.status).toBe('done');
expect(row.completed_at, `${id} must be stamped like any other write`).toBeTruthy();
}
});

it('leaves an administrative predicate write alone — the guard turns on the STATUS in the payload', async () => {
// The over-refusal control, and the one that keeps the seed's second pass
// working: a bulk write that does not carry `status` computes no stamp, so
// there is nothing to leak and nothing to refuse — even over a done row.
const alreadyDone = (await newTask({ subject: 'admin backfill target' })).id;
await complete(alreadyDone);
const original = (await read(alreadyDone)).completed_at;
await tick();

await data.update('duly_task', { business_unit: 'bu_north' }, {
multi: true,
where: { id: { $in: [alreadyDone] } },
});

const row = await read(alreadyDone);
expect(row.business_unit, 'a backfill must still land').toBe('bu_north');
expect(row.completed_at, 'and must not disturb the completion instant').toBe(original);
});

it('does not fire on the single-record path — a re-save of a done task is still a no-op', async () => {
// `mode: 'record'` has a payload of its own, so the row-conditional stamp
// is sound there. Re-sending `status: 'done'` on a done task must keep
// behaving as it always has: accepted, and NOT re-stamped.
const task = await newTask();
const done = await data.update('duly_task', { id: task.id, status: 'done' });
const first = done.completed_at;

await tick();
const resaved = await data.update('duly_task', { id: task.id, status: 'done', note: 'after the fact' });

expect(resaved.completed_at, 'the by-id path is unchanged by the bulk guard').toBe(first);
});

it('a predicate write clearing done is NOT refused — that direction is row-invariant', async () => {
// Reopening a batch sets `completed_at = null`, and null is the correct
// value for EVERY row being moved out of done, including one that was
// never completed. Nothing row-specific leaks, so nothing is refused.
const wasDone = (await newTask({ subject: 'reopen me' })).id;
const neverDone = (await newTask({ subject: 'never completed' })).id;
await complete(wasDone);

const affected = await data.update('duly_task', { status: 'in_progress' }, {
multi: true,
where: { id: { $in: [wasDone, neverDone] } },
});
expect(affected).toBe(2);

for (const id of [wasDone, neverDone]) {
const row = await read(id);
expect(row.status).toBe('in_progress');
expect(row.completed_at ?? null, `${id} must come out of done with no completion`).toBeNull();
}
});
});
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all \u003cpre\u003e\u003ccode\u003e blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks"); } } catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); } })(); (function(){ try { var __m = "github.com"; var __re = new RegExp('^' + "github\\.com" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 61 additions & 1 deletion src/hooks/task.hook.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -32,6 +32,48 @@ import type { Hook, HookContext } from '@objectstack/spec/data';
* holding exactly what the caller supplied, so a value derived here
* replaces a caller-supplied one instead of being dropped with it.
*
* ── The shared-payload path, and why this hook REFUSES it ────────────────
* A predicate (`multi: true`) write carries ONE payload for all N matched rows
* — `driver.updateMany` takes a single SET clause — and ADR-0058 Addendum II
* governs what a hook may do with it. D1: a `before*` event dispatches once per
* matched row. D3: every per-row context carries THAT one payload, so "a
* rewrite takes effect on the WHOLE batch, whichever row's dispatch made it",
* and therefore "a rewrite CONDITIONED on the row (`ctx.previous`,
* `ctx.input.id`) is outside this contract". D3 names the sanctioned route in
* as many words: per-row `previous` is supplied "so a guard can REFUSE the
* write, not so a rewrite can be aimed at one row".
*
* `completed_at` is precisely such a rewrite: it is read off THIS row's
* pre-image. Measured against a booted engine — a batch of one `open` row and
* one already-`done` row, both written `status: 'done'` — the open row's
* dispatch stamps `completed_at = now` and the done row's completion instant,
* days old, is silently overwritten. Nothing errors; the history just moves.
*
* So on this path the hook throws. The guard is decided from the ROW ALONE —
* its own pre-image plus the payload — never from what an earlier dispatch left
* in the shared payload, because dispatch order is not the caller's to control
* and an accumulator would only catch the orders in which the done row happens
* to come second.
*
* Three boundaries this guard deliberately holds:
*
* - It turns on `status` being IN THE PAYLOAD. A predicate write that does not
* write `status` computes no stamp, so there is nothing to leak — an
* administrative bulk backfill over done rows, and the seed's `mode:
* 'update'` backdating pass, are untouched.
* - Only the stamping direction is guarded. A batch moving rows OUT of done
* writes `completed_at = null`, and null is the correct value for every row
* being moved out of done, including one that was never completed. That
* rewrite is genuinely row-invariant, so it is allowed.
* - A batch in which EVERY row is already done is refused too, even though
* nothing would leak. The hook cannot see the batch — `dispatch.index` is a
* position, not a total — and a rule stated on the row is one a caller can
* predict and a test can pin. Re-completing a done task in bulk is a caller
* mistake either way, and the answer is now loud instead of silent.
*
* The single-record path (`dispatch.mode === 'record'`) has a payload of its
* own, so the row-conditional stamp is sound there and is unchanged.
*
* ── 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
Expand DownExpand Up@@ -76,6 +118,22 @@ const stampTaskLifecycle = (ctx: HookContext): void => {
const wasDone = previous.status === 'done';
const isDone = nextStatus === 'done';

// ── The shared-payload guard — refuse, never aim a rewrite at one row ──
// `ctx.dispatch` is the engine's own dispatch marker: `'record'` for a
// single-record write, `'per-row'` for one dispatch of a predicate write.
// Conditioned on `status` being in the payload because that is the only
// shape that computes a stamp at all — see the module header.
if (ctx.dispatch?.mode === 'per-row' && 'status' in input && wasDone && isDone) {
throw Object.assign(
new Error(
`Task ${String(input.id ?? previous.id ?? '')} is already done. A bulk status write carries one `
+ 'payload for every matched row, so completing this batch would overwrite that task\'s original '
+ 'completion timestamp. Leave the done rows out of the selection, or write them one at a time.',
),
{ code: 'DULY_TASK_BULK_ALREADY_DONE', status: 409 },
);
}

if (!wasDone && isDone) {
input.completed_at = now;
} else if (wasDone && !isDone) {
Expand DownExpand Up@@ -125,7 +183,9 @@ export const TaskLifecycleHook: Hook = {
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.',
+ 'never on an administrative or bulk write, which would reset the stagnation signal. '
+ 'A predicate write that would re-stamp an already-done row is refused outright '
+ '(ADR-0058 Addendum II D3: one payload for the whole batch, so a guard throws).',
// 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
Expand Down
31 changes: 21 additions & 10 deletions src/views/task.view.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -27,17 +27,28 @@ const columns = [
* 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 ────────────────────────
* ── `visible` here is the OUTER of two layers ─────────────────────────────
* 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.
* that pass, which is what keeps an already-`done` row out of the batch. That
* matters 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 would write that timestamp to the whole batch —
* silently re-dating a task completed days ago.
*
* A view predicate is a client-side hide, though, and the write it guards is
* server-side: an import, a backfill, the dispatcher or an MCP caller
* reassembles the same batch without ever reading this file. So the authority
* lives at the write. `task.hook.ts` REFUSES a predicate write that would
* re-stamp an already-done row (`DULY_TASK_BULK_ALREADY_DONE`, 409) — the one
* route ADR-0058 Addendum II D3 sanctions for a row-conditional decision on a
* batch-scoped payload.
*
* This predicate is kept because it is still the right UX: it stops the
* console from assembling a batch the server would refuse, so a user gets an
* unavailable action rather than an error they did not cause.
* `test/task-hook.test.ts` pins the refusal; `test/task-actions.test.ts` pins
* both layers.
*
* 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
Expand Down
36 changes: 26 additions & 10 deletions test/task-actions.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -531,26 +531,42 @@ describe('bulk', () => {
}
});

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.
it('an already-done row in the batch is refused by the SERVER, not merely hidden', async () => {
// The layer that actually decides. 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 would be
// written to every row in the batch — including one completed days ago.
// ADR-0058 Addendum II D3 hands a `before*` hook exactly one way out of
// that, and `task.hook.ts` takes it: refuse the write.
//
// This is the assertion that makes the `visible` predicate below a
// convenience rather than the only thing between a caller and moved
// history — an import, a backfill, the dispatcher or an MCP caller never
// goes near a view predicate.
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] } } });
const { code, status } = await refusal(
data.update('duly_task', { ...COMPLETE_PATCH }, { multi: true, where: { id: { $in: [open, alreadyDone] } } }),
);
expect(code).toBe('DULY_TASK_BULK_ALREADY_DONE');
expect(status).toBe(409);

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);
'the original completion instant must survive a batch that tried to re-stamp it',
).toBe(original);
expect((await read(open)).status, 'and the refusal writes nothing at all').toBe('open');
});

// So the declaration has to exclude it, and does.
it('and the visible predicate still keeps such a batch from being assembled', async () => {
// The outer layer, kept. `visible` is evaluated once per selected record
// and the run covers only the passing rows, so the console cannot build
// the batch the server now refuses. Defence in depth: the user gets a
// greyed-out row instead of an error they did not cause.
const complete = allBulkDefs().find((d) => d.name === BULK_COMPLETE);
const source = String(complete.visible?.source ?? '');
expect(source).not.toContain('"done"');
Expand Down
162 changes: 162 additions & 0 deletions test/task-hook.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -323,3 +323,165 @@ describe('last_update_at — the stagnation signal', () => {
expect(edited.last_update_at as string < forged).toBe(true);
});
});

// ── The shared-payload path: a predicate (bulk) write ──────────────────────
//
// A `multi: true` update carries ONE payload for all N matched rows —
// `driver.updateMany` takes a single SET clause — and ADR-0058 Addendum II D3
// says what that means for a hook: every per-row `beforeUpdate` context
// carries THAT payload, so "a rewrite takes effect on the WHOLE batch,
// whichever row's dispatch made it". D3 names the consequence outright: a
// rewrite CONDITIONED on the row is *expressible and wrong*, and the sanctioned
// route for row-specific work in a `before*` hook is to THROW.
//
// `completed_at` is exactly such a rewrite — it is stamped on the TRANSITION,
// read off this row's own pre-image. So on this path the hook refuses instead
// of stamping. These assertions run against the real engine, and the dispatch
// mode they turn on is the engine's own (`ctx.dispatch.mode`), measured here
// rather than inferred.
describe('completed_at on a predicate write — one payload, N rows', () => {
/** Assert a refusal by its ENVELOPE (ADR-0112), never by the bare fact that it threw. */
const refusal = async (promise: Promise<unknown>) => {
try {
await promise;
} catch (error: any) {
return { code: error?.code, status: error?.status, message: String(error?.message ?? '') };
}
throw new Error('expected the predicate write to be refused, but it resolved');
};

const complete = async (id: string) => data.update('duly_task', { id, status: 'done' });

it('refuses a done batch that contains an already-done row, and does not move its clock', async () => {
// THE assertion this guard exists for. Without it the open row's dispatch
// stamps `completed_at = now` into the shared payload and the already-done
// row — completed days ago — is silently re-dated.
const open = (await newTask({ subject: 'still open' })).id;
const alreadyDone = (await newTask({ subject: 'done last week' })).id;
await complete(alreadyDone);
const original = (await read(alreadyDone)).completed_at;
expect(original).toBeTruthy();

await tick();
const { code, status, message } = await refusal(
data.update('duly_task', { status: 'done' }, { multi: true, where: { id: { $in: [open, alreadyDone] } } }),
);

expect(code).toBe('DULY_TASK_BULK_ALREADY_DONE');
expect(status).toBe(409);
expect(message, 'the refusal must name the row a caller has to remove').toContain(alreadyDone);

expect(
(await read(alreadyDone)).completed_at,
'the original completion instant must survive the refused batch',
).toBe(original);
});

it('writes nothing at all — the refusal is not a partial batch', async () => {
const open = (await newTask({ subject: 'untouched by a refused batch' })).id;
const alreadyDone = (await newTask({ subject: 'already done' })).id;
await complete(alreadyDone);

await refusal(
data.update('duly_task', { status: 'done' }, { multi: true, where: { id: { $in: [open, alreadyDone] } } }),
);

expect((await read(open)).status, 'the transitioning row must not commit either').toBe('open');
expect((await read(open)).completed_at ?? null).toBeNull();
});

it('refuses whichever dispatch order the batch arrives in', async () => {
// The guard is decided from the ROW alone — its own pre-image and the
// payload — never from what an earlier dispatch happened to leave behind.
// An accumulator would only catch the order in which the done row is
// dispatched second.
for (const doneFirst of [true, false]) {
const openRow = (await newTask({ subject: `order open ${doneFirst}` })).id;
const doneRow = (await newTask({ subject: `order done ${doneFirst}` })).id;
await complete(doneRow);
const original = (await read(doneRow)).completed_at;
await tick();

const ids = doneFirst ? [doneRow, openRow] : [openRow, doneRow];
const { code } = await refusal(
data.update('duly_task', { status: 'done' }, { multi: true, where: { id: { $in: ids } } }),
);

expect(code, `done-first=${doneFirst} must refuse`).toBe('DULY_TASK_BULK_ALREADY_DONE');
expect((await read(doneRow)).completed_at).toBe(original);
}
});

it('still completes a homogeneous batch — every row stamped, in one write', async () => {
// The negative control. The guard must refuse the mixed batch WITHOUT
// taking bulk complete away: a week of ticks in one gesture is the feature.
const ids: string[] = [];
for (let i = 0; i < 5; i += 1) ids.push((await newTask({ subject: `homogeneous ${i}` })).id);

const affected = await data.update('duly_task', { status: 'done' }, {
multi: true,
where: { id: { $in: ids } },
});
expect(affected).toBe(5);

for (const id of ids) {
const row = await read(id);
expect(row.status).toBe('done');
expect(row.completed_at, `${id} must be stamped like any other write`).toBeTruthy();
}
});

it('leaves an administrative predicate write alone — the guard turns on the STATUS in the payload', async () => {
// The over-refusal control, and the one that keeps the seed's second pass
// working: a bulk write that does not carry `status` computes no stamp, so
// there is nothing to leak and nothing to refuse — even over a done row.
const alreadyDone = (await newTask({ subject: 'admin backfill target' })).id;
await complete(alreadyDone);
const original = (await read(alreadyDone)).completed_at;
await tick();

await data.update('duly_task', { business_unit: 'bu_north' }, {
multi: true,
where: { id: { $in: [alreadyDone] } },
});

const row = await read(alreadyDone);
expect(row.business_unit, 'a backfill must still land').toBe('bu_north');
expect(row.completed_at, 'and must not disturb the completion instant').toBe(original);
});

it('does not fire on the single-record path — a re-save of a done task is still a no-op', async () => {
// `mode: 'record'` has a payload of its own, so the row-conditional stamp
// is sound there. Re-sending `status: 'done'` on a done task must keep
// behaving as it always has: accepted, and NOT re-stamped.
const task = await newTask();
const done = await data.update('duly_task', { id: task.id, status: 'done' });
const first = done.completed_at;

await tick();
const resaved = await data.update('duly_task', { id: task.id, status: 'done', note: 'after the fact' });

expect(resaved.completed_at, 'the by-id path is unchanged by the bulk guard').toBe(first);
});

it('a predicate write clearing done is NOT refused — that direction is row-invariant', async () => {
// Reopening a batch sets `completed_at = null`, and null is the correct
// value for EVERY row being moved out of done, including one that was
// never completed. Nothing row-specific leaks, so nothing is refused.
const wasDone = (await newTask({ subject: 'reopen me' })).id;
const neverDone = (await newTask({ subject: 'never completed' })).id;
await complete(wasDone);

const affected = await data.update('duly_task', { status: 'in_progress' }, {
multi: true,
where: { id: { $in: [wasDone, neverDone] } },
});
expect(affected).toBe(2);

for (const id of [wasDone, neverDone]) {
const row = await read(id);
expect(row.status).toBe('in_progress');
expect(row.completed_at ?? null, `${id} must come out of done with no completion`).toBeNull();
}
});
});
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 61 additions & 1 deletion src/hooks/task.hook.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -32,6 +32,48 @@ import type { Hook, HookContext } from '@objectstack/spec/data';
* holding exactly what the caller supplied, so a value derived here
* replaces a caller-supplied one instead of being dropped with it.
*
* ── The shared-payload path, and why this hook REFUSES it ────────────────
* A predicate (`multi: true`) write carries ONE payload for all N matched rows
* — `driver.updateMany` takes a single SET clause — and ADR-0058 Addendum II
* governs what a hook may do with it. D1: a `before*` event dispatches once per
* matched row. D3: every per-row context carries THAT one payload, so "a
* rewrite takes effect on the WHOLE batch, whichever row's dispatch made it",
* and therefore "a rewrite CONDITIONED on the row (`ctx.previous`,
* `ctx.input.id`) is outside this contract". D3 names the sanctioned route in
* as many words: per-row `previous` is supplied "so a guard can REFUSE the
* write, not so a rewrite can be aimed at one row".
*
* `completed_at` is precisely such a rewrite: it is read off THIS row's
* pre-image. Measured against a booted engine — a batch of one `open` row and
* one already-`done` row, both written `status: 'done'` — the open row's
* dispatch stamps `completed_at = now` and the done row's completion instant,
* days old, is silently overwritten. Nothing errors; the history just moves.
*
* So on this path the hook throws. The guard is decided from the ROW ALONE —
* its own pre-image plus the payload — never from what an earlier dispatch left
* in the shared payload, because dispatch order is not the caller's to control
* and an accumulator would only catch the orders in which the done row happens
* to come second.
*
* Three boundaries this guard deliberately holds:
*
* - It turns on `status` being IN THE PAYLOAD. A predicate write that does not
* write `status` computes no stamp, so there is nothing to leak — an
* administrative bulk backfill over done rows, and the seed's `mode:
* 'update'` backdating pass, are untouched.
* - Only the stamping direction is guarded. A batch moving rows OUT of done
* writes `completed_at = null`, and null is the correct value for every row
* being moved out of done, including one that was never completed. That
* rewrite is genuinely row-invariant, so it is allowed.
* - A batch in which EVERY row is already done is refused too, even though
* nothing would leak. The hook cannot see the batch — `dispatch.index` is a
* position, not a total — and a rule stated on the row is one a caller can
* predict and a test can pin. Re-completing a done task in bulk is a caller
* mistake either way, and the answer is now loud instead of silent.
*
* The single-record path (`dispatch.mode === 'record'`) has a payload of its
* own, so the row-conditional stamp is sound there and is unchanged.
*
* ── 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
Expand DownExpand Up@@ -76,6 +118,22 @@ const stampTaskLifecycle = (ctx: HookContext): void => {
const wasDone = previous.status === 'done';
const isDone = nextStatus === 'done';

// ── The shared-payload guard — refuse, never aim a rewrite at one row ──
// `ctx.dispatch` is the engine's own dispatch marker: `'record'` for a
// single-record write, `'per-row'` for one dispatch of a predicate write.
// Conditioned on `status` being in the payload because that is the only
// shape that computes a stamp at all — see the module header.
if (ctx.dispatch?.mode === 'per-row' && 'status' in input && wasDone && isDone) {
throw Object.assign(
new Error(
`Task ${String(input.id ?? previous.id ?? '')} is already done. A bulk status write carries one `
+ 'payload for every matched row, so completing this batch would overwrite that task\'s original '
+ 'completion timestamp. Leave the done rows out of the selection, or write them one at a time.',
),
{ code: 'DULY_TASK_BULK_ALREADY_DONE', status: 409 },
);
}

if (!wasDone && isDone) {
input.completed_at = now;
} else if (wasDone && !isDone) {
Expand DownExpand Up@@ -125,7 +183,9 @@ export const TaskLifecycleHook: Hook = {
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.',
+ 'never on an administrative or bulk write, which would reset the stagnation signal. '
+ 'A predicate write that would re-stamp an already-done row is refused outright '
+ '(ADR-0058 Addendum II D3: one payload for the whole batch, so a guard throws).',
// 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
Expand Down
31 changes: 21 additions & 10 deletions src/views/task.view.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -27,17 +27,28 @@ const columns = [
* 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 ────────────────────────
* ── `visible` here is the OUTER of two layers ─────────────────────────────
* 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.
* that pass, which is what keeps an already-`done` row out of the batch. That
* matters 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 would write that timestamp to the whole batch —
* silently re-dating a task completed days ago.
*
* A view predicate is a client-side hide, though, and the write it guards is
* server-side: an import, a backfill, the dispatcher or an MCP caller
* reassembles the same batch without ever reading this file. So the authority
* lives at the write. `task.hook.ts` REFUSES a predicate write that would
* re-stamp an already-done row (`DULY_TASK_BULK_ALREADY_DONE`, 409) — the one
* route ADR-0058 Addendum II D3 sanctions for a row-conditional decision on a
* batch-scoped payload.
*
* This predicate is kept because it is still the right UX: it stops the
* console from assembling a batch the server would refuse, so a user gets an
* unavailable action rather than an error they did not cause.
* `test/task-hook.test.ts` pins the refusal; `test/task-actions.test.ts` pins
* both layers.
*
* 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
Expand Down
36 changes: 26 additions & 10 deletions test/task-actions.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -531,26 +531,42 @@ describe('bulk', () => {
}
});

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.
it('an already-done row in the batch is refused by the SERVER, not merely hidden', async () => {
// The layer that actually decides. 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 would be
// written to every row in the batch — including one completed days ago.
// ADR-0058 Addendum II D3 hands a `before*` hook exactly one way out of
// that, and `task.hook.ts` takes it: refuse the write.
//
// This is the assertion that makes the `visible` predicate below a
// convenience rather than the only thing between a caller and moved
// history — an import, a backfill, the dispatcher or an MCP caller never
// goes near a view predicate.
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] } } });
const { code, status } = await refusal(
data.update('duly_task', { ...COMPLETE_PATCH }, { multi: true, where: { id: { $in: [open, alreadyDone] } } }),
);
expect(code).toBe('DULY_TASK_BULK_ALREADY_DONE');
expect(status).toBe(409);

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);
'the original completion instant must survive a batch that tried to re-stamp it',
).toBe(original);
expect((await read(open)).status, 'and the refusal writes nothing at all').toBe('open');
});

// So the declaration has to exclude it, and does.
it('and the visible predicate still keeps such a batch from being assembled', async () => {
// The outer layer, kept. `visible` is evaluated once per selected record
// and the run covers only the passing rows, so the console cannot build
// the batch the server now refuses. Defence in depth: the user gets a
// greyed-out row instead of an error they did not cause.
const complete = allBulkDefs().find((d) => d.name === BULK_COMPLETE);
const source = String(complete.visible?.source ?? '');
expect(source).not.toContain('"done"');
Expand Down
162 changes: 162 additions & 0 deletions test/task-hook.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -323,3 +323,165 @@ describe('last_update_at — the stagnation signal', () => {
expect(edited.last_update_at as string < forged).toBe(true);
});
});

// ── The shared-payload path: a predicate (bulk) write ──────────────────────
//
// A `multi: true` update carries ONE payload for all N matched rows —
// `driver.updateMany` takes a single SET clause — and ADR-0058 Addendum II D3
// says what that means for a hook: every per-row `beforeUpdate` context
// carries THAT payload, so "a rewrite takes effect on the WHOLE batch,
// whichever row's dispatch made it". D3 names the consequence outright: a
// rewrite CONDITIONED on the row is *expressible and wrong*, and the sanctioned
// route for row-specific work in a `before*` hook is to THROW.
//
// `completed_at` is exactly such a rewrite — it is stamped on the TRANSITION,
// read off this row's own pre-image. So on this path the hook refuses instead
// of stamping. These assertions run against the real engine, and the dispatch
// mode they turn on is the engine's own (`ctx.dispatch.mode`), measured here
// rather than inferred.
describe('completed_at on a predicate write — one payload, N rows', () => {
/** Assert a refusal by its ENVELOPE (ADR-0112), never by the bare fact that it threw. */
const refusal = async (promise: Promise<unknown>) => {
try {
await promise;
} catch (error: any) {
return { code: error?.code, status: error?.status, message: String(error?.message ?? '') };
}
throw new Error('expected the predicate write to be refused, but it resolved');
};

const complete = async (id: string) => data.update('duly_task', { id, status: 'done' });

it('refuses a done batch that contains an already-done row, and does not move its clock', async () => {
// THE assertion this guard exists for. Without it the open row's dispatch
// stamps `completed_at = now` into the shared payload and the already-done
// row — completed days ago — is silently re-dated.
const open = (await newTask({ subject: 'still open' })).id;
const alreadyDone = (await newTask({ subject: 'done last week' })).id;
await complete(alreadyDone);
const original = (await read(alreadyDone)).completed_at;
expect(original).toBeTruthy();

await tick();
const { code, status, message } = await refusal(
data.update('duly_task', { status: 'done' }, { multi: true, where: { id: { $in: [open, alreadyDone] } } }),
);

expect(code).toBe('DULY_TASK_BULK_ALREADY_DONE');
expect(status).toBe(409);
expect(message, 'the refusal must name the row a caller has to remove').toContain(alreadyDone);

expect(
(await read(alreadyDone)).completed_at,
'the original completion instant must survive the refused batch',
).toBe(original);
});

it('writes nothing at all — the refusal is not a partial batch', async () => {
const open = (await newTask({ subject: 'untouched by a refused batch' })).id;
const alreadyDone = (await newTask({ subject: 'already done' })).id;
await complete(alreadyDone);

await refusal(
data.update('duly_task', { status: 'done' }, { multi: true, where: { id: { $in: [open, alreadyDone] } } }),
);

expect((await read(open)).status, 'the transitioning row must not commit either').toBe('open');
expect((await read(open)).completed_at ?? null).toBeNull();
});

it('refuses whichever dispatch order the batch arrives in', async () => {
// The guard is decided from the ROW alone — its own pre-image and the
// payload — never from what an earlier dispatch happened to leave behind.
// An accumulator would only catch the order in which the done row is
// dispatched second.
for (const doneFirst of [true, false]) {
const openRow = (await newTask({ subject: `order open ${doneFirst}` })).id;
const doneRow = (await newTask({ subject: `order done ${doneFirst}` })).id;
await complete(doneRow);
const original = (await read(doneRow)).completed_at;
await tick();

const ids = doneFirst ? [doneRow, openRow] : [openRow, doneRow];
const { code } = await refusal(
data.update('duly_task', { status: 'done' }, { multi: true, where: { id: { $in: ids } } }),
);

expect(code, `done-first=${doneFirst} must refuse`).toBe('DULY_TASK_BULK_ALREADY_DONE');
expect((await read(doneRow)).completed_at).toBe(original);
}
});

it('still completes a homogeneous batch — every row stamped, in one write', async () => {
// The negative control. The guard must refuse the mixed batch WITHOUT
// taking bulk complete away: a week of ticks in one gesture is the feature.
const ids: string[] = [];
for (let i = 0; i < 5; i += 1) ids.push((await newTask({ subject: `homogeneous ${i}` })).id);

const affected = await data.update('duly_task', { status: 'done' }, {
multi: true,
where: { id: { $in: ids } },
});
expect(affected).toBe(5);

for (const id of ids) {
const row = await read(id);
expect(row.status).toBe('done');
expect(row.completed_at, `${id} must be stamped like any other write`).toBeTruthy();
}
});

it('leaves an administrative predicate write alone — the guard turns on the STATUS in the payload', async () => {
// The over-refusal control, and the one that keeps the seed's second pass
// working: a bulk write that does not carry `status` computes no stamp, so
// there is nothing to leak and nothing to refuse — even over a done row.
const alreadyDone = (await newTask({ subject: 'admin backfill target' })).id;
await complete(alreadyDone);
const original = (await read(alreadyDone)).completed_at;
await tick();

await data.update('duly_task', { business_unit: 'bu_north' }, {
multi: true,
where: { id: { $in: [alreadyDone] } },
});

const row = await read(alreadyDone);
expect(row.business_unit, 'a backfill must still land').toBe('bu_north');
expect(row.completed_at, 'and must not disturb the completion instant').toBe(original);
});

it('does not fire on the single-record path — a re-save of a done task is still a no-op', async () => {
// `mode: 'record'` has a payload of its own, so the row-conditional stamp
// is sound there. Re-sending `status: 'done'` on a done task must keep
// behaving as it always has: accepted, and NOT re-stamped.
const task = await newTask();
const done = await data.update('duly_task', { id: task.id, status: 'done' });
const first = done.completed_at;

await tick();
const resaved = await data.update('duly_task', { id: task.id, status: 'done', note: 'after the fact' });

expect(resaved.completed_at, 'the by-id path is unchanged by the bulk guard').toBe(first);
});

it('a predicate write clearing done is NOT refused — that direction is row-invariant', async () => {
// Reopening a batch sets `completed_at = null`, and null is the correct
// value for EVERY row being moved out of done, including one that was
// never completed. Nothing row-specific leaks, so nothing is refused.
const wasDone = (await newTask({ subject: 'reopen me' })).id;
const neverDone = (await newTask({ subject: 'never completed' })).id;
await complete(wasDone);

const affected = await data.update('duly_task', { status: 'in_progress' }, {
multi: true,
where: { id: { $in: [wasDone, neverDone] } },
});
expect(affected).toBe(2);

for (const id of [wasDone, neverDone]) {
const row = await read(id);
expect(row.status).toBe('in_progress');
expect(row.completed_at ?? null, `${id} must come out of done with no completion`).toBeNull();
}
});
});
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length \u003e 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 61 additions & 1 deletion src/hooks/task.hook.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -32,6 +32,48 @@ import type { Hook, HookContext } from '@objectstack/spec/data';
* holding exactly what the caller supplied, so a value derived here
* replaces a caller-supplied one instead of being dropped with it.
*
* ── The shared-payload path, and why this hook REFUSES it ────────────────
* A predicate (`multi: true`) write carries ONE payload for all N matched rows
* — `driver.updateMany` takes a single SET clause — and ADR-0058 Addendum II
* governs what a hook may do with it. D1: a `before*` event dispatches once per
* matched row. D3: every per-row context carries THAT one payload, so "a
* rewrite takes effect on the WHOLE batch, whichever row's dispatch made it",
* and therefore "a rewrite CONDITIONED on the row (`ctx.previous`,
* `ctx.input.id`) is outside this contract". D3 names the sanctioned route in
* as many words: per-row `previous` is supplied "so a guard can REFUSE the
* write, not so a rewrite can be aimed at one row".
*
* `completed_at` is precisely such a rewrite: it is read off THIS row's
* pre-image. Measured against a booted engine — a batch of one `open` row and
* one already-`done` row, both written `status: 'done'` — the open row's
* dispatch stamps `completed_at = now` and the done row's completion instant,
* days old, is silently overwritten. Nothing errors; the history just moves.
*
* So on this path the hook throws. The guard is decided from the ROW ALONE —
* its own pre-image plus the payload — never from what an earlier dispatch left
* in the shared payload, because dispatch order is not the caller's to control
* and an accumulator would only catch the orders in which the done row happens
* to come second.
*
* Three boundaries this guard deliberately holds:
*
* - It turns on `status` being IN THE PAYLOAD. A predicate write that does not
* write `status` computes no stamp, so there is nothing to leak — an
* administrative bulk backfill over done rows, and the seed's `mode:
* 'update'` backdating pass, are untouched.
* - Only the stamping direction is guarded. A batch moving rows OUT of done
* writes `completed_at = null`, and null is the correct value for every row
* being moved out of done, including one that was never completed. That
* rewrite is genuinely row-invariant, so it is allowed.
* - A batch in which EVERY row is already done is refused too, even though
* nothing would leak. The hook cannot see the batch — `dispatch.index` is a
* position, not a total — and a rule stated on the row is one a caller can
* predict and a test can pin. Re-completing a done task in bulk is a caller
* mistake either way, and the answer is now loud instead of silent.
*
* The single-record path (`dispatch.mode === 'record'`) has a payload of its
* own, so the row-conditional stamp is sound there and is unchanged.
*
* ── 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
Expand DownExpand Up@@ -76,6 +118,22 @@ const stampTaskLifecycle = (ctx: HookContext): void => {
const wasDone = previous.status === 'done';
const isDone = nextStatus === 'done';

// ── The shared-payload guard — refuse, never aim a rewrite at one row ──
// `ctx.dispatch` is the engine's own dispatch marker: `'record'` for a
// single-record write, `'per-row'` for one dispatch of a predicate write.
// Conditioned on `status` being in the payload because that is the only
// shape that computes a stamp at all — see the module header.
if (ctx.dispatch?.mode === 'per-row' && 'status' in input && wasDone && isDone) {
throw Object.assign(
new Error(
`Task ${String(input.id ?? previous.id ?? '')} is already done. A bulk status write carries one `
+ 'payload for every matched row, so completing this batch would overwrite that task\'s original '
+ 'completion timestamp. Leave the done rows out of the selection, or write them one at a time.',
),
{ code: 'DULY_TASK_BULK_ALREADY_DONE', status: 409 },
);
}

if (!wasDone && isDone) {
input.completed_at = now;
} else if (wasDone && !isDone) {
Expand DownExpand Up@@ -125,7 +183,9 @@ export const TaskLifecycleHook: Hook = {
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.',
+ 'never on an administrative or bulk write, which would reset the stagnation signal. '
+ 'A predicate write that would re-stamp an already-done row is refused outright '
+ '(ADR-0058 Addendum II D3: one payload for the whole batch, so a guard throws).',
// 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
Expand Down
31 changes: 21 additions & 10 deletions src/views/task.view.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -27,17 +27,28 @@ const columns = [
* 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 ────────────────────────
* ── `visible` here is the OUTER of two layers ─────────────────────────────
* 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.
* that pass, which is what keeps an already-`done` row out of the batch. That
* matters 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 would write that timestamp to the whole batch —
* silently re-dating a task completed days ago.
*
* A view predicate is a client-side hide, though, and the write it guards is
* server-side: an import, a backfill, the dispatcher or an MCP caller
* reassembles the same batch without ever reading this file. So the authority
* lives at the write. `task.hook.ts` REFUSES a predicate write that would
* re-stamp an already-done row (`DULY_TASK_BULK_ALREADY_DONE`, 409) — the one
* route ADR-0058 Addendum II D3 sanctions for a row-conditional decision on a
* batch-scoped payload.
*
* This predicate is kept because it is still the right UX: it stops the
* console from assembling a batch the server would refuse, so a user gets an
* unavailable action rather than an error they did not cause.
* `test/task-hook.test.ts` pins the refusal; `test/task-actions.test.ts` pins
* both layers.
*
* 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
Expand Down
36 changes: 26 additions & 10 deletions test/task-actions.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -531,26 +531,42 @@ describe('bulk', () => {
}
});

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.
it('an already-done row in the batch is refused by the SERVER, not merely hidden', async () => {
// The layer that actually decides. 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 would be
// written to every row in the batch — including one completed days ago.
// ADR-0058 Addendum II D3 hands a `before*` hook exactly one way out of
// that, and `task.hook.ts` takes it: refuse the write.
//
// This is the assertion that makes the `visible` predicate below a
// convenience rather than the only thing between a caller and moved
// history — an import, a backfill, the dispatcher or an MCP caller never
// goes near a view predicate.
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] } } });
const { code, status } = await refusal(
data.update('duly_task', { ...COMPLETE_PATCH }, { multi: true, where: { id: { $in: [open, alreadyDone] } } }),
);
expect(code).toBe('DULY_TASK_BULK_ALREADY_DONE');
expect(status).toBe(409);

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);
'the original completion instant must survive a batch that tried to re-stamp it',
).toBe(original);
expect((await read(open)).status, 'and the refusal writes nothing at all').toBe('open');
});

// So the declaration has to exclude it, and does.
it('and the visible predicate still keeps such a batch from being assembled', async () => {
// The outer layer, kept. `visible` is evaluated once per selected record
// and the run covers only the passing rows, so the console cannot build
// the batch the server now refuses. Defence in depth: the user gets a
// greyed-out row instead of an error they did not cause.
const complete = allBulkDefs().find((d) => d.name === BULK_COMPLETE);
const source = String(complete.visible?.source ?? '');
expect(source).not.toContain('"done"');
Expand Down
162 changes: 162 additions & 0 deletions test/task-hook.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -323,3 +323,165 @@ describe('last_update_at — the stagnation signal', () => {
expect(edited.last_update_at as string < forged).toBe(true);
});
});

// ── The shared-payload path: a predicate (bulk) write ──────────────────────
//
// A `multi: true` update carries ONE payload for all N matched rows —
// `driver.updateMany` takes a single SET clause — and ADR-0058 Addendum II D3
// says what that means for a hook: every per-row `beforeUpdate` context
// carries THAT payload, so "a rewrite takes effect on the WHOLE batch,
// whichever row's dispatch made it". D3 names the consequence outright: a
// rewrite CONDITIONED on the row is *expressible and wrong*, and the sanctioned
// route for row-specific work in a `before*` hook is to THROW.
//
// `completed_at` is exactly such a rewrite — it is stamped on the TRANSITION,
// read off this row's own pre-image. So on this path the hook refuses instead
// of stamping. These assertions run against the real engine, and the dispatch
// mode they turn on is the engine's own (`ctx.dispatch.mode`), measured here
// rather than inferred.
describe('completed_at on a predicate write — one payload, N rows', () => {
/** Assert a refusal by its ENVELOPE (ADR-0112), never by the bare fact that it threw. */
const refusal = async (promise: Promise<unknown>) => {
try {
await promise;
} catch (error: any) {
return { code: error?.code, status: error?.status, message: String(error?.message ?? '') };
}
throw new Error('expected the predicate write to be refused, but it resolved');
};

const complete = async (id: string) => data.update('duly_task', { id, status: 'done' });

it('refuses a done batch that contains an already-done row, and does not move its clock', async () => {
// THE assertion this guard exists for. Without it the open row's dispatch
// stamps `completed_at = now` into the shared payload and the already-done
// row — completed days ago — is silently re-dated.
const open = (await newTask({ subject: 'still open' })).id;
const alreadyDone = (await newTask({ subject: 'done last week' })).id;
await complete(alreadyDone);
const original = (await read(alreadyDone)).completed_at;
expect(original).toBeTruthy();

await tick();
const { code, status, message } = await refusal(
data.update('duly_task', { status: 'done' }, { multi: true, where: { id: { $in: [open, alreadyDone] } } }),
);

expect(code).toBe('DULY_TASK_BULK_ALREADY_DONE');
expect(status).toBe(409);
expect(message, 'the refusal must name the row a caller has to remove').toContain(alreadyDone);

expect(
(await read(alreadyDone)).completed_at,
'the original completion instant must survive the refused batch',
).toBe(original);
});

it('writes nothing at all — the refusal is not a partial batch', async () => {
const open = (await newTask({ subject: 'untouched by a refused batch' })).id;
const alreadyDone = (await newTask({ subject: 'already done' })).id;
await complete(alreadyDone);

await refusal(
data.update('duly_task', { status: 'done' }, { multi: true, where: { id: { $in: [open, alreadyDone] } } }),
);

expect((await read(open)).status, 'the transitioning row must not commit either').toBe('open');
expect((await read(open)).completed_at ?? null).toBeNull();
});

it('refuses whichever dispatch order the batch arrives in', async () => {
// The guard is decided from the ROW alone — its own pre-image and the
// payload — never from what an earlier dispatch happened to leave behind.
// An accumulator would only catch the order in which the done row is
// dispatched second.
for (const doneFirst of [true, false]) {
const openRow = (await newTask({ subject: `order open ${doneFirst}` })).id;
const doneRow = (await newTask({ subject: `order done ${doneFirst}` })).id;
await complete(doneRow);
const original = (await read(doneRow)).completed_at;
await tick();

const ids = doneFirst ? [doneRow, openRow] : [openRow, doneRow];
const { code } = await refusal(
data.update('duly_task', { status: 'done' }, { multi: true, where: { id: { $in: ids } } }),
);

expect(code, `done-first=${doneFirst} must refuse`).toBe('DULY_TASK_BULK_ALREADY_DONE');
expect((await read(doneRow)).completed_at).toBe(original);
}
});

it('still completes a homogeneous batch — every row stamped, in one write', async () => {
// The negative control. The guard must refuse the mixed batch WITHOUT
// taking bulk complete away: a week of ticks in one gesture is the feature.
const ids: string[] = [];
for (let i = 0; i < 5; i += 1) ids.push((await newTask({ subject: `homogeneous ${i}` })).id);

const affected = await data.update('duly_task', { status: 'done' }, {
multi: true,
where: { id: { $in: ids } },
});
expect(affected).toBe(5);

for (const id of ids) {
const row = await read(id);
expect(row.status).toBe('done');
expect(row.completed_at, `${id} must be stamped like any other write`).toBeTruthy();
}
});

it('leaves an administrative predicate write alone — the guard turns on the STATUS in the payload', async () => {
// The over-refusal control, and the one that keeps the seed's second pass
// working: a bulk write that does not carry `status` computes no stamp, so
// there is nothing to leak and nothing to refuse — even over a done row.
const alreadyDone = (await newTask({ subject: 'admin backfill target' })).id;
await complete(alreadyDone);
const original = (await read(alreadyDone)).completed_at;
await tick();

await data.update('duly_task', { business_unit: 'bu_north' }, {
multi: true,
where: { id: { $in: [alreadyDone] } },
});

const row = await read(alreadyDone);
expect(row.business_unit, 'a backfill must still land').toBe('bu_north');
expect(row.completed_at, 'and must not disturb the completion instant').toBe(original);
});

it('does not fire on the single-record path — a re-save of a done task is still a no-op', async () => {
// `mode: 'record'` has a payload of its own, so the row-conditional stamp
// is sound there. Re-sending `status: 'done'` on a done task must keep
// behaving as it always has: accepted, and NOT re-stamped.
const task = await newTask();
const done = await data.update('duly_task', { id: task.id, status: 'done' });
const first = done.completed_at;

await tick();
const resaved = await data.update('duly_task', { id: task.id, status: 'done', note: 'after the fact' });

expect(resaved.completed_at, 'the by-id path is unchanged by the bulk guard').toBe(first);
});

it('a predicate write clearing done is NOT refused — that direction is row-invariant', async () => {
// Reopening a batch sets `completed_at = null`, and null is the correct
// value for EVERY row being moved out of done, including one that was
// never completed. Nothing row-specific leaks, so nothing is refused.
const wasDone = (await newTask({ subject: 'reopen me' })).id;
const neverDone = (await newTask({ subject: 'never completed' })).id;
await complete(wasDone);

const affected = await data.update('duly_task', { status: 'in_progress' }, {
multi: true,
where: { id: { $in: [wasDone, neverDone] } },
});
expect(affected).toBe(2);

for (const id of [wasDone, neverDone]) {
const row = await read(id);
expect(row.status).toBe('in_progress');
expect(row.completed_at ?? null, `${id} must come out of done with no completion`).toBeNull();
}
});
});
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 61 additions & 1 deletion src/hooks/task.hook.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -32,6 +32,48 @@ import type { Hook, HookContext } from '@objectstack/spec/data';
* holding exactly what the caller supplied, so a value derived here
* replaces a caller-supplied one instead of being dropped with it.
*
* ── The shared-payload path, and why this hook REFUSES it ────────────────
* A predicate (`multi: true`) write carries ONE payload for all N matched rows
* — `driver.updateMany` takes a single SET clause — and ADR-0058 Addendum II
* governs what a hook may do with it. D1: a `before*` event dispatches once per
* matched row. D3: every per-row context carries THAT one payload, so "a
* rewrite takes effect on the WHOLE batch, whichever row's dispatch made it",
* and therefore "a rewrite CONDITIONED on the row (`ctx.previous`,
* `ctx.input.id`) is outside this contract". D3 names the sanctioned route in
* as many words: per-row `previous` is supplied "so a guard can REFUSE the
* write, not so a rewrite can be aimed at one row".
*
* `completed_at` is precisely such a rewrite: it is read off THIS row's
* pre-image. Measured against a booted engine — a batch of one `open` row and
* one already-`done` row, both written `status: 'done'` — the open row's
* dispatch stamps `completed_at = now` and the done row's completion instant,
* days old, is silently overwritten. Nothing errors; the history just moves.
*
* So on this path the hook throws. The guard is decided from the ROW ALONE —
* its own pre-image plus the payload — never from what an earlier dispatch left
* in the shared payload, because dispatch order is not the caller's to control
* and an accumulator would only catch the orders in which the done row happens
* to come second.
*
* Three boundaries this guard deliberately holds:
*
* - It turns on `status` being IN THE PAYLOAD. A predicate write that does not
* write `status` computes no stamp, so there is nothing to leak — an
* administrative bulk backfill over done rows, and the seed's `mode:
* 'update'` backdating pass, are untouched.
* - Only the stamping direction is guarded. A batch moving rows OUT of done
* writes `completed_at = null`, and null is the correct value for every row
* being moved out of done, including one that was never completed. That
* rewrite is genuinely row-invariant, so it is allowed.
* - A batch in which EVERY row is already done is refused too, even though
* nothing would leak. The hook cannot see the batch — `dispatch.index` is a
* position, not a total — and a rule stated on the row is one a caller can
* predict and a test can pin. Re-completing a done task in bulk is a caller
* mistake either way, and the answer is now loud instead of silent.
*
* The single-record path (`dispatch.mode === 'record'`) has a payload of its
* own, so the row-conditional stamp is sound there and is unchanged.
*
* ── 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
Expand DownExpand Up@@ -76,6 +118,22 @@ const stampTaskLifecycle = (ctx: HookContext): void => {
const wasDone = previous.status === 'done';
const isDone = nextStatus === 'done';

// ── The shared-payload guard — refuse, never aim a rewrite at one row ──
// `ctx.dispatch` is the engine's own dispatch marker: `'record'` for a
// single-record write, `'per-row'` for one dispatch of a predicate write.
// Conditioned on `status` being in the payload because that is the only
// shape that computes a stamp at all — see the module header.
if (ctx.dispatch?.mode === 'per-row' && 'status' in input && wasDone && isDone) {
throw Object.assign(
new Error(
`Task ${String(input.id ?? previous.id ?? '')} is already done. A bulk status write carries one `
+ 'payload for every matched row, so completing this batch would overwrite that task\'s original '
+ 'completion timestamp. Leave the done rows out of the selection, or write them one at a time.',
),
{ code: 'DULY_TASK_BULK_ALREADY_DONE', status: 409 },
);
}

if (!wasDone && isDone) {
input.completed_at = now;
} else if (wasDone && !isDone) {
Expand DownExpand Up@@ -125,7 +183,9 @@ export const TaskLifecycleHook: Hook = {
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.',
+ 'never on an administrative or bulk write, which would reset the stagnation signal. '
+ 'A predicate write that would re-stamp an already-done row is refused outright '
+ '(ADR-0058 Addendum II D3: one payload for the whole batch, so a guard throws).',
// 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
Expand Down
31 changes: 21 additions & 10 deletions src/views/task.view.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -27,17 +27,28 @@ const columns = [
* 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 ────────────────────────
* ── `visible` here is the OUTER of two layers ─────────────────────────────
* 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.
* that pass, which is what keeps an already-`done` row out of the batch. That
* matters 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 would write that timestamp to the whole batch —
* silently re-dating a task completed days ago.
*
* A view predicate is a client-side hide, though, and the write it guards is
* server-side: an import, a backfill, the dispatcher or an MCP caller
* reassembles the same batch without ever reading this file. So the authority
* lives at the write. `task.hook.ts` REFUSES a predicate write that would
* re-stamp an already-done row (`DULY_TASK_BULK_ALREADY_DONE`, 409) — the one
* route ADR-0058 Addendum II D3 sanctions for a row-conditional decision on a
* batch-scoped payload.
*
* This predicate is kept because it is still the right UX: it stops the
* console from assembling a batch the server would refuse, so a user gets an
* unavailable action rather than an error they did not cause.
* `test/task-hook.test.ts` pins the refusal; `test/task-actions.test.ts` pins
* both layers.
*
* 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
Expand Down
36 changes: 26 additions & 10 deletions test/task-actions.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -531,26 +531,42 @@ describe('bulk', () => {
}
});

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.
it('an already-done row in the batch is refused by the SERVER, not merely hidden', async () => {
// The layer that actually decides. 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 would be
// written to every row in the batch — including one completed days ago.
// ADR-0058 Addendum II D3 hands a `before*` hook exactly one way out of
// that, and `task.hook.ts` takes it: refuse the write.
//
// This is the assertion that makes the `visible` predicate below a
// convenience rather than the only thing between a caller and moved
// history — an import, a backfill, the dispatcher or an MCP caller never
// goes near a view predicate.
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] } } });
const { code, status } = await refusal(
data.update('duly_task', { ...COMPLETE_PATCH }, { multi: true, where: { id: { $in: [open, alreadyDone] } } }),
);
expect(code).toBe('DULY_TASK_BULK_ALREADY_DONE');
expect(status).toBe(409);

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);
'the original completion instant must survive a batch that tried to re-stamp it',
).toBe(original);
expect((await read(open)).status, 'and the refusal writes nothing at all').toBe('open');
});

// So the declaration has to exclude it, and does.
it('and the visible predicate still keeps such a batch from being assembled', async () => {
// The outer layer, kept. `visible` is evaluated once per selected record
// and the run covers only the passing rows, so the console cannot build
// the batch the server now refuses. Defence in depth: the user gets a
// greyed-out row instead of an error they did not cause.
const complete = allBulkDefs().find((d) => d.name === BULK_COMPLETE);
const source = String(complete.visible?.source ?? '');
expect(source).not.toContain('"done"');
Expand Down
162 changes: 162 additions & 0 deletions test/task-hook.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -323,3 +323,165 @@ describe('last_update_at — the stagnation signal', () => {
expect(edited.last_update_at as string < forged).toBe(true);
});
});

// ── The shared-payload path: a predicate (bulk) write ──────────────────────
//
// A `multi: true` update carries ONE payload for all N matched rows —
// `driver.updateMany` takes a single SET clause — and ADR-0058 Addendum II D3
// says what that means for a hook: every per-row `beforeUpdate` context
// carries THAT payload, so "a rewrite takes effect on the WHOLE batch,
// whichever row's dispatch made it". D3 names the consequence outright: a
// rewrite CONDITIONED on the row is *expressible and wrong*, and the sanctioned
// route for row-specific work in a `before*` hook is to THROW.
//
// `completed_at` is exactly such a rewrite — it is stamped on the TRANSITION,
// read off this row's own pre-image. So on this path the hook refuses instead
// of stamping. These assertions run against the real engine, and the dispatch
// mode they turn on is the engine's own (`ctx.dispatch.mode`), measured here
// rather than inferred.
describe('completed_at on a predicate write — one payload, N rows', () => {
/** Assert a refusal by its ENVELOPE (ADR-0112), never by the bare fact that it threw. */
const refusal = async (promise: Promise<unknown>) => {
try {
await promise;
} catch (error: any) {
return { code: error?.code, status: error?.status, message: String(error?.message ?? '') };
}
throw new Error('expected the predicate write to be refused, but it resolved');
};

const complete = async (id: string) => data.update('duly_task', { id, status: 'done' });

it('refuses a done batch that contains an already-done row, and does not move its clock', async () => {
// THE assertion this guard exists for. Without it the open row's dispatch
// stamps `completed_at = now` into the shared payload and the already-done
// row — completed days ago — is silently re-dated.
const open = (await newTask({ subject: 'still open' })).id;
const alreadyDone = (await newTask({ subject: 'done last week' })).id;
await complete(alreadyDone);
const original = (await read(alreadyDone)).completed_at;
expect(original).toBeTruthy();

await tick();
const { code, status, message } = await refusal(
data.update('duly_task', { status: 'done' }, { multi: true, where: { id: { $in: [open, alreadyDone] } } }),
);

expect(code).toBe('DULY_TASK_BULK_ALREADY_DONE');
expect(status).toBe(409);
expect(message, 'the refusal must name the row a caller has to remove').toContain(alreadyDone);

expect(
(await read(alreadyDone)).completed_at,
'the original completion instant must survive the refused batch',
).toBe(original);
});

it('writes nothing at all — the refusal is not a partial batch', async () => {
const open = (await newTask({ subject: 'untouched by a refused batch' })).id;
const alreadyDone = (await newTask({ subject: 'already done' })).id;
await complete(alreadyDone);

await refusal(
data.update('duly_task', { status: 'done' }, { multi: true, where: { id: { $in: [open, alreadyDone] } } }),
);

expect((await read(open)).status, 'the transitioning row must not commit either').toBe('open');
expect((await read(open)).completed_at ?? null).toBeNull();
});

it('refuses whichever dispatch order the batch arrives in', async () => {
// The guard is decided from the ROW alone — its own pre-image and the
// payload — never from what an earlier dispatch happened to leave behind.
// An accumulator would only catch the order in which the done row is
// dispatched second.
for (const doneFirst of [true, false]) {
const openRow = (await newTask({ subject: `order open ${doneFirst}` })).id;
const doneRow = (await newTask({ subject: `order done ${doneFirst}` })).id;
await complete(doneRow);
const original = (await read(doneRow)).completed_at;
await tick();

const ids = doneFirst ? [doneRow, openRow] : [openRow, doneRow];
const { code } = await refusal(
data.update('duly_task', { status: 'done' }, { multi: true, where: { id: { $in: ids } } }),
);

expect(code, `done-first=${doneFirst} must refuse`).toBe('DULY_TASK_BULK_ALREADY_DONE');
expect((await read(doneRow)).completed_at).toBe(original);
}
});

it('still completes a homogeneous batch — every row stamped, in one write', async () => {
// The negative control. The guard must refuse the mixed batch WITHOUT
// taking bulk complete away: a week of ticks in one gesture is the feature.
const ids: string[] = [];
for (let i = 0; i < 5; i += 1) ids.push((await newTask({ subject: `homogeneous ${i}` })).id);

const affected = await data.update('duly_task', { status: 'done' }, {
multi: true,
where: { id: { $in: ids } },
});
expect(affected).toBe(5);

for (const id of ids) {
const row = await read(id);
expect(row.status).toBe('done');
expect(row.completed_at, `${id} must be stamped like any other write`).toBeTruthy();
}
});

it('leaves an administrative predicate write alone — the guard turns on the STATUS in the payload', async () => {
// The over-refusal control, and the one that keeps the seed's second pass
// working: a bulk write that does not carry `status` computes no stamp, so
// there is nothing to leak and nothing to refuse — even over a done row.
const alreadyDone = (await newTask({ subject: 'admin backfill target' })).id;
await complete(alreadyDone);
const original = (await read(alreadyDone)).completed_at;
await tick();

await data.update('duly_task', { business_unit: 'bu_north' }, {
multi: true,
where: { id: { $in: [alreadyDone] } },
});

const row = await read(alreadyDone);
expect(row.business_unit, 'a backfill must still land').toBe('bu_north');
expect(row.completed_at, 'and must not disturb the completion instant').toBe(original);
});

it('does not fire on the single-record path — a re-save of a done task is still a no-op', async () => {
// `mode: 'record'` has a payload of its own, so the row-conditional stamp
// is sound there. Re-sending `status: 'done'` on a done task must keep
// behaving as it always has: accepted, and NOT re-stamped.
const task = await newTask();
const done = await data.update('duly_task', { id: task.id, status: 'done' });
const first = done.completed_at;

await tick();
const resaved = await data.update('duly_task', { id: task.id, status: 'done', note: 'after the fact' });

expect(resaved.completed_at, 'the by-id path is unchanged by the bulk guard').toBe(first);
});

it('a predicate write clearing done is NOT refused — that direction is row-invariant', async () => {
// Reopening a batch sets `completed_at = null`, and null is the correct
// value for EVERY row being moved out of done, including one that was
// never completed. Nothing row-specific leaks, so nothing is refused.
const wasDone = (await newTask({ subject: 'reopen me' })).id;
const neverDone = (await newTask({ subject: 'never completed' })).id;
await complete(wasDone);

const affected = await data.update('duly_task', { status: 'in_progress' }, {
multi: true,
where: { id: { $in: [wasDone, neverDone] } },
});
expect(affected).toBe(2);

for (const id of [wasDone, neverDone]) {
const row = await read(id);
expect(row.status).toBe('in_progress');
expect(row.completed_at ?? null, `${id} must come out of done with no completion`).toBeNull();
}
});
});
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 61 additions & 1 deletion src/hooks/task.hook.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -32,6 +32,48 @@ import type { Hook, HookContext } from '@objectstack/spec/data';
* holding exactly what the caller supplied, so a value derived here
* replaces a caller-supplied one instead of being dropped with it.
*
* ── The shared-payload path, and why this hook REFUSES it ────────────────
* A predicate (`multi: true`) write carries ONE payload for all N matched rows
* — `driver.updateMany` takes a single SET clause — and ADR-0058 Addendum II
* governs what a hook may do with it. D1: a `before*` event dispatches once per
* matched row. D3: every per-row context carries THAT one payload, so "a
* rewrite takes effect on the WHOLE batch, whichever row's dispatch made it",
* and therefore "a rewrite CONDITIONED on the row (`ctx.previous`,
* `ctx.input.id`) is outside this contract". D3 names the sanctioned route in
* as many words: per-row `previous` is supplied "so a guard can REFUSE the
* write, not so a rewrite can be aimed at one row".
*
* `completed_at` is precisely such a rewrite: it is read off THIS row's
* pre-image. Measured against a booted engine — a batch of one `open` row and
* one already-`done` row, both written `status: 'done'` — the open row's
* dispatch stamps `completed_at = now` and the done row's completion instant,
* days old, is silently overwritten. Nothing errors; the history just moves.
*
* So on this path the hook throws. The guard is decided from the ROW ALONE —
* its own pre-image plus the payload — never from what an earlier dispatch left
* in the shared payload, because dispatch order is not the caller's to control
* and an accumulator would only catch the orders in which the done row happens
* to come second.
*
* Three boundaries this guard deliberately holds:
*
* - It turns on `status` being IN THE PAYLOAD. A predicate write that does not
* write `status` computes no stamp, so there is nothing to leak — an
* administrative bulk backfill over done rows, and the seed's `mode:
* 'update'` backdating pass, are untouched.
* - Only the stamping direction is guarded. A batch moving rows OUT of done
* writes `completed_at = null`, and null is the correct value for every row
* being moved out of done, including one that was never completed. That
* rewrite is genuinely row-invariant, so it is allowed.
* - A batch in which EVERY row is already done is refused too, even though
* nothing would leak. The hook cannot see the batch — `dispatch.index` is a
* position, not a total — and a rule stated on the row is one a caller can
* predict and a test can pin. Re-completing a done task in bulk is a caller
* mistake either way, and the answer is now loud instead of silent.
*
* The single-record path (`dispatch.mode === 'record'`) has a payload of its
* own, so the row-conditional stamp is sound there and is unchanged.
*
* ── 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
Expand DownExpand Up@@ -76,6 +118,22 @@ const stampTaskLifecycle = (ctx: HookContext): void => {
const wasDone = previous.status === 'done';
const isDone = nextStatus === 'done';

// ── The shared-payload guard — refuse, never aim a rewrite at one row ──
// `ctx.dispatch` is the engine's own dispatch marker: `'record'` for a
// single-record write, `'per-row'` for one dispatch of a predicate write.
// Conditioned on `status` being in the payload because that is the only
// shape that computes a stamp at all — see the module header.
if (ctx.dispatch?.mode === 'per-row' && 'status' in input && wasDone && isDone) {
throw Object.assign(
new Error(
`Task ${String(input.id ?? previous.id ?? '')} is already done. A bulk status write carries one `
+ 'payload for every matched row, so completing this batch would overwrite that task\'s original '
+ 'completion timestamp. Leave the done rows out of the selection, or write them one at a time.',
),
{ code: 'DULY_TASK_BULK_ALREADY_DONE', status: 409 },
);
}

if (!wasDone && isDone) {
input.completed_at = now;
} else if (wasDone && !isDone) {
Expand DownExpand Up@@ -125,7 +183,9 @@ export const TaskLifecycleHook: Hook = {
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.',
+ 'never on an administrative or bulk write, which would reset the stagnation signal. '
+ 'A predicate write that would re-stamp an already-done row is refused outright '
+ '(ADR-0058 Addendum II D3: one payload for the whole batch, so a guard throws).',
// 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
Expand Down
31 changes: 21 additions & 10 deletions src/views/task.view.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -27,17 +27,28 @@ const columns = [
* 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 ────────────────────────
* ── `visible` here is the OUTER of two layers ─────────────────────────────
* 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.
* that pass, which is what keeps an already-`done` row out of the batch. That
* matters 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 would write that timestamp to the whole batch —
* silently re-dating a task completed days ago.
*
* A view predicate is a client-side hide, though, and the write it guards is
* server-side: an import, a backfill, the dispatcher or an MCP caller
* reassembles the same batch without ever reading this file. So the authority
* lives at the write. `task.hook.ts` REFUSES a predicate write that would
* re-stamp an already-done row (`DULY_TASK_BULK_ALREADY_DONE`, 409) — the one
* route ADR-0058 Addendum II D3 sanctions for a row-conditional decision on a
* batch-scoped payload.
*
* This predicate is kept because it is still the right UX: it stops the
* console from assembling a batch the server would refuse, so a user gets an
* unavailable action rather than an error they did not cause.
* `test/task-hook.test.ts` pins the refusal; `test/task-actions.test.ts` pins
* both layers.
*
* 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
Expand Down
36 changes: 26 additions & 10 deletions test/task-actions.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -531,26 +531,42 @@ describe('bulk', () => {
}
});

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.
it('an already-done row in the batch is refused by the SERVER, not merely hidden', async () => {
// The layer that actually decides. 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 would be
// written to every row in the batch — including one completed days ago.
// ADR-0058 Addendum II D3 hands a `before*` hook exactly one way out of
// that, and `task.hook.ts` takes it: refuse the write.
//
// This is the assertion that makes the `visible` predicate below a
// convenience rather than the only thing between a caller and moved
// history — an import, a backfill, the dispatcher or an MCP caller never
// goes near a view predicate.
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] } } });
const { code, status } = await refusal(
data.update('duly_task', { ...COMPLETE_PATCH }, { multi: true, where: { id: { $in: [open, alreadyDone] } } }),
);
expect(code).toBe('DULY_TASK_BULK_ALREADY_DONE');
expect(status).toBe(409);

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);
'the original completion instant must survive a batch that tried to re-stamp it',
).toBe(original);
expect((await read(open)).status, 'and the refusal writes nothing at all').toBe('open');
});

// So the declaration has to exclude it, and does.
it('and the visible predicate still keeps such a batch from being assembled', async () => {
// The outer layer, kept. `visible` is evaluated once per selected record
// and the run covers only the passing rows, so the console cannot build
// the batch the server now refuses. Defence in depth: the user gets a
// greyed-out row instead of an error they did not cause.
const complete = allBulkDefs().find((d) => d.name === BULK_COMPLETE);
const source = String(complete.visible?.source ?? '');
expect(source).not.toContain('"done"');
Expand Down
162 changes: 162 additions & 0 deletions test/task-hook.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -323,3 +323,165 @@ describe('last_update_at — the stagnation signal', () => {
expect(edited.last_update_at as string < forged).toBe(true);
});
});

// ── The shared-payload path: a predicate (bulk) write ──────────────────────
//
// A `multi: true` update carries ONE payload for all N matched rows —
// `driver.updateMany` takes a single SET clause — and ADR-0058 Addendum II D3
// says what that means for a hook: every per-row `beforeUpdate` context
// carries THAT payload, so "a rewrite takes effect on the WHOLE batch,
// whichever row's dispatch made it". D3 names the consequence outright: a
// rewrite CONDITIONED on the row is *expressible and wrong*, and the sanctioned
// route for row-specific work in a `before*` hook is to THROW.
//
// `completed_at` is exactly such a rewrite — it is stamped on the TRANSITION,
// read off this row's own pre-image. So on this path the hook refuses instead
// of stamping. These assertions run against the real engine, and the dispatch
// mode they turn on is the engine's own (`ctx.dispatch.mode`), measured here
// rather than inferred.
describe('completed_at on a predicate write — one payload, N rows', () => {
/** Assert a refusal by its ENVELOPE (ADR-0112), never by the bare fact that it threw. */
const refusal = async (promise: Promise<unknown>) => {
try {
await promise;
} catch (error: any) {
return { code: error?.code, status: error?.status, message: String(error?.message ?? '') };
}
throw new Error('expected the predicate write to be refused, but it resolved');
};

const complete = async (id: string) => data.update('duly_task', { id, status: 'done' });

it('refuses a done batch that contains an already-done row, and does not move its clock', async () => {
// THE assertion this guard exists for. Without it the open row's dispatch
// stamps `completed_at = now` into the shared payload and the already-done
// row — completed days ago — is silently re-dated.
const open = (await newTask({ subject: 'still open' })).id;
const alreadyDone = (await newTask({ subject: 'done last week' })).id;
await complete(alreadyDone);
const original = (await read(alreadyDone)).completed_at;
expect(original).toBeTruthy();

await tick();
const { code, status, message } = await refusal(
data.update('duly_task', { status: 'done' }, { multi: true, where: { id: { $in: [open, alreadyDone] } } }),
);

expect(code).toBe('DULY_TASK_BULK_ALREADY_DONE');
expect(status).toBe(409);
expect(message, 'the refusal must name the row a caller has to remove').toContain(alreadyDone);

expect(
(await read(alreadyDone)).completed_at,
'the original completion instant must survive the refused batch',
).toBe(original);
});

it('writes nothing at all — the refusal is not a partial batch', async () => {
const open = (await newTask({ subject: 'untouched by a refused batch' })).id;
const alreadyDone = (await newTask({ subject: 'already done' })).id;
await complete(alreadyDone);

await refusal(
data.update('duly_task', { status: 'done' }, { multi: true, where: { id: { $in: [open, alreadyDone] } } }),
);

expect((await read(open)).status, 'the transitioning row must not commit either').toBe('open');
expect((await read(open)).completed_at ?? null).toBeNull();
});

it('refuses whichever dispatch order the batch arrives in', async () => {
// The guard is decided from the ROW alone — its own pre-image and the
// payload — never from what an earlier dispatch happened to leave behind.
// An accumulator would only catch the order in which the done row is
// dispatched second.
for (const doneFirst of [true, false]) {
const openRow = (await newTask({ subject: `order open ${doneFirst}` })).id;
const doneRow = (await newTask({ subject: `order done ${doneFirst}` })).id;
await complete(doneRow);
const original = (await read(doneRow)).completed_at;
await tick();

const ids = doneFirst ? [doneRow, openRow] : [openRow, doneRow];
const { code } = await refusal(
data.update('duly_task', { status: 'done' }, { multi: true, where: { id: { $in: ids } } }),
);

expect(code, `done-first=${doneFirst} must refuse`).toBe('DULY_TASK_BULK_ALREADY_DONE');
expect((await read(doneRow)).completed_at).toBe(original);
}
});

it('still completes a homogeneous batch — every row stamped, in one write', async () => {
// The negative control. The guard must refuse the mixed batch WITHOUT
// taking bulk complete away: a week of ticks in one gesture is the feature.
const ids: string[] = [];
for (let i = 0; i < 5; i += 1) ids.push((await newTask({ subject: `homogeneous ${i}` })).id);

const affected = await data.update('duly_task', { status: 'done' }, {
multi: true,
where: { id: { $in: ids } },
});
expect(affected).toBe(5);

for (const id of ids) {
const row = await read(id);
expect(row.status).toBe('done');
expect(row.completed_at, `${id} must be stamped like any other write`).toBeTruthy();
}
});

it('leaves an administrative predicate write alone — the guard turns on the STATUS in the payload', async () => {
// The over-refusal control, and the one that keeps the seed's second pass
// working: a bulk write that does not carry `status` computes no stamp, so
// there is nothing to leak and nothing to refuse — even over a done row.
const alreadyDone = (await newTask({ subject: 'admin backfill target' })).id;
await complete(alreadyDone);
const original = (await read(alreadyDone)).completed_at;
await tick();

await data.update('duly_task', { business_unit: 'bu_north' }, {
multi: true,
where: { id: { $in: [alreadyDone] } },
});

const row = await read(alreadyDone);
expect(row.business_unit, 'a backfill must still land').toBe('bu_north');
expect(row.completed_at, 'and must not disturb the completion instant').toBe(original);
});

it('does not fire on the single-record path — a re-save of a done task is still a no-op', async () => {
// `mode: 'record'` has a payload of its own, so the row-conditional stamp
// is sound there. Re-sending `status: 'done'` on a done task must keep
// behaving as it always has: accepted, and NOT re-stamped.
const task = await newTask();
const done = await data.update('duly_task', { id: task.id, status: 'done' });
const first = done.completed_at;

await tick();
const resaved = await data.update('duly_task', { id: task.id, status: 'done', note: 'after the fact' });

expect(resaved.completed_at, 'the by-id path is unchanged by the bulk guard').toBe(first);
});

it('a predicate write clearing done is NOT refused — that direction is row-invariant', async () => {
// Reopening a batch sets `completed_at = null`, and null is the correct
// value for EVERY row being moved out of done, including one that was
// never completed. Nothing row-specific leaks, so nothing is refused.
const wasDone = (await newTask({ subject: 'reopen me' })).id;
const neverDone = (await newTask({ subject: 'never completed' })).id;
await complete(wasDone);

const affected = await data.update('duly_task', { status: 'in_progress' }, {
multi: true,
where: { id: { $in: [wasDone, neverDone] } },
});
expect(affected).toBe(2);

for (const id of [wasDone, neverDone]) {
const row = await read(id);
expect(row.status).toBe('in_progress');
expect(row.completed_at ?? null, `${id} must come out of done with no completion`).toBeNull();
}
});
});
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 61 additions & 1 deletion src/hooks/task.hook.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -32,6 +32,48 @@ import type { Hook, HookContext } from '@objectstack/spec/data';
* holding exactly what the caller supplied, so a value derived here
* replaces a caller-supplied one instead of being dropped with it.
*
* ── The shared-payload path, and why this hook REFUSES it ────────────────
* A predicate (`multi: true`) write carries ONE payload for all N matched rows
* — `driver.updateMany` takes a single SET clause — and ADR-0058 Addendum II
* governs what a hook may do with it. D1: a `before*` event dispatches once per
* matched row. D3: every per-row context carries THAT one payload, so "a
* rewrite takes effect on the WHOLE batch, whichever row's dispatch made it",
* and therefore "a rewrite CONDITIONED on the row (`ctx.previous`,
* `ctx.input.id`) is outside this contract". D3 names the sanctioned route in
* as many words: per-row `previous` is supplied "so a guard can REFUSE the
* write, not so a rewrite can be aimed at one row".
*
* `completed_at` is precisely such a rewrite: it is read off THIS row's
* pre-image. Measured against a booted engine — a batch of one `open` row and
* one already-`done` row, both written `status: 'done'` — the open row's
* dispatch stamps `completed_at = now` and the done row's completion instant,
* days old, is silently overwritten. Nothing errors; the history just moves.
*
* So on this path the hook throws. The guard is decided from the ROW ALONE —
* its own pre-image plus the payload — never from what an earlier dispatch left
* in the shared payload, because dispatch order is not the caller's to control
* and an accumulator would only catch the orders in which the done row happens
* to come second.
*
* Three boundaries this guard deliberately holds:
*
* - It turns on `status` being IN THE PAYLOAD. A predicate write that does not
* write `status` computes no stamp, so there is nothing to leak — an
* administrative bulk backfill over done rows, and the seed's `mode:
* 'update'` backdating pass, are untouched.
* - Only the stamping direction is guarded. A batch moving rows OUT of done
* writes `completed_at = null`, and null is the correct value for every row
* being moved out of done, including one that was never completed. That
* rewrite is genuinely row-invariant, so it is allowed.
* - A batch in which EVERY row is already done is refused too, even though
* nothing would leak. The hook cannot see the batch — `dispatch.index` is a
* position, not a total — and a rule stated on the row is one a caller can
* predict and a test can pin. Re-completing a done task in bulk is a caller
* mistake either way, and the answer is now loud instead of silent.
*
* The single-record path (`dispatch.mode === 'record'`) has a payload of its
* own, so the row-conditional stamp is sound there and is unchanged.
*
* ── 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
Expand DownExpand Up@@ -76,6 +118,22 @@ const stampTaskLifecycle = (ctx: HookContext): void => {
const wasDone = previous.status === 'done';
const isDone = nextStatus === 'done';

// ── The shared-payload guard — refuse, never aim a rewrite at one row ──
// `ctx.dispatch` is the engine's own dispatch marker: `'record'` for a
// single-record write, `'per-row'` for one dispatch of a predicate write.
// Conditioned on `status` being in the payload because that is the only
// shape that computes a stamp at all — see the module header.
if (ctx.dispatch?.mode === 'per-row' && 'status' in input && wasDone && isDone) {
throw Object.assign(
new Error(
`Task ${String(input.id ?? previous.id ?? '')} is already done. A bulk status write carries one `
+ 'payload for every matched row, so completing this batch would overwrite that task\'s original '
+ 'completion timestamp. Leave the done rows out of the selection, or write them one at a time.',
),
{ code: 'DULY_TASK_BULK_ALREADY_DONE', status: 409 },
);
}

if (!wasDone && isDone) {
input.completed_at = now;
} else if (wasDone && !isDone) {
Expand DownExpand Up@@ -125,7 +183,9 @@ export const TaskLifecycleHook: Hook = {
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.',
+ 'never on an administrative or bulk write, which would reset the stagnation signal. '
+ 'A predicate write that would re-stamp an already-done row is refused outright '
+ '(ADR-0058 Addendum II D3: one payload for the whole batch, so a guard throws).',
// 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
Expand Down
31 changes: 21 additions & 10 deletions src/views/task.view.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -27,17 +27,28 @@ const columns = [
* 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 ────────────────────────
* ── `visible` here is the OUTER of two layers ─────────────────────────────
* 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.
* that pass, which is what keeps an already-`done` row out of the batch. That
* matters 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 would write that timestamp to the whole batch —
* silently re-dating a task completed days ago.
*
* A view predicate is a client-side hide, though, and the write it guards is
* server-side: an import, a backfill, the dispatcher or an MCP caller
* reassembles the same batch without ever reading this file. So the authority
* lives at the write. `task.hook.ts` REFUSES a predicate write that would
* re-stamp an already-done row (`DULY_TASK_BULK_ALREADY_DONE`, 409) — the one
* route ADR-0058 Addendum II D3 sanctions for a row-conditional decision on a
* batch-scoped payload.
*
* This predicate is kept because it is still the right UX: it stops the
* console from assembling a batch the server would refuse, so a user gets an
* unavailable action rather than an error they did not cause.
* `test/task-hook.test.ts` pins the refusal; `test/task-actions.test.ts` pins
* both layers.
*
* 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
Expand Down
36 changes: 26 additions & 10 deletions test/task-actions.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -531,26 +531,42 @@ describe('bulk', () => {
}
});

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.
it('an already-done row in the batch is refused by the SERVER, not merely hidden', async () => {
// The layer that actually decides. 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 would be
// written to every row in the batch — including one completed days ago.
// ADR-0058 Addendum II D3 hands a `before*` hook exactly one way out of
// that, and `task.hook.ts` takes it: refuse the write.
//
// This is the assertion that makes the `visible` predicate below a
// convenience rather than the only thing between a caller and moved
// history — an import, a backfill, the dispatcher or an MCP caller never
// goes near a view predicate.
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] } } });
const { code, status } = await refusal(
data.update('duly_task', { ...COMPLETE_PATCH }, { multi: true, where: { id: { $in: [open, alreadyDone] } } }),
);
expect(code).toBe('DULY_TASK_BULK_ALREADY_DONE');
expect(status).toBe(409);

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);
'the original completion instant must survive a batch that tried to re-stamp it',
).toBe(original);
expect((await read(open)).status, 'and the refusal writes nothing at all').toBe('open');
});

// So the declaration has to exclude it, and does.
it('and the visible predicate still keeps such a batch from being assembled', async () => {
// The outer layer, kept. `visible` is evaluated once per selected record
// and the run covers only the passing rows, so the console cannot build
// the batch the server now refuses. Defence in depth: the user gets a
// greyed-out row instead of an error they did not cause.
const complete = allBulkDefs().find((d) => d.name === BULK_COMPLETE);
const source = String(complete.visible?.source ?? '');
expect(source).not.toContain('"done"');
Expand Down
162 changes: 162 additions & 0 deletions test/task-hook.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -323,3 +323,165 @@ describe('last_update_at — the stagnation signal', () => {
expect(edited.last_update_at as string < forged).toBe(true);
});
});

// ── The shared-payload path: a predicate (bulk) write ──────────────────────
//
// A `multi: true` update carries ONE payload for all N matched rows —
// `driver.updateMany` takes a single SET clause — and ADR-0058 Addendum II D3
// says what that means for a hook: every per-row `beforeUpdate` context
// carries THAT payload, so "a rewrite takes effect on the WHOLE batch,
// whichever row's dispatch made it". D3 names the consequence outright: a
// rewrite CONDITIONED on the row is *expressible and wrong*, and the sanctioned
// route for row-specific work in a `before*` hook is to THROW.
//
// `completed_at` is exactly such a rewrite — it is stamped on the TRANSITION,
// read off this row's own pre-image. So on this path the hook refuses instead
// of stamping. These assertions run against the real engine, and the dispatch
// mode they turn on is the engine's own (`ctx.dispatch.mode`), measured here
// rather than inferred.
describe('completed_at on a predicate write — one payload, N rows', () => {
/** Assert a refusal by its ENVELOPE (ADR-0112), never by the bare fact that it threw. */
const refusal = async (promise: Promise<unknown>) => {
try {
await promise;
} catch (error: any) {
return { code: error?.code, status: error?.status, message: String(error?.message ?? '') };
}
throw new Error('expected the predicate write to be refused, but it resolved');
};

const complete = async (id: string) => data.update('duly_task', { id, status: 'done' });

it('refuses a done batch that contains an already-done row, and does not move its clock', async () => {
// THE assertion this guard exists for. Without it the open row's dispatch
// stamps `completed_at = now` into the shared payload and the already-done
// row — completed days ago — is silently re-dated.
const open = (await newTask({ subject: 'still open' })).id;
const alreadyDone = (await newTask({ subject: 'done last week' })).id;
await complete(alreadyDone);
const original = (await read(alreadyDone)).completed_at;
expect(original).toBeTruthy();

await tick();
const { code, status, message } = await refusal(
data.update('duly_task', { status: 'done' }, { multi: true, where: { id: { $in: [open, alreadyDone] } } }),
);

expect(code).toBe('DULY_TASK_BULK_ALREADY_DONE');
expect(status).toBe(409);
expect(message, 'the refusal must name the row a caller has to remove').toContain(alreadyDone);

expect(
(await read(alreadyDone)).completed_at,
'the original completion instant must survive the refused batch',
).toBe(original);
});

it('writes nothing at all — the refusal is not a partial batch', async () => {
const open = (await newTask({ subject: 'untouched by a refused batch' })).id;
const alreadyDone = (await newTask({ subject: 'already done' })).id;
await complete(alreadyDone);

await refusal(
data.update('duly_task', { status: 'done' }, { multi: true, where: { id: { $in: [open, alreadyDone] } } }),
);

expect((await read(open)).status, 'the transitioning row must not commit either').toBe('open');
expect((await read(open)).completed_at ?? null).toBeNull();
});

it('refuses whichever dispatch order the batch arrives in', async () => {
// The guard is decided from the ROW alone — its own pre-image and the
// payload — never from what an earlier dispatch happened to leave behind.
// An accumulator would only catch the order in which the done row is
// dispatched second.
for (const doneFirst of [true, false]) {
const openRow = (await newTask({ subject: `order open ${doneFirst}` })).id;
const doneRow = (await newTask({ subject: `order done ${doneFirst}` })).id;
await complete(doneRow);
const original = (await read(doneRow)).completed_at;
await tick();

const ids = doneFirst ? [doneRow, openRow] : [openRow, doneRow];
const { code } = await refusal(
data.update('duly_task', { status: 'done' }, { multi: true, where: { id: { $in: ids } } }),
);

expect(code, `done-first=${doneFirst} must refuse`).toBe('DULY_TASK_BULK_ALREADY_DONE');
expect((await read(doneRow)).completed_at).toBe(original);
}
});

it('still completes a homogeneous batch — every row stamped, in one write', async () => {
// The negative control. The guard must refuse the mixed batch WITHOUT
// taking bulk complete away: a week of ticks in one gesture is the feature.
const ids: string[] = [];
for (let i = 0; i < 5; i += 1) ids.push((await newTask({ subject: `homogeneous ${i}` })).id);

const affected = await data.update('duly_task', { status: 'done' }, {
multi: true,
where: { id: { $in: ids } },
});
expect(affected).toBe(5);

for (const id of ids) {
const row = await read(id);
expect(row.status).toBe('done');
expect(row.completed_at, `${id} must be stamped like any other write`).toBeTruthy();
}
});

it('leaves an administrative predicate write alone — the guard turns on the STATUS in the payload', async () => {
// The over-refusal control, and the one that keeps the seed's second pass
// working: a bulk write that does not carry `status` computes no stamp, so
// there is nothing to leak and nothing to refuse — even over a done row.
const alreadyDone = (await newTask({ subject: 'admin backfill target' })).id;
await complete(alreadyDone);
const original = (await read(alreadyDone)).completed_at;
await tick();

await data.update('duly_task', { business_unit: 'bu_north' }, {
multi: true,
where: { id: { $in: [alreadyDone] } },
});

const row = await read(alreadyDone);
expect(row.business_unit, 'a backfill must still land').toBe('bu_north');
expect(row.completed_at, 'and must not disturb the completion instant').toBe(original);
});

it('does not fire on the single-record path — a re-save of a done task is still a no-op', async () => {
// `mode: 'record'` has a payload of its own, so the row-conditional stamp
// is sound there. Re-sending `status: 'done'` on a done task must keep
// behaving as it always has: accepted, and NOT re-stamped.
const task = await newTask();
const done = await data.update('duly_task', { id: task.id, status: 'done' });
const first = done.completed_at;

await tick();
const resaved = await data.update('duly_task', { id: task.id, status: 'done', note: 'after the fact' });

expect(resaved.completed_at, 'the by-id path is unchanged by the bulk guard').toBe(first);
});

it('a predicate write clearing done is NOT refused — that direction is row-invariant', async () => {
// Reopening a batch sets `completed_at = null`, and null is the correct
// value for EVERY row being moved out of done, including one that was
// never completed. Nothing row-specific leaks, so nothing is refused.
const wasDone = (await newTask({ subject: 'reopen me' })).id;
const neverDone = (await newTask({ subject: 'never completed' })).id;
await complete(wasDone);

const affected = await data.update('duly_task', { status: 'in_progress' }, {
multi: true,
where: { id: { $in: [wasDone, neverDone] } },
});
expect(affected).toBe(2);

for (const id of [wasDone, neverDone]) {
const row = await read(id);
expect(row.status).toBe('in_progress');
expect(row.completed_at ?? null, `${id} must come out of done with no completion`).toBeNull();
}
});
});
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 61 additions & 1 deletion src/hooks/task.hook.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -32,6 +32,48 @@ import type { Hook, HookContext } from '@objectstack/spec/data';
* holding exactly what the caller supplied, so a value derived here
* replaces a caller-supplied one instead of being dropped with it.
*
* ── The shared-payload path, and why this hook REFUSES it ────────────────
* A predicate (`multi: true`) write carries ONE payload for all N matched rows
* — `driver.updateMany` takes a single SET clause — and ADR-0058 Addendum II
* governs what a hook may do with it. D1: a `before*` event dispatches once per
* matched row. D3: every per-row context carries THAT one payload, so "a
* rewrite takes effect on the WHOLE batch, whichever row's dispatch made it",
* and therefore "a rewrite CONDITIONED on the row (`ctx.previous`,
* `ctx.input.id`) is outside this contract". D3 names the sanctioned route in
* as many words: per-row `previous` is supplied "so a guard can REFUSE the
* write, not so a rewrite can be aimed at one row".
*
* `completed_at` is precisely such a rewrite: it is read off THIS row's
* pre-image. Measured against a booted engine — a batch of one `open` row and
* one already-`done` row, both written `status: 'done'` — the open row's
* dispatch stamps `completed_at = now` and the done row's completion instant,
* days old, is silently overwritten. Nothing errors; the history just moves.
*
* So on this path the hook throws. The guard is decided from the ROW ALONE —
* its own pre-image plus the payload — never from what an earlier dispatch left
* in the shared payload, because dispatch order is not the caller's to control
* and an accumulator would only catch the orders in which the done row happens
* to come second.
*
* Three boundaries this guard deliberately holds:
*
* - It turns on `status` being IN THE PAYLOAD. A predicate write that does not
* write `status` computes no stamp, so there is nothing to leak — an
* administrative bulk backfill over done rows, and the seed's `mode:
* 'update'` backdating pass, are untouched.
* - Only the stamping direction is guarded. A batch moving rows OUT of done
* writes `completed_at = null`, and null is the correct value for every row
* being moved out of done, including one that was never completed. That
* rewrite is genuinely row-invariant, so it is allowed.
* - A batch in which EVERY row is already done is refused too, even though
* nothing would leak. The hook cannot see the batch — `dispatch.index` is a
* position, not a total — and a rule stated on the row is one a caller can
* predict and a test can pin. Re-completing a done task in bulk is a caller
* mistake either way, and the answer is now loud instead of silent.
*
* The single-record path (`dispatch.mode === 'record'`) has a payload of its
* own, so the row-conditional stamp is sound there and is unchanged.
*
* ── 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
Expand DownExpand Up@@ -76,6 +118,22 @@ const stampTaskLifecycle = (ctx: HookContext): void => {
const wasDone = previous.status === 'done';
const isDone = nextStatus === 'done';

// ── The shared-payload guard — refuse, never aim a rewrite at one row ──
// `ctx.dispatch` is the engine's own dispatch marker: `'record'` for a
// single-record write, `'per-row'` for one dispatch of a predicate write.
// Conditioned on `status` being in the payload because that is the only
// shape that computes a stamp at all — see the module header.
if (ctx.dispatch?.mode === 'per-row' && 'status' in input && wasDone && isDone) {
throw Object.assign(
new Error(
`Task ${String(input.id ?? previous.id ?? '')} is already done. A bulk status write carries one `
+ 'payload for every matched row, so completing this batch would overwrite that task\'s original '
+ 'completion timestamp. Leave the done rows out of the selection, or write them one at a time.',
),
{ code: 'DULY_TASK_BULK_ALREADY_DONE', status: 409 },
);
}

if (!wasDone && isDone) {
input.completed_at = now;
} else if (wasDone && !isDone) {
Expand DownExpand Up@@ -125,7 +183,9 @@ export const TaskLifecycleHook: Hook = {
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.',
+ 'never on an administrative or bulk write, which would reset the stagnation signal. '
+ 'A predicate write that would re-stamp an already-done row is refused outright '
+ '(ADR-0058 Addendum II D3: one payload for the whole batch, so a guard throws).',
// 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
Expand Down
31 changes: 21 additions & 10 deletions src/views/task.view.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -27,17 +27,28 @@ const columns = [
* 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 ────────────────────────
* ── `visible` here is the OUTER of two layers ─────────────────────────────
* 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.
* that pass, which is what keeps an already-`done` row out of the batch. That
* matters 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 would write that timestamp to the whole batch —
* silently re-dating a task completed days ago.
*
* A view predicate is a client-side hide, though, and the write it guards is
* server-side: an import, a backfill, the dispatcher or an MCP caller
* reassembles the same batch without ever reading this file. So the authority
* lives at the write. `task.hook.ts` REFUSES a predicate write that would
* re-stamp an already-done row (`DULY_TASK_BULK_ALREADY_DONE`, 409) — the one
* route ADR-0058 Addendum II D3 sanctions for a row-conditional decision on a
* batch-scoped payload.
*
* This predicate is kept because it is still the right UX: it stops the
* console from assembling a batch the server would refuse, so a user gets an
* unavailable action rather than an error they did not cause.
* `test/task-hook.test.ts` pins the refusal; `test/task-actions.test.ts` pins
* both layers.
*
* 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
Expand Down
36 changes: 26 additions & 10 deletions test/task-actions.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -531,26 +531,42 @@ describe('bulk', () => {
}
});

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.
it('an already-done row in the batch is refused by the SERVER, not merely hidden', async () => {
// The layer that actually decides. 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 would be
// written to every row in the batch — including one completed days ago.
// ADR-0058 Addendum II D3 hands a `before*` hook exactly one way out of
// that, and `task.hook.ts` takes it: refuse the write.
//
// This is the assertion that makes the `visible` predicate below a
// convenience rather than the only thing between a caller and moved
// history — an import, a backfill, the dispatcher or an MCP caller never
// goes near a view predicate.
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] } } });
const { code, status } = await refusal(
data.update('duly_task', { ...COMPLETE_PATCH }, { multi: true, where: { id: { $in: [open, alreadyDone] } } }),
);
expect(code).toBe('DULY_TASK_BULK_ALREADY_DONE');
expect(status).toBe(409);

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);
'the original completion instant must survive a batch that tried to re-stamp it',
).toBe(original);
expect((await read(open)).status, 'and the refusal writes nothing at all').toBe('open');
});

// So the declaration has to exclude it, and does.
it('and the visible predicate still keeps such a batch from being assembled', async () => {
// The outer layer, kept. `visible` is evaluated once per selected record
// and the run covers only the passing rows, so the console cannot build
// the batch the server now refuses. Defence in depth: the user gets a
// greyed-out row instead of an error they did not cause.
const complete = allBulkDefs().find((d) => d.name === BULK_COMPLETE);
const source = String(complete.visible?.source ?? '');
expect(source).not.toContain('"done"');
Expand Down
162 changes: 162 additions & 0 deletions test/task-hook.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -323,3 +323,165 @@ describe('last_update_at — the stagnation signal', () => {
expect(edited.last_update_at as string < forged).toBe(true);
});
});

// ── The shared-payload path: a predicate (bulk) write ──────────────────────
//
// A `multi: true` update carries ONE payload for all N matched rows —
// `driver.updateMany` takes a single SET clause — and ADR-0058 Addendum II D3
// says what that means for a hook: every per-row `beforeUpdate` context
// carries THAT payload, so "a rewrite takes effect on the WHOLE batch,
// whichever row's dispatch made it". D3 names the consequence outright: a
// rewrite CONDITIONED on the row is *expressible and wrong*, and the sanctioned
// route for row-specific work in a `before*` hook is to THROW.
//
// `completed_at` is exactly such a rewrite — it is stamped on the TRANSITION,
// read off this row's own pre-image. So on this path the hook refuses instead
// of stamping. These assertions run against the real engine, and the dispatch
// mode they turn on is the engine's own (`ctx.dispatch.mode`), measured here
// rather than inferred.
describe('completed_at on a predicate write — one payload, N rows', () => {
/** Assert a refusal by its ENVELOPE (ADR-0112), never by the bare fact that it threw. */
const refusal = async (promise: Promise<unknown>) => {
try {
await promise;
} catch (error: any) {
return { code: error?.code, status: error?.status, message: String(error?.message ?? '') };
}
throw new Error('expected the predicate write to be refused, but it resolved');
};

const complete = async (id: string) => data.update('duly_task', { id, status: 'done' });

it('refuses a done batch that contains an already-done row, and does not move its clock', async () => {
// THE assertion this guard exists for. Without it the open row's dispatch
// stamps `completed_at = now` into the shared payload and the already-done
// row — completed days ago — is silently re-dated.
const open = (await newTask({ subject: 'still open' })).id;
const alreadyDone = (await newTask({ subject: 'done last week' })).id;
await complete(alreadyDone);
const original = (await read(alreadyDone)).completed_at;
expect(original).toBeTruthy();

await tick();
const { code, status, message } = await refusal(
data.update('duly_task', { status: 'done' }, { multi: true, where: { id: { $in: [open, alreadyDone] } } }),
);

expect(code).toBe('DULY_TASK_BULK_ALREADY_DONE');
expect(status).toBe(409);
expect(message, 'the refusal must name the row a caller has to remove').toContain(alreadyDone);

expect(
(await read(alreadyDone)).completed_at,
'the original completion instant must survive the refused batch',
).toBe(original);
});

it('writes nothing at all — the refusal is not a partial batch', async () => {
const open = (await newTask({ subject: 'untouched by a refused batch' })).id;
const alreadyDone = (await newTask({ subject: 'already done' })).id;
await complete(alreadyDone);

await refusal(
data.update('duly_task', { status: 'done' }, { multi: true, where: { id: { $in: [open, alreadyDone] } } }),
);

expect((await read(open)).status, 'the transitioning row must not commit either').toBe('open');
expect((await read(open)).completed_at ?? null).toBeNull();
});

it('refuses whichever dispatch order the batch arrives in', async () => {
// The guard is decided from the ROW alone — its own pre-image and the
// payload — never from what an earlier dispatch happened to leave behind.
// An accumulator would only catch the order in which the done row is
// dispatched second.
for (const doneFirst of [true, false]) {
const openRow = (await newTask({ subject: `order open ${doneFirst}` })).id;
const doneRow = (await newTask({ subject: `order done ${doneFirst}` })).id;
await complete(doneRow);
const original = (await read(doneRow)).completed_at;
await tick();

const ids = doneFirst ? [doneRow, openRow] : [openRow, doneRow];
const { code } = await refusal(
data.update('duly_task', { status: 'done' }, { multi: true, where: { id: { $in: ids } } }),
);

expect(code, `done-first=${doneFirst} must refuse`).toBe('DULY_TASK_BULK_ALREADY_DONE');
expect((await read(doneRow)).completed_at).toBe(original);
}
});

it('still completes a homogeneous batch — every row stamped, in one write', async () => {
// The negative control. The guard must refuse the mixed batch WITHOUT
// taking bulk complete away: a week of ticks in one gesture is the feature.
const ids: string[] = [];
for (let i = 0; i < 5; i += 1) ids.push((await newTask({ subject: `homogeneous ${i}` })).id);

const affected = await data.update('duly_task', { status: 'done' }, {
multi: true,
where: { id: { $in: ids } },
});
expect(affected).toBe(5);

for (const id of ids) {
const row = await read(id);
expect(row.status).toBe('done');
expect(row.completed_at, `${id} must be stamped like any other write`).toBeTruthy();
}
});

it('leaves an administrative predicate write alone — the guard turns on the STATUS in the payload', async () => {
// The over-refusal control, and the one that keeps the seed's second pass
// working: a bulk write that does not carry `status` computes no stamp, so
// there is nothing to leak and nothing to refuse — even over a done row.
const alreadyDone = (await newTask({ subject: 'admin backfill target' })).id;
await complete(alreadyDone);
const original = (await read(alreadyDone)).completed_at;
await tick();

await data.update('duly_task', { business_unit: 'bu_north' }, {
multi: true,
where: { id: { $in: [alreadyDone] } },
});

const row = await read(alreadyDone);
expect(row.business_unit, 'a backfill must still land').toBe('bu_north');
expect(row.completed_at, 'and must not disturb the completion instant').toBe(original);
});

it('does not fire on the single-record path — a re-save of a done task is still a no-op', async () => {
// `mode: 'record'` has a payload of its own, so the row-conditional stamp
// is sound there. Re-sending `status: 'done'` on a done task must keep
// behaving as it always has: accepted, and NOT re-stamped.
const task = await newTask();
const done = await data.update('duly_task', { id: task.id, status: 'done' });
const first = done.completed_at;

await tick();
const resaved = await data.update('duly_task', { id: task.id, status: 'done', note: 'after the fact' });

expect(resaved.completed_at, 'the by-id path is unchanged by the bulk guard').toBe(first);
});

it('a predicate write clearing done is NOT refused — that direction is row-invariant', async () => {
// Reopening a batch sets `completed_at = null`, and null is the correct
// value for EVERY row being moved out of done, including one that was
// never completed. Nothing row-specific leaks, so nothing is refused.
const wasDone = (await newTask({ subject: 'reopen me' })).id;
const neverDone = (await newTask({ subject: 'never completed' })).id;
await complete(wasDone);

const affected = await data.update('duly_task', { status: 'in_progress' }, {
multi: true,
where: { id: { $in: [wasDone, neverDone] } },
});
expect(affected).toBe(2);

for (const id of [wasDone, neverDone]) {
const row = await read(id);
expect(row.status).toBe('in_progress');
expect(row.completed_at ?? null, `${id} must come out of done with no completion`).toBeNull();
}
});
});
Loading