From 13feae864c3dbec4e7d32d80006f00c3798287b0 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 03:52:55 +0000 Subject: [PATCH 1/2] Catalog instantiation: apply and sync a position's duty catalog MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds two object-less actions and their handlers: - duly_catalog_apply — instantiate every active duly_catalog_item for a position onto one or more people. Idempotent on (catalog_item, owner): a second apply creates nothing and reports the skips. - duly_catalog_sync — replay catalog CADENCE edits (frequency, due_anchor, due_offset_days, lead_days, grace_days) onto derived duties. Never touches owner, status, timezone or the effective_* window; never touches a duty whose source is 'self'; reports duties from deactivated catalog items rather than deleting them. The handler↔declaration wiring is asserted in tests because no author-time gate covers it: an unregistered handler renders, is clickable, and 404s at call time. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01SqkTcrxUFci7nqXdbBSe2p --- src/actions/catalog.actions.ts | 116 ++++++ src/actions/catalog.handlers.ts | Bin 0 -> 17555 bytes src/actions/index.ts | 6 +- src/actions/register-handlers.ts | 5 +- test/catalog-instantiate.test.ts | 589 +++++++++++++++++++++++++++++++ 5 files changed, 714 insertions(+), 2 deletions(-) create mode 100644 src/actions/catalog.actions.ts create mode 100644 src/actions/catalog.handlers.ts create mode 100644 test/catalog-instantiate.test.ts diff --git a/src/actions/catalog.actions.ts b/src/actions/catalog.actions.ts new file mode 100644 index 0000000..98bf4bd --- /dev/null +++ b/src/actions/catalog.actions.ts @@ -0,0 +1,116 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { defineAction } from '@objectstack/spec'; + +import { + CATALOG_APPLY_ACTION, + CATALOG_SYNC_ACTION, +} from './catalog.handlers.js'; + +/** + * Catalog instantiation — the onboarding path. + * + * Customers arrive with their catalog already written, usually as a + * spreadsheet. Taking a position has to mean "apply the list", not "hand-type + * 26 duties", or the rollout dies in week one. + * + * ── Why these are OBJECT-LESS and headless ──────────────────────────────── + * Neither action operates on a record: `duly_catalog_apply` reads a whole + * position's worth of `duly_catalog_item` rows and writes `duly_duty` rows for + * several people at once. That makes it a GLOBAL action — no `objectName` — and + * an object-less action in protocol 17 has no UI home to declare. + * `global_nav` was removed from `ACTION_LOCATIONS` in @objectstack/spec 17 + * (#6888, ADR-0049 enforce-or-remove): the console's ⌘K palette reads no action + * metadata, so the location never rendered. Every surviving location + * (`list_toolbar`, `list_item`, `record_*`) is bound to an object. + * + * So `locations: []` is the honest declaration the spec itself prescribes for + * this case — it keeps the param contract, the capability gate and the audit + * trail, and the action is invoked over the platform action route + * (`POST /api/v1/actions/global/duly_catalog_apply`) or MCP rather than from a + * button. Declaring a location a renderer does not serve would be the + * ADR-0078 declares-renders-does-nothing shape. + * + * ── Why `type: 'script'` with a `target` and no `body` ──────────────────── + * The cadence maths and the idempotency probe are real code with real tests, + * not a sandboxed L1/L2 snippet. `target` names the handler registered from + * `src/actions/register-handlers.ts`; a `script` action with neither `body` + * nor `target` is rejected at author time precisely because it would otherwise + * render, be clickable, and 404 at call time. + */ + +/** + * `duly_catalog_apply` — instantiate a position's catalog onto people. + * + * `position_code` is free text on purpose: a customer can load their catalog on + * day one, before positions are modelled in the platform, so this deliberately + * does NOT pick from `sys_position` and does NOT require a `sys_user_position` + * row to exist for the selected people. + */ +export const CatalogApplyAction = defineAction({ + name: CATALOG_APPLY_ACTION, + label: 'Apply role catalog', + // Dialog copy, not the confirm prompt: an action that collects params and + // also sets `confirmText` shows two dialogs for one decision (#7278). The + // question is asked here, and the user's own Confirm sends it. + description: + 'Create the duties this position owes for each person selected. Runs again safely — anyone who already has a duty from a catalog item is skipped, not duplicated.', + icon: 'user-plus', + type: 'script', + target: CATALOG_APPLY_ACTION, + locations: [], + variant: 'primary', + params: [ + { + name: 'position_code', + label: 'Position', + type: 'text', + required: true, + placeholder: 'plant_compliance_officer', + helpText: + 'Matches duly_catalog_item.position_code exactly. Free text — the position does not have to be modelled in the platform yet.', + }, + { + name: 'users', + label: 'People', + type: 'user', + multiple: true, + required: true, + helpText: 'Each person gets their own copy of every active duty in this position\'s catalog.', + }, + ], +}); + +/** + * `duly_catalog_sync` — replay catalog cadence edits onto instantiated duties. + * + * Cadence only (`frequency`, `due_anchor`, `due_offset_days`, `lead_days`, + * `grace_days`). `owner`, `status`, `timezone` and the `effective_*` window are + * local decisions the catalog has no business overwriting, and a retired + * catalog item is REPORTED rather than acted on — deleting someone's duties + * because a template was deactivated is a decision for a human. + * + * `position_code` is optional here and narrows the sweep. Sync rewrites + * authored cadence, so being able to run it for one position instead of the + * whole org is the difference between a correction and an incident. + */ +export const CatalogSyncAction = defineAction({ + name: CATALOG_SYNC_ACTION, + label: 'Sync duties from catalog', + description: + 'Replay cadence changes from the role catalog onto the duties created from it. Owner, status, timezone and the effective window are left alone; duties from a deactivated catalog item are reported, never deleted.', + icon: 'refresh-cw', + type: 'script', + target: CATALOG_SYNC_ACTION, + locations: [], + params: [ + { + name: 'position_code', + label: 'Position', + type: 'text', + required: false, + placeholder: 'plant_compliance_officer', + helpText: 'Limit the sync to one position. Leave blank to sync every catalog-sourced duty.', + }, + ], +}); diff --git a/src/actions/catalog.handlers.ts b/src/actions/catalog.handlers.ts new file mode 100644 index 0000000000000000000000000000000000000000..6b120114d2ddaeda57f7b59833bbdf1abb4fe728 GIT binary patch literal 17555 zcmdU1+j1O7a`iL6qQp?_3}|3LQTDD4iiAKAl3-H=0|0kJW+-MF(=|W~J>8?eU^s#h zp(lTULlOQ1|A_xYU*dBzv-&asq_tOmAY)fRcUNas<@ID%Hnz9je!f^0$@xX)hSLrA zPblJUhtFlgxh+O{3Uv+P$x$EKWh_#?6X+?gpRdGF!>nw#$W|Zr91+={h}o z>)4+sWmQBZzMp56e_u6(M%#rv^~KhO&QXrOD@khm(W3|Mkvp!lDw%s1p1N7C`(SV7gesPvvTB4!Rl(6JCJkVIwUb|;*5ZCEyY-=7zi$%JEO5XYA$v3a}?|ZRaWz&u-Q#HI8 zS77?3Us2{Ng00k-Sh%dT5p}5+9Q1h-!~~|P{_XF*K6-t)zxS#~1%x+6jFDR=X*`XJ zc*jlrdq1r!uSzhXa44?K`S+N03v8kUCtpX4iA(eRvR+U>CZFl@p6{sAOhn{SFi1B` zQXi9+x|%3kRTre=Qz$U#*ID7=d8nLNk(){{)Vii?X)i90WBC6@f&-|3X2+tNUdkX-rkqX~2&i}U5k?SV_;J)Mt}%FSyqK&%?F%qm?|oo&h7 zE^=IygWH|wSJ)ZCMGi}xd(0$K7KKp0xw9JTR$@@;7Ev|55XNwE9p1og<4yR!3kgk7 zz&M%Bs59UTcvtvoUc}mUcEel3cP#Z++bGt?`z0>H2@oEyM7#H z(~G=luk+cg^wl_yR;Atq+v3jUIUKtW*Wbd0segcGnIcMr+*j55f|`ZNhr9{B+28A~ zYU5p2&(D1EZL5|q$-f{R31E@S{1Q>24PKBj*&H;CnmmW&Ay?IqkE_s#|pvN$Xpm+}3&PBcads-R;c$AT#8NY>g zsuf&CY#{}^)EzM_Ym5OS;K%SLUv{4$*nV~3-kuyB4^G^_9lbtqjG@#ju|GJQvZe-7 zTnJ?4svMJLli+G3aytB*Xk_+tgiWxBKgO`|1aD$B*v5Eg#}{g61qQzzs%0@ zIKs%q1q3`+rRra`iwsMkCELs6Ky1^^XAu_meT~&9$uz<+T@GjHTqcWVF;Oy(rc=Zu zfQ)%`j{VM~3_pu;0uC$++$>h(v$_PxDZ$z-spLtS*Tod0F)A!;3br7A%XaDofdqe_ zh`b=CU>XcrWK5L$ksCGw8!rk!gHYks#W7Q4&<+O9h0h#f-8S$^jc^}{3xS)A^Jz_E zfp#Of(R45#Y60Z{mZWpkX`Y4^CfOB+)7qkrA0=&5sAXQm=+F4FBOplNLE+R9w6A;` zr~qsT8Te)FB$x=)uz&)2jUnY|Lxw+5^*uWN(!D(RV(;y%Q(*@<1swzi6^p@@g-!`D zRBd_)a==XvB%HUPdKCEBfCDoi1TbhWqLjFbp=>?*LDnCL!8>yIKFs1M3@Zz~*v@fH7uKLSo#cN(;_1Y@LIIQ4*n>ol4(MqnZ+A>P57WoA0(kvpttO|Oqmu}!9OH9R};g7{*>BJ6?C zPBNsJ7C{e8GohHc&_?gv?yeh+Mo1Da5K){gqqQ5%j1-b;NK6j%EF3ep7-~m?ctIiq z6(p{w3Jt1OZzOnZf^7+WIu=X>33OZJuvyV-ovFM?79>|1lnKcNcXH-w;IrgCL<#nj zNfmAaj+{3cV^9M+m=v;_NGc$qxiK&3D1;osNQ^NWUjIxl)PgTAgV~5f6jvb1h*;(4 zo)WTT1{sQGR59WWBq1_o{+N|Z;5=&U8VE9PF@E{>^qcYN;nxQe`T&Qizb#^Ebk&>y zLCC2A#NoyccrS4O8jz{*G=boT`h=f8xIs#`?SA?BC;ZQyTto|4pm&sz`V`#W@ z+}t4wD>HFpG)`wjxo9cRU_Z!nEIA|MbR-~~ae}lV5*4Lr_A@bnVRjCw z_Htu*u`>^s>$jsB)PLltebj*%K1(VlS3q%afoF)Gpn%(gp)@`7fYhyL>MDk$-}?e! zg=Iu8J&UOF&gE}dLx&E<+-2o0z3FOMcVPGaE$!1X!H$F3p@x`#W>(%!^)|uW&ZeF} z?=V($Pm6Un4Ppum4VVbrB^&^PLpUYm9l?cYAq)5bu(7~H79j5Bq@?JS%CWDA-G!b1 zA8hyyA^f)fcdB6bsy~c4tJ9RvOU68pl2k|kH7)4fLJ}`&Q1_ZujuxYx(8oA>Db%{K4y3OMT2@!K`e{ z&ld=tI_+q(VfI5T$Z5pQHCxUX%&emHjy<=VR1;S$@Hg$%AgQowS2;#6K=%P>)hx>P z_EvJj@~;%dyKg%SETSc3u~!t)YLt}nQ^{-uF@XQ?L>}T_RD?+k52SnpHPj*H8Fg@| z0~nW4GeuP{T5ia6i2cHmgr!5P9O|ew*~`s!*`wkC>Eqo${K0*M*+)q>O>2Z?+}qed zt)(0->hgkDl;REkuP>A%lyuJ#z&Xp~u#~?|e~93Cx`ZMs)S0QJq=BkHoZ6&^RjrGQ zmEFU52<_du4^X$-bPZA+rV)%LF}KI984JOZkGi(=W5w=~CQY24hM`e`hy!l{XV%5j?OZlrOA)h?I#DLbA*c=)cW5 zHR6_}6+qP}Jf%20(=!O93V0H#q`FJXTQn2H3n5!z!2rQBInP+T427yr9n{KOqd+bc zT_ILPWT-O>O!Sg++Gb~XRZhA>rT3mH%)@tD$ouEVZV#obEUxF@y1ix*THkap-ZtBj z!ikhNSUn3%@2p$Pqbw5q^k42Z76X4*qx{Gd86V@|fvnO~1H|1cLW4|JxOTo!qguXI zq8^39+6q+_meeL|WBO+hlfbfLDQK`*h{6?4mVAt}ucqFn$7e+7EYccJCF2zgVLOF` zfM=5uHUlT6C}1C(#_w5^aQ@}+^sA${ES?4KijcWYxJZm{i-}S=n}ivqRv?nY0TwHJ zKm|Zqk(;83kNlT~Kb-I7kxUMg4p2w4HnJ>`QaCn8?E%;e#d!rlbB`jDdx%ZmR-p;^ zEAW6wpqNKe!^Zj87Tq~dA6e-(yg<$iBqI~Vj|4`yMf%Zw2Lh&>5M-WVX^a%6RH_o- zrnW>#fltmfIZddFcI%VS?^vsf!PTjUV>}Y6kSDDH^`_AyQB*z&=OSz;P0!H0A_r7z zJz94}#nH}dMf3^*p$pyP$_gGelQRDHgQ#TI-1tx{w!JW~_7+ZUh=8TX{yWy!(Ce_l zQ}W~xDT@M%%?EIXBKM;lEJ5iP0< zC8dloReDBRySLmUA%p9JhA_Mf9008J_C|m&dKuu9#p*^5YjNy$$H)f|%$iE1agh*V zUn`(r!TOQM1(mhS&+Ms`#okDU`J4(UMli&Iz=nI)RZ!JQjNn;;UXY(UT~N(^wopNI zdO~?h(8S@A!j!wCFiiaArW^FG6@w`~>N3q?Eh*^Ik|CfWKlv^e17$p7hZehk{(K%i zQa7At<4rjPf_GP2fd?jC&{SHW!~lhH)}PX>#Hi896DkCtjMd%a5_%fSA{AW%UUaQn z=&=?@ffgGnAox#dBLSW%04i_b9nEK$?#NR@K2A^aC0tR8!s2kqD^U#ovMa53IA0=c zhZ`<^M+{O+Kzd3)M7~->NNp!-WH(R~5+M-MShAsbLNG55F~K}#oHX+&^hI7T!AC=v z1#ZF#o(Z^k7^*CX^%A$ExJ_4SOg!E5{i`f-FmBjUp#iaBN7A(Zz%U zFgj2(f4ucIg^#VtwNOzUypZ=-U z_W)OkQE0PNPxSsZ2V9CqdeGyR%97oOau6eo4;tGejidZ(r~~0d1ghMHk~BJ~Q6HkF z(J)FS6b@gpP<#7j??98obtLO2_Z^AgZ7tQH?%p7H)J6CF=MpPq%(RIiz?BtH?BDJn1*vU!Qf=z`V&1s!YG%)9< z^ME%UYct*FBV}4wQ^1w(WjVq@`A|0$)~zqmpzbXnfl9k~csi>ySc4|dz_jy`8?lEC zQz(iZv1XA9W1Rj&p>0|uQUSH+ojdI?s0%8;B31s{c|c6U0#bMyuG=AP+hUlZ>0|5< z>hv;pMt1JD!<#Ng8`xa8L)y+q8`Ius2i?I&^+xLq6lt~Zp&4Y@6)8%p*Nu#&c(K!5 z(hVVSB0ITZJFEQ*X5Zdv*Mfu%u>mMuvjfB>KV3}`%T(kI4ZJbY7{w!H6+aq%EDE7T z_wdERv9<}42Yk>$4=9s!5nXC*3e{41mIQCa1@bp@aQLD`+VIx^60kDv54CBYwh4&DALjb-Ixq`c@wy%eJ3oQ7ZSkB% zj+6i_(e{E=5B>sYNMc3A;exS)(T372Poz`UC-Tg;0ea+)f()(je8;xtUgLD(x1cn@ zmuN!@XI~!0UkFkl2aaNm+}*Lf<6(nG>if}#r@zke?$dR@+K^bYrPj}eu83BG>JU{9 zKHf!@JUr91o^ID%uZj{O6deuCA!)bEA-GQ}1Dn?G`b0vhhx9-av$bqNcG2A?&PzIx z(+(w+js!RZ&%Fn>Q5|K`MK6r&ynvQA2M)R|WaiLZD1^EGUg9H}>IYF^UQ5oo@cq9g zH$TC^-#In!o&9z5>7EKGhymYUcH1Y}Sg47h09_nJNTxU;j%|`zJd$_}*K19S;({Sr zvujjeqys|hadgph^ke`Y$;mXFEX%pb7uYz2K428~EZ}1#Go}{ya-O?0WYO9Fr#~EQ z{xED+suhf5O))Jzxx)^a2}($#riwo_*s~I(xDL6k`NZmgNVl}b>~^Xs%mOx)%>>lV zEG4W(R*B4{fthP{lnqJ0yQ9|=qcbc~pnD(AqOTh~k*!D?(;zYiz_q}jrO10S=Ud^* zvSu(!Y5rQL2WCkGv3Epoozcr-yZiUVhy@@4t5&=3} z;&S}No&)otr>D%Hbz2jSY1C819G1-SDFK7oR7xo_F1~(1F-hwM>1?a%Pm%3|5_uzG5LF8Nw9IPj(tCawqaoNFgl)9#7e?|x z_7+~g+yYOOzmm!`_u?G8Bjv7ILxT;wrXUMeEu-R#*>HfVbro-#4_i9P|VGg&T&BnP9uf!1z6lm?ap7dJqcaY-wg&x+;{GQ&1^; zjRJa(kHF5UGm}8U4x*Z`z|6;z`3R`;X`BdxpAsoS_*!$wEmLL7w>E5@ULEMyI1F`x zjd!6ujDtEv1P?ZKxGlGOgUGsH&8R624hmIAmTt7_*rbWjpkwLu(GxFz12&BX|y1N+9Xdm^ea7j72JR= zD_fgxwveThM?2yi8YJ3J(i?Ce(p*XC*C1vmKL@8f*||9M;C`i}>{qjFL0)0_|D}%- z!o4;k9seqn$cp{`cXf@tR6R1DmujW?oxJjGfaLld=f?PCh(pxU*9K89j^rbDwEYaj zV0^&9V<^am)a~k4%@F;){I=03Vg`;5vnwOoXV>EQc5tdI>%sSAXnz&*wsqrrXF>SJ zf|VUiv2kbj4o(c40M*T@?v!Tla8P62?Np8Gq!a2kLQHI_TQvTCH4&f2=~ud%rgQ+s zT7sehUAnOE%TGEjsG1}6+}mC0iTNfSZ9BG$z+Szyz3YWJK3<3bam|LgyHk(9=vVO{ zM85Hxi)?yb^ba~9tV?Hjh`iRq$@njU+s>nLCeLH}eYS<-3o2j$e41r{$lxVDv)_|E z)6N&=j{&sL+rJ5cKNDbW4J0<{e5pEu*3Xi2`$Ga(Ih&ny7XB#ij literal 0 HcmV?d00001 diff --git a/src/actions/index.ts b/src/actions/index.ts index b560efe..f36c0af 100644 --- a/src/actions/index.ts +++ b/src/actions/index.ts @@ -13,4 +13,8 @@ // makes `name` optional and fails the assignment. A named array is `never[]` // while empty and infers correctly the moment something is pushed into it. -export const dulyActions = []; +import { CatalogApplyAction, CatalogSyncAction } from './catalog.actions.js'; + +export { CatalogApplyAction, CatalogSyncAction }; + +export const dulyActions = [CatalogApplyAction, CatalogSyncAction]; diff --git a/src/actions/register-handlers.ts b/src/actions/register-handlers.ts index c5ec2f7..21ace8f 100644 --- a/src/actions/register-handlers.ts +++ b/src/actions/register-handlers.ts @@ -1,5 +1,7 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +import { registerCatalogActionHandlers } from './catalog.handlers.js'; + /** * Action handler registration. * @@ -16,7 +18,8 @@ export interface HandlerRegistrationContext { registerAction: (...args: unknown[]) => void; } -export function registerDulyActionHandlers(_ql: HandlerRegistrationContext): void { +export function registerDulyActionHandlers(ql: HandlerRegistrationContext): void { // Register handlers here, one call per action: // registerTaskActionHandlers(ql); + registerCatalogActionHandlers(ql); } diff --git a/test/catalog-instantiate.test.ts b/test/catalog-instantiate.test.ts new file mode 100644 index 0000000..38f5cb0 --- /dev/null +++ b/test/catalog-instantiate.test.ts @@ -0,0 +1,589 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { beforeEach, describe, expect, it } from 'vitest'; + +import type { ActionEngineFacade, ActionHandlerContext } from '@objectstack/spec/ui'; + +import { Duty } from '../src/objects/index.js'; +import { dulyActions } from '../src/actions/index.js'; +import { registerDulyActionHandlers } from '../src/actions/register-handlers.js'; +import type { HandlerRegistrationContext } from '../src/actions/register-handlers.js'; +import { + CADENCE_FIELDS, + CATALOG_APPLY_ACTION, + CATALOG_SYNC_ACTION, + DEFAULT_DUTY_TIMEZONE, + GLOBAL_ACTION_OBJECT, + applyCatalogHandler, + resolveDutyTimezone, + syncCatalogHandler, +} from '../src/actions/catalog.handlers.js'; +import type { CatalogApplyResult, CatalogSyncResult } from '../src/actions/catalog.handlers.js'; + +// ─── A fake engine ────────────────────────────────────────────────────────── +// +// `ActionEngineFacade` is four methods, so the handlers can be driven directly +// without a kernel. The fake HONOURS `where` (equality plus `$in`) rather than +// returning everything: a fake that ignored filters would make every test pass +// for the wrong reason, and would hide a handler that forgot to narrow its +// read. The one test that needs an unfiltered read builds its own lenient +// engine, deliberately — see "the source guard is in the code". + +interface Row extends Record { + id: string; +} + +function matches(row: Row, where: Record | undefined): boolean { + if (!where) return true; + for (const [field, expected] of Object.entries(where)) { + const actual = row[field]; + if (expected !== null && typeof expected === 'object' && '$in' in (expected as object)) { + const set = (expected as { $in: unknown[] }).$in; + if (!Array.isArray(set) || !set.includes(actual)) return false; + continue; + } + if (actual !== expected) return false; + } + return true; +} + +class FakeEngine implements ActionEngineFacade { + readonly tables = new Map(); + readonly inserts: Array<{ object: string; data: Record }> = []; + readonly updates: Array<{ object: string; id: string; data: Record }> = []; + readonly deletes: Array<{ object: string; id: string }> = []; + private seq = 0; + + seed(object: string, rows: Array>): void { + const table = this.tables.get(object) ?? []; + for (const row of rows) table.push({ ...row, id: String(row.id ?? `${object}_${++this.seq}`) }); + this.tables.set(object, table); + } + + async insert(object: string, data: Record): Promise<{ id: string }> { + const id = `${object}_${++this.seq}`; + const table = this.tables.get(object) ?? []; + table.push({ ...data, id }); + this.tables.set(object, table); + this.inserts.push({ object, data }); + return { id }; + } + + async update(object: string, id: string, data: Record): Promise { + const row = (this.tables.get(object) ?? []).find((r) => r.id === id); + if (!row) throw new Error(`update: no ${object} row ${id}`); + Object.assign(row, data); + this.updates.push({ object, id, data }); + } + + async delete(object: string, id: string): Promise { + this.deletes.push({ object, id }); + } + + async find(object: string, query: Record): Promise>> { + const where = query?.where as Record | undefined; + return (this.tables.get(object) ?? []).filter((row) => matches(row, where)).map((row) => ({ ...row })); + } + + rows(object: string): Row[] { + return this.tables.get(object) ?? []; + } +} + +function contextFor(engine: ActionEngineFacade, params: Record): ActionHandlerContext { + return { + record: {}, + params, + user: { id: 'admin_1', organizationId: 'org_1' }, + session: { userId: 'admin_1', organizationId: 'org_1' }, + engine, + }; +} + +/** A 26-item catalog, the size the issue uses as the adoption bar. */ +function seedCatalog(engine: FakeEngine, positionCode: string, count = 26, idPrefix = 'item'): void { + engine.seed( + 'duly_catalog_item', + Array.from({ length: count }, (_, i) => ({ + id: `${idPrefix}_${i + 1}`, + name: `Duty ${i + 1}`, + description: `What done means for duty ${i + 1}`, + position_code: positionCode, + form: 'recurring', + frequency: 'monthly', + due_anchor: 'period_start', + due_offset_days: 5, + lead_days: 7, + grace_days: 0, + active: true, + })), + ); +} + +const THREE_USERS = ['user_a', 'user_b', 'user_c']; + +describe('duly_catalog_apply', () => { + let engine: FakeEngine; + + beforeEach(() => { + engine = new FakeEngine(); + seedCatalog(engine, 'plant_compliance_officer'); + }); + + it('applying a 26-item catalog to 3 users creates 78 duties, all catalog-sourced', async () => { + const result = (await applyCatalogHandler( + contextFor(engine, { position_code: 'plant_compliance_officer', users: THREE_USERS }), + )) as CatalogApplyResult; + + expect(result.created).toBe(78); + expect(result.skipped).toBe(0); + expect(result.catalog_items).toBe(26); + expect(result.users).toBe(3); + + const duties = engine.rows('duly_duty'); + expect(duties).toHaveLength(78); + for (const duty of duties) { + expect(duty.source).toBe('catalog'); + expect(typeof duty.catalog_item).toBe('string'); + expect(duty.catalog_item).toBeTruthy(); + expect(duty.status).toBe('active'); + } + + // Every (item, owner) pair exactly once — 78 rows could still be 26 + // duplicates of three. + const pairs = new Set(duties.map((d) => `${String(d.catalog_item)} ${String(d.owner)}`)); + expect(pairs.size).toBe(78); + }); + + it('copies the catalog item\'s content and cadence onto the duty', async () => { + await applyCatalogHandler( + contextFor(engine, { position_code: 'plant_compliance_officer', users: ['user_a'] }), + ); + + const duty = engine.rows('duly_duty').find((d) => d.catalog_item === 'item_1'); + expect(duty).toBeDefined(); + expect(duty).toMatchObject({ + name: 'Duty 1', + description: 'What done means for duty 1', + form: 'recurring', + frequency: 'monthly', + due_anchor: 'period_start', + due_offset_days: 5, + lead_days: 7, + grace_days: 0, + owner: 'user_a', + source: 'catalog', + status: 'active', + }); + }); + + it('applying the same input again creates 0 and reports 78 skipped', async () => { + const first = (await applyCatalogHandler( + contextFor(engine, { position_code: 'plant_compliance_officer', users: THREE_USERS }), + )) as CatalogApplyResult; + expect(first.created).toBe(78); + + const insertsAfterFirst = engine.inserts.length; + + const second = (await applyCatalogHandler( + contextFor(engine, { position_code: 'plant_compliance_officer', users: THREE_USERS }), + )) as CatalogApplyResult; + + expect(second.created).toBe(0); + expect(second.skipped).toBe(78); + // Counting the report is not enough — assert nothing was written. + expect(engine.inserts.length).toBe(insertsAfterFirst); + expect(engine.rows('duly_duty')).toHaveLength(78); + expect(second.entries.every((e) => e.outcome === 'skipped')).toBe(true); + }); + + it('adding a person to an already-applied position creates only their duties', async () => { + await applyCatalogHandler( + contextFor(engine, { position_code: 'plant_compliance_officer', users: ['user_a', 'user_b'] }), + ); + + const result = (await applyCatalogHandler( + contextFor(engine, { position_code: 'plant_compliance_officer', users: THREE_USERS }), + )) as CatalogApplyResult; + + expect(result.created).toBe(26); + expect(result.skipped).toBe(52); + expect(result.entries.filter((e) => e.outcome === 'created').every((e) => e.owner === 'user_c')).toBe(true); + }); + + it('instantiates only ACTIVE items, and only for the requested position', async () => { + engine.seed('duly_catalog_item', [ + { id: 'item_off', name: 'Retired duty', position_code: 'plant_compliance_officer', form: 'recurring', active: false }, + { id: 'item_other', name: 'Someone else\'s duty', position_code: 'shift_supervisor', form: 'recurring', active: true }, + ]); + + const result = (await applyCatalogHandler( + contextFor(engine, { position_code: 'plant_compliance_officer', users: ['user_a'] }), + )) as CatalogApplyResult; + + expect(result.created).toBe(26); + const items = engine.rows('duly_duty').map((d) => d.catalog_item); + expect(items).not.toContain('item_off'); + expect(items).not.toContain('item_other'); + }); + + it('anchors business_unit on sys_user_position.business_unit_id', async () => { + engine.seed('sys_user_position', [ + { id: 'up_1', user_id: 'user_a', position: 'plant_compliance_officer', business_unit_id: 'bu_north' }, + ]); + + await applyCatalogHandler( + contextFor(engine, { position_code: 'plant_compliance_officer', users: ['user_a'] }), + ); + + for (const duty of engine.rows('duly_duty')) expect(duty.business_unit).toBe('bu_north'); + }); + + it('does NOT require a sys_user_position row — day one, before positions are modelled', async () => { + const result = (await applyCatalogHandler( + contextFor(engine, { position_code: 'plant_compliance_officer', users: ['user_a'] }), + )) as CatalogApplyResult; + + expect(result.created).toBe(26); + // Absent, not null: an unanchored duty must not claim a business unit. + for (const duty of engine.rows('duly_duty')) expect(duty.business_unit).toBeUndefined(); + }); + + it('ignores an unanchored position row rather than writing a null business unit', async () => { + engine.seed('sys_user_position', [ + { id: 'up_1', user_id: 'user_a', position: 'plant_compliance_officer', business_unit_id: null }, + ]); + + await applyCatalogHandler( + contextFor(engine, { position_code: 'plant_compliance_officer', users: ['user_a'] }), + ); + + for (const duty of engine.rows('duly_duty')) expect(duty.business_unit).toBeUndefined(); + }); + + it('refuses a blank position_code or an empty user list instead of reporting a no-op run', async () => { + await expect( + applyCatalogHandler(contextFor(engine, { position_code: ' ', users: THREE_USERS })), + ).rejects.toThrow(/position_code/); + + await expect( + applyCatalogHandler(contextFor(engine, { position_code: 'plant_compliance_officer', users: [] })), + ).rejects.toThrow(/users/); + + expect(engine.inserts).toHaveLength(0); + }); + + it('de-duplicates a repeated user id in one call', async () => { + const result = (await applyCatalogHandler( + contextFor(engine, { position_code: 'plant_compliance_officer', users: ['user_a', 'user_a'] }), + )) as CatalogApplyResult; + + expect(result.users).toBe(1); + expect(result.created).toBe(26); + }); +}); + +describe('duly_catalog_sync', () => { + let engine: FakeEngine; + + async function applyThenEdit(edit: Record): Promise { + await applyCatalogHandler( + contextFor(engine, { position_code: 'plant_compliance_officer', users: THREE_USERS }), + ); + const item = engine.rows('duly_catalog_item').find((i) => i.id === 'item_1'); + Object.assign(item as Row, edit); + } + + beforeEach(() => { + engine = new FakeEngine(); + seedCatalog(engine, 'plant_compliance_officer'); + }); + + it('replays an edited due_offset_days onto every derived duty', async () => { + await applyThenEdit({ due_offset_days: 12 }); + + const result = (await syncCatalogHandler(contextFor(engine, {}))) as CatalogSyncResult; + + expect(result.updated).toBe(3); + expect(result.unchanged).toBe(75); + expect(result.scanned).toBe(78); + + const derived = engine.rows('duly_duty').filter((d) => d.catalog_item === 'item_1'); + expect(derived).toHaveLength(3); + for (const duty of derived) expect(duty.due_offset_days).toBe(12); + }); + + it('leaves owner, status, timezone and the effective window untouched', async () => { + await applyThenEdit({ due_offset_days: 12, frequency: 'quarterly' }); + + // Local decisions made after instantiation: a paused duty, a moved window. + for (const duty of engine.rows('duly_duty')) { + if (duty.catalog_item !== 'item_1') continue; + duty.status = 'paused'; + duty.timezone = 'Europe/Berlin'; + duty.effective_from = '2026-01-01'; + duty.effective_to = '2026-12-31'; + } + + await syncCatalogHandler(contextFor(engine, {})); + + for (const duty of engine.rows('duly_duty')) { + if (duty.catalog_item !== 'item_1') continue; + expect(duty.due_offset_days).toBe(12); + expect(duty.frequency).toBe('quarterly'); + // untouched + expect(duty.status).toBe('paused'); + expect(duty.timezone).toBe('Europe/Berlin'); + expect(duty.effective_from).toBe('2026-01-01'); + expect(duty.effective_to).toBe('2026-12-31'); + expect(THREE_USERS).toContain(duty.owner); + } + + // The patch itself must never name a non-cadence key — asserting the + // written record is not enough, since a patch could rewrite a field to the + // value it already had and look untouched. + for (const update of engine.updates) { + expect(Object.keys(update.data).sort()).toEqual( + Object.keys(update.data).filter((k) => (CADENCE_FIELDS as readonly string[]).includes(k)).sort(), + ); + } + }); + + it('writes every cadence field the catalog owns, and only those', async () => { + await applyThenEdit({ + frequency: 'weekly', + due_anchor: 'period_end', + due_offset_days: -3, + lead_days: 2, + grace_days: 4, + }); + + const result = (await syncCatalogHandler(contextFor(engine, {}))) as CatalogSyncResult; + + expect(result.updated).toBe(3); + for (const change of result.changes) { + expect(Object.keys(change.fields).sort()).toEqual([...CADENCE_FIELDS].sort()); + expect(change.fields.due_offset_days).toEqual({ from: 5, to: -3 }); + } + for (const duty of engine.rows('duly_duty')) { + if (duty.catalog_item !== 'item_1') continue; + expect(duty).toMatchObject({ + frequency: 'weekly', + due_anchor: 'period_end', + due_offset_days: -3, + lead_days: 2, + grace_days: 4, + }); + } + }); + + it('reports each change with a legible from/to — sync is destructive to authored cadence', async () => { + await applyThenEdit({ due_offset_days: 12 }); + + const result = (await syncCatalogHandler(contextFor(engine, {}))) as CatalogSyncResult; + + expect(result.changes).toHaveLength(3); + for (const change of result.changes) { + expect(change.catalog_item).toBe('item_1'); + expect(change.catalog_item_name).toBe('Duty 1'); + expect(THREE_USERS).toContain(change.owner); + expect(change.fields.due_offset_days).toEqual({ from: 5, to: 12 }); + } + }); + + it('a self-declared duty is never touched, even with catalog_item set', async () => { + await applyThenEdit({ due_offset_days: 12 }); + + engine.seed('duly_duty', [ + { + id: 'duty_self', + name: 'My own note to self', + owner: 'user_a', + source: 'self', + catalog_item: 'item_1', + due_offset_days: 0, + frequency: 'daily', + status: 'active', + }, + ]); + + const result = (await syncCatalogHandler(contextFor(engine, {}))) as CatalogSyncResult; + + expect(result.updated).toBe(3); + expect(engine.updates.some((u) => u.id === 'duty_self')).toBe(false); + const self = engine.rows('duly_duty').find((d) => d.id === 'duty_self'); + expect(self?.due_offset_days).toBe(0); + expect(self?.frequency).toBe('daily'); + }); + + it('the source guard is in the code, not only in the query filter', async () => { + // A driver that ignores `where` must not be able to make a self-declared + // duty catalog-writable. The product invariant lives in the handler. + await applyThenEdit({ due_offset_days: 12 }); + engine.seed('duly_duty', [ + { id: 'duty_self', owner: 'user_a', source: 'self', catalog_item: 'item_1', due_offset_days: 0 }, + ]); + + const lenient: ActionEngineFacade = { + insert: (o, d) => engine.insert(o, d), + update: (o, i, d) => engine.update(o, i, d), + delete: (o, i) => engine.delete(o, i), + // Deliberately filter-blind: returns every row whatever the query says. + find: async (o) => engine.rows(o).map((r) => ({ ...r })), + }; + + const result = (await syncCatalogHandler(contextFor(lenient, {}))) as CatalogSyncResult; + + expect(engine.updates.some((u) => u.id === 'duty_self')).toBe(false); + expect(result.changes.some((c) => c.duty === 'duty_self')).toBe(false); + }); + + it('deactivating a catalog item and syncing reports it and changes nothing', async () => { + await applyThenEdit({ active: false, due_offset_days: 12 }); + + const before = engine.rows('duly_duty').map((d) => ({ ...d })); + const result = (await syncCatalogHandler(contextFor(engine, {}))) as CatalogSyncResult; + + expect(result.retired).toHaveLength(3); + expect(result.updated).toBe(0); + for (const entry of result.retired) { + expect(entry.catalog_item).toBe('item_1'); + expect(entry.catalog_item_name).toBe('Duty 1'); + expect(THREE_USERS).toContain(entry.owner); + } + + // Reported, never deleted, never edited. + expect(engine.deletes).toHaveLength(0); + expect(engine.updates).toHaveLength(0); + expect(engine.rows('duly_duty')).toEqual(before); + }); + + it('narrowing by position_code leaves other positions alone', async () => { + await applyThenEdit({ due_offset_days: 12 }); + + seedCatalog(engine, 'shift_supervisor', 2, 'sup'); + const other = engine.rows('duly_catalog_item').filter((i) => i.position_code === 'shift_supervisor'); + await applyCatalogHandler(contextFor(engine, { position_code: 'shift_supervisor', users: ['user_d'] })); + Object.assign(other[0] as Row, { due_offset_days: 99 }); + + const result = (await syncCatalogHandler( + contextFor(engine, { position_code: 'plant_compliance_officer' }), + )) as CatalogSyncResult; + + expect(result.position_code).toBe('plant_compliance_officer'); + expect(result.updated).toBe(3); + // The other position was neither updated nor reported as retired. + const otherDuty = engine.rows('duly_duty').find((d) => d.catalog_item === other[0].id); + expect(otherDuty?.due_offset_days).toBe(5); + expect(result.retired).toHaveLength(0); + expect(result.changes.every((c) => c.catalog_item === 'item_1')).toBe(true); + }); + + it('is idempotent — a second sync with no catalog edit writes nothing', async () => { + await applyThenEdit({ due_offset_days: 12 }); + await syncCatalogHandler(contextFor(engine, {})); + const writesAfterFirst = engine.updates.length; + + const second = (await syncCatalogHandler(contextFor(engine, {}))) as CatalogSyncResult; + + expect(second.updated).toBe(0); + expect(second.unchanged).toBe(78); + expect(engine.updates.length).toBe(writesAfterFirst); + }); +}); + +describe('handler wiring', () => { + // The failure mode with no author-time gate: an action whose handler is not + // registered renders, is clickable, and 404s at call time. `pnpm validate` + // checks the declaration and knows nothing about the registry, so the + // declaration↔handler bijection is asserted here. + + function registered(): Array<{ object: string; action: string; handler: unknown }> { + const calls: Array<{ object: string; action: string; handler: unknown }> = []; + const ql: HandlerRegistrationContext = { + registerAction: (...args: unknown[]) => { + calls.push({ object: String(args[0]), action: String(args[1]), handler: args[2] }); + }, + }; + registerDulyActionHandlers(ql); + return calls; + } + + it('every declared action has a registered handler', () => { + const calls = registered(); + const declared = dulyActions.map((a) => a.name).sort(); + const wired = calls.map((c) => c.action).sort(); + + expect(declared).toEqual([CATALOG_APPLY_ACTION, CATALOG_SYNC_ACTION].sort()); + expect(wired).toEqual(declared); + }); + + it('object-less actions register under the canonical "global" key', () => { + // `executeAction` is an exact-string Map lookup on `:` — a + // handler filed under any other key is unreachable, however the action is + // declared. + for (const call of registered()) { + expect(call.object).toBe(GLOBAL_ACTION_OBJECT); + expect(typeof call.handler).toBe('function'); + } + }); + + it('the declared actions are object-less and headless, matching that key', () => { + for (const action of dulyActions) { + expect(action.objectName).toBeUndefined(); + // `global_nav` was retired in protocol 17 and every surviving location is + // object-bound, so `locations: []` is the only honest declaration here. + expect(action.locations).toEqual([]); + } + }); + + it('each script action names a target, so it cannot 404 for want of a binding', () => { + for (const action of dulyActions) { + expect(action.type).toBe('script'); + expect(action.target).toBe(action.name); + } + }); + + it('duly_catalog_apply declares the position_code + multi-user input the flow needs', () => { + const apply = dulyActions.find((a) => a.name === CATALOG_APPLY_ACTION); + const params = apply?.params ?? []; + + const position = params.find((p) => p.name === 'position_code'); + expect(position?.type).toBe('text'); + expect(position?.required).toBe(true); + + const users = params.find((p) => p.name === 'users'); + expect(users?.type).toBe('user'); + expect(users?.multiple).toBe(true); + expect(users?.required).toBe(true); + }); + + it('duly_catalog_sync scopes by an OPTIONAL position_code', () => { + const sync = dulyActions.find((a) => a.name === CATALOG_SYNC_ACTION); + const position = (sync?.params ?? []).find((p) => p.name === 'position_code'); + expect(position?.type).toBe('text'); + expect(position?.required).toBe(false); + }); +}); + +describe('duty timezone resolution', () => { + it('falls back to UTC — sys_user carries no zone and the org default is not in the action context', () => { + expect(resolveDutyTimezone()).toBe(DEFAULT_DUTY_TIMEZONE); + expect(DEFAULT_DUTY_TIMEZONE).toBe('UTC'); + }); + + it('agrees with duly_duty.timezone\'s own declared default', () => { + // Two answers to one question is the drift this pins: if the field default + // moves, the instantiated duties must move with it. + expect(Duty.fields.timezone.defaultValue).toBe(DEFAULT_DUTY_TIMEZONE); + }); + + it('stamps the resolved zone on every instantiated duty', async () => { + const engine = new FakeEngine(); + seedCatalog(engine, 'plant_compliance_officer', 2); + await applyCatalogHandler( + contextFor(engine, { position_code: 'plant_compliance_officer', users: ['user_a'] }), + ); + for (const duty of engine.rows('duly_duty')) expect(duty.timezone).toBe(DEFAULT_DUTY_TIMEZONE); + }); +}); From dd4fb3b33c38ef2072e9f645690ef9b7ede370f4 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 04:06:42 +0000 Subject: [PATCH 2/2] Write pairKey's separator as an escape, not a raw NUL byte The separator itself is right: a NUL cannot occur in a record id, so the composite key cannot collide the way plain concatenation can - ('ab','c') and ('a','bc') would otherwise produce the same key and silently skip a duty that was never created. Encoding it as a literal 0x00 in the source was the defect. It made git treat catalog.handlers.ts as binary (Bin 0 -> 17555 bytes, no diff, no blame, no review for the life of the file), left the separator invisible in an editor, and would be dropped silently on copy-paste - degrading the key back to plain concatenation with no error. Now written as the \u0000 escape, so the file stays ASCII while the runtime value is unchanged. pairKey is exported and the collision property is pinned in tests, since the reason for the separator is not obvious and would otherwise be "simplified" away. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01SqkTcrxUFci7nqXdbBSe2p --- src/actions/catalog.handlers.ts | Bin 17555 -> 18228 bytes test/catalog-instantiate.test.ts | 25 +++++++++++++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/src/actions/catalog.handlers.ts b/src/actions/catalog.handlers.ts index 6b120114d2ddaeda57f7b59833bbdf1abb4fe728..74e296b0364656e12c1aa8e50c4e723dbb627d73 100644 GIT binary patch delta 707 zcmZ9K&x+JQ5XM;#9_Af(DHdc@;;d(Hi|oxm3o2d~q|;rQl+)7{syh>NidQdQ#`jRr zCla4Stx6QckVAJm_3Q8Zs*WE{zI{CT^6T{03XVTdpH3^duP?l85jrO&gA;_eObG)F zR9bNG?A4PBhL(H_-lfC^*!y09k{$|rmH-qsa2*{bgLzKg6-=%WCm6VoLBGXF0UyEL zwX4_F+OO|{OJTCTZfo0j!lCyFMa5*dM`Pt8l(rRQhB7b&Y8n(wMb(nzKk(d|YEB3{ z91Xi5RGKbBUq&M>I%-8zR6rad?`Y~eHR{xM6m(&P;&zxe`oERD7Aoq)&P~%z|4<*e zdhz)3`l^D22b8%M#0^l71vhXBkqU~77Mn$+g|iRtMXl`>pJ7V{ir!aXGVaiFL}*l& z(P~OgM)>~z6QsqkUd2#HO~h746Fd+O6*MffvPg~kZ|3#Pv1-bC|A8{q3btk?z(9qi znj_NVAQM{vv%tTp)FbRUt3c+c(QB;Qyl#BZxm;AhG6+kU0k`6UHB4c(^E)jvR%es& w-xyd|1Syq&n(X;pry|c&VzZ_;QG~_vzgXeBj-|ly`PrM>OY~nqpZ&P=8~Q=<#{d8T delta 29 lcmdne$2hr@al?Pv$xL!SEDBosT$5ib#4|E%4p4le3ILsC3Eltz diff --git a/test/catalog-instantiate.test.ts b/test/catalog-instantiate.test.ts index 38f5cb0..950d721 100644 --- a/test/catalog-instantiate.test.ts +++ b/test/catalog-instantiate.test.ts @@ -15,6 +15,7 @@ import { DEFAULT_DUTY_TIMEZONE, GLOBAL_ACTION_OBJECT, applyCatalogHandler, + pairKey, resolveDutyTimezone, syncCatalogHandler, } from '../src/actions/catalog.handlers.js'; @@ -492,6 +493,30 @@ describe('duly_catalog_sync', () => { }); }); +describe('pairKey — the idempotency key', () => { + // The separator is deliberate and the reason is not obvious, so it is pinned + // rather than left to be "simplified" away. + + it('does not collide across a shifted boundary', () => { + // Plain concatenation makes these the same string, which would make apply + // skip a duty it has never created. + expect(pairKey('ab', 'c')).not.toBe(pairKey('a', 'bc')); + expect(pairKey('item', '1_user')).not.toBe(pairKey('item_1', 'user')); + }); + + it('is stable and distinguishes each component', () => { + expect(pairKey('item_1', 'user_a')).toBe(pairKey('item_1', 'user_a')); + expect(pairKey('item_1', 'user_a')).not.toBe(pairKey('item_1', 'user_b')); + expect(pairKey('item_1', 'user_a')).not.toBe(pairKey('item_2', 'user_a')); + }); + + it('separates with an escaped NUL, which no record id can contain', () => { + // Asserted via the escape, never a raw byte in this file either. + expect(pairKey('a', 'b')).toBe('a' + '\u0000' + 'b'); + expect(pairKey('a', 'b')).toHaveLength(3); + }); +}); + describe('handler wiring', () => { // The failure mode with no author-time gate: an action whose handler is not // registered renders, is clickable, and 404s at call time. `pnpm validate`