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
55 changes: 50 additions & 5 deletions src/actions/catalog.handlers.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -110,6 +110,41 @@ export function resolveDutyTimezone(): string {
return DEFAULT_DUTY_TIMEZONE;
}

// ── The engine facade's query shape ───────────────────────────────────
//
// `ctx.engine.find(object, query)` takes a BARE FILTER, not an ObjectQL query
// envelope. The runtime builds the envelope itself — `buildActionEngineFacade`
// in @objectstack/runtime 17.2.0, read verbatim from its `dist/index.js`:
//
// async find(object, query) {
// const where = query && Object.keys(query).length ? { where: query } : {};
// const rows = await ql.find(object, { ...where, context });
//
// So every read in this file passes `{ field: value }`, never
// `{ where: { field: value } }`. Handing it an envelope produces
// `{ where: { where: { … } } }`; no row has a field called `where`, so the read
// comes back EMPTY WITH NO ERROR. That is the failure this file shipped with:
// `duly_catalog_apply` reported a successful run of zero, `duly_catalog_sync`
// scanned nothing and called every duty unchanged, and `resolveBusinessUnit`
// anchored no duty at all — silently, because "no position row" is a legitimate
// day-one state. The ONE unfiltered read survived, because
// `Object.keys({}).length === 0` skips the wrapping entirely, which is exactly
// what made the handler look partially alive.
//
// `ActionEngineFacade.find` in @objectstack/spec types `query` as a plain
// record of string to unknown and says nothing about which of the two shapes it
// is — the runtime's implementation is the only thing that decides, and this
// app read it the other way. Filed upstream as
// **objectstack-ai/objectstack#14175** so the shape is DECLARED rather than
// discovered; until that lands this comment is the contract.
//
// ⛔ Do NOT add a tolerant `query.where ?? query` rung — not here, not in a
// test double. A consumer that accepts both shapes is precisely what let the
// wrong one ship green: `test/catalog-instantiate.test.ts`'s fake honoured the
// envelope, so 78 assertions passed against a shape production never produces.
// The test that can see this is one that dispatches through the REAL route and
// lets the runtime build its own facade — `test/catalog-engine-facade.test.ts`.

// ── Shapes ──────────────────────────────────────────────────────────────────

export interface CatalogApplyParams extends Record<string, unknown> {
Expand DownExpand Up@@ -256,12 +291,21 @@ export function pairKey(catalogItem: unknown, owner: unknown): string {
* exists on the platform. It is NOT read here — the issue names the
* assignment-level anchor, and adding a fallback rung is a product decision,
* reported rather than taken.)
*
* ⚠️ That tolerance is why this read's query shape matters more than the other
* three. The other reads fail into a visibly empty report — zero items, zero
* scanned — but this one fails into a state the handler is WRITTEN to accept:
* an envelope-shaped filter returned nothing, "nothing" reads as "not yet
* modelled", and every duty was created unanchored with no error anywhere. The
* rollups that the business unit exists to feed were simply empty. See the
* facade-shape note above; the end-to-end coverage is in
* `test/catalog-engine-facade.test.ts`.
*/
async function resolveBusinessUnit(
engine: ActionEngineFacade,
userId: string,
): Promise<string | undefined> {
const rows = await engine.find('sys_user_position', { where: { user_id: userId } });
const rows = await engine.find('sys_user_position', { user_id: userId });
for (const row of rows) {
// A person can hold several positions; take the first anchored one.
// Unanchored rows (`null`) are legacy/tenant-wide and carry no depth.
Expand All@@ -282,13 +326,14 @@ export const applyCatalogHandler: ActionHandler<CatalogApplyParams> = async (ctx
// stopped asking for; handing it to a new hire on their first day is the
// opposite of what deactivating it meant.
const items = await engine.find('duly_catalog_item', {
where: { position_code: positionCode, active: true },
position_code: positionCode,
active: true,
});
const activeItems = items.filter((item) => item?.active !== false);

// One probe for the whole run, not one per (item, user). The pair set is
// what makes a second apply create nothing.
const existing = await engine.find('duly_duty', { where: { owner: { $in: users } } });
const existing = await engine.find('duly_duty', { owner: { $in: users } });
const taken = new Set<string>();
for (const duty of existing) {
// Any duty already pointing at this catalog item for this person counts —
Expand DownExpand Up@@ -384,15 +429,15 @@ export const syncCatalogHandler: ActionHandler<CatalogSyncParams> = async (ctx)
// the retired report is made of, so it has to come back from this read.
const items = await engine.find(
'duly_catalog_item',
positionCode ? { where: { position_code: positionCode } } : {},
positionCode ? { position_code: positionCode } : {},
);
const byId = new Map<string, Record<string, unknown>>();
for (const item of items) {
if (positionCode && text(item?.position_code) !== positionCode) continue;
byId.set(recordId(item), item);
}

const duties = await engine.find('duly_duty', { where: { source: 'catalog' } });
const duties = await engine.find('duly_duty', { source: 'catalog' });

const changes: CatalogSyncChange[] = [];
const retired: CatalogSyncRetired[] = [];
Expand Down
90 changes: 22 additions & 68 deletions test/catalog-apply-cadence.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -23,7 +23,7 @@ import type { CatalogApplyResult } from '../src/actions/catalog.handlers.js';
* apply path unprotected is half a fix.
*
* ── Why a REAL booted engine and not `catalog-instantiate.test.ts`'s fake ──
* That suite's `FakeEngine` is a Map with a `where` matcher: it runs no
* That suite's `FakeEngine` is a Map with a filter matcher: it runs no
* validation rules and stamps no defaults, so every claim below would pass on
* it for the wrong reason. Validation and `applyFieldDefaults` are precisely
* what is under test here, so the handler is dispatched through the app's own
Expand DownExpand Up@@ -60,7 +60,7 @@ afterAll(async () => {
await kernel?.shutdown?.();
});

// ── The two engine facades ──────────────────────────────────────────────────
// ── The engine facade ───────────────────────────────────────────────────────

interface Facade {
insert(object: string, values: AnyRow): Promise<{ id: string }>;
Expand All@@ -70,15 +70,29 @@ interface Facade {
}

/**
* The facade `applyCatalogHandler` is WRITTEN against: `find(object, query)`
* takes ObjectQL's own query envelope, `where` and all. It is the convention
* `catalog-instantiate.test.ts`'s `FakeEngine` honours too, so this is the
* shape every existing assertion about the handler is made under.
* The facade the RUNTIME builds, reproduced line for line —
* `buildActionEngineFacade` in @objectstack/runtime 17.2.0:
*
* async find(object, query) {
* const where = query && Object.keys(query).length ? { where: query } : {};
* const rows = await ql.find(object, { ...where, context });
*
* `find(object, query)` therefore takes a BARE FILTER and wraps it here. This
* file used to carry TWO facades — this one honouring the handler's own
* `where` envelope, and a second reproducing the runtime — with a tripwire
* pinning the gap between them (#79). The handler now passes flat filters, so
* there is one convention and one facade.
*
* It is still a double, which is all it can be: the second test below has to
* put a row in front of the handler that `duly_catalog_item`'s own rules
* refuse, and only a hand-supplied facade can do that. What a double cannot do
* is prove the wire shape is right — it encodes the author's belief about it.
* That proof lives in `test/catalog-engine-facade.test.ts`, which dispatches
* through the real action route and lets the runtime build its own facade.
*
* `catalogItems`, when given, replaces the catalog read with rows handed
* straight to the handler (unfiltered — the handler re-applies its own
* `active` filter). That is how a row the object's own rules now REFUSE can
* still be put in front of the handler, which one test below needs.
* `active` filter).
*/
function handlerFacade(catalogItems?: AnyRow[]): Facade {
return {
Expand All@@ -94,28 +108,6 @@ function handlerFacade(catalogItems?: AnyRow[]): Facade {
},
find: async (object, query) => {
if (catalogItems && object === 'duly_catalog_item') return catalogItems.map((r) => ({ ...r }));
return data.find(object, query);
},
};
}

/**
* The facade the RUNTIME actually builds — `buildActionEngineFacade` in
* @objectstack/runtime 17.2.0, reproduced line for line:
*
* async find(object, query) {
* const where = query && Object.keys(query).length ? { where: query } : {};
* const rows = await ql.find(object, { ...where, context });
* ...
*
* It wraps whatever it is handed in a `where` of its own. Used by exactly one
* test, the tripwire at the bottom.
*/
function runtimeFacade(): Facade {
const base = handlerFacade();
return {
...base,
find: async (object, query) => {
const where = query && Object.keys(query).length ? { where: query } : {};
return data.find(object, { ...where });
},
Expand DownExpand Up@@ -227,41 +219,3 @@ describe('duly_catalog_apply — the cadence it replicates (#65)', () => {
for (const field of CADENCE_FIELDS) expect(duty?.[field] ?? null, field).toBeNull();
});
});

// ───────────────────────────────────────────────────────────────────────────
// A tripwire on a filed defect — NOT an assertion that this is correct
// ───────────────────────────────────────────────────────────────────────────
describe('the handler\'s query shape does not survive the runtime\'s own facade', () => {
/**
* Measured while covering the apply path for #65 and filed as #79 — a
* different defect from the missing validation rule, and not fixed here.
*
* `applyCatalogHandler` calls `engine.find('duly_catalog_item', { where: …
* })`. The runtime's `buildActionEngineFacade` wraps whatever it is given:
* `ql.find(object, { where: query })`. So through the real dispatcher the
* handler's own `where` becomes `{ where: { where: … } }`, no row has a
* field called `where`, and the read comes back EMPTY — with no error. The
* action then reports `{ created: 0 }` and a successful run.
*
* Pinned so the seam is visible rather than folklore. When #79 is fixed
* this goes red: delete this describe block — do not adjust it — and
* `handlerFacade` above becomes the only convention in the file.
*/
it('finds nothing, creates nothing, and reports success', async () => {
const position = 'apply_runtime_facade';
await insertItem({ position_code: position, form: 'recurring', frequency: 'weekly' });

// Same item, same params, the only difference being which facade.
const viaHandlerConvention = await apply(handlerFacade(), {
position_code: position,
users: ['u_f1'],
});
expect(viaHandlerConvention.catalog_items).toBe(1);
expect(viaHandlerConvention.created).toBe(1);

const viaRuntime = await apply(runtimeFacade(), { position_code: position, users: ['u_f2'] });
expect(viaRuntime.catalog_items).toBe(0);
expect(viaRuntime.created).toBe(0);
expect(await dutiesOf('u_f2')).toEqual([]);
});
});
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 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
55 changes: 50 additions & 5 deletions src/actions/catalog.handlers.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -110,6 +110,41 @@ export function resolveDutyTimezone(): string {
return DEFAULT_DUTY_TIMEZONE;
}

// ── The engine facade's query shape ───────────────────────────────────
//
// `ctx.engine.find(object, query)` takes a BARE FILTER, not an ObjectQL query
// envelope. The runtime builds the envelope itself — `buildActionEngineFacade`
// in @objectstack/runtime 17.2.0, read verbatim from its `dist/index.js`:
//
// async find(object, query) {
// const where = query && Object.keys(query).length ? { where: query } : {};
// const rows = await ql.find(object, { ...where, context });
//
// So every read in this file passes `{ field: value }`, never
// `{ where: { field: value } }`. Handing it an envelope produces
// `{ where: { where: { … } } }`; no row has a field called `where`, so the read
// comes back EMPTY WITH NO ERROR. That is the failure this file shipped with:
// `duly_catalog_apply` reported a successful run of zero, `duly_catalog_sync`
// scanned nothing and called every duty unchanged, and `resolveBusinessUnit`
// anchored no duty at all — silently, because "no position row" is a legitimate
// day-one state. The ONE unfiltered read survived, because
// `Object.keys({}).length === 0` skips the wrapping entirely, which is exactly
// what made the handler look partially alive.
//
// `ActionEngineFacade.find` in @objectstack/spec types `query` as a plain
// record of string to unknown and says nothing about which of the two shapes it
// is — the runtime's implementation is the only thing that decides, and this
// app read it the other way. Filed upstream as
// **objectstack-ai/objectstack#14175** so the shape is DECLARED rather than
// discovered; until that lands this comment is the contract.
//
// ⛔ Do NOT add a tolerant `query.where ?? query` rung — not here, not in a
// test double. A consumer that accepts both shapes is precisely what let the
// wrong one ship green: `test/catalog-instantiate.test.ts`'s fake honoured the
// envelope, so 78 assertions passed against a shape production never produces.
// The test that can see this is one that dispatches through the REAL route and
// lets the runtime build its own facade — `test/catalog-engine-facade.test.ts`.

// ── Shapes ──────────────────────────────────────────────────────────────────

export interface CatalogApplyParams extends Record<string, unknown> {
Expand DownExpand Up@@ -256,12 +291,21 @@ export function pairKey(catalogItem: unknown, owner: unknown): string {
* exists on the platform. It is NOT read here — the issue names the
* assignment-level anchor, and adding a fallback rung is a product decision,
* reported rather than taken.)
*
* ⚠️ That tolerance is why this read's query shape matters more than the other
* three. The other reads fail into a visibly empty report — zero items, zero
* scanned — but this one fails into a state the handler is WRITTEN to accept:
* an envelope-shaped filter returned nothing, "nothing" reads as "not yet
* modelled", and every duty was created unanchored with no error anywhere. The
* rollups that the business unit exists to feed were simply empty. See the
* facade-shape note above; the end-to-end coverage is in
* `test/catalog-engine-facade.test.ts`.
*/
async function resolveBusinessUnit(
engine: ActionEngineFacade,
userId: string,
): Promise<string | undefined> {
const rows = await engine.find('sys_user_position', { where: { user_id: userId } });
const rows = await engine.find('sys_user_position', { user_id: userId });
for (const row of rows) {
// A person can hold several positions; take the first anchored one.
// Unanchored rows (`null`) are legacy/tenant-wide and carry no depth.
Expand All@@ -282,13 +326,14 @@ export const applyCatalogHandler: ActionHandler<CatalogApplyParams> = async (ctx
// stopped asking for; handing it to a new hire on their first day is the
// opposite of what deactivating it meant.
const items = await engine.find('duly_catalog_item', {
where: { position_code: positionCode, active: true },
position_code: positionCode,
active: true,
});
const activeItems = items.filter((item) => item?.active !== false);

// One probe for the whole run, not one per (item, user). The pair set is
// what makes a second apply create nothing.
const existing = await engine.find('duly_duty', { where: { owner: { $in: users } } });
const existing = await engine.find('duly_duty', { owner: { $in: users } });
const taken = new Set<string>();
for (const duty of existing) {
// Any duty already pointing at this catalog item for this person counts —
Expand DownExpand Up@@ -384,15 +429,15 @@ export const syncCatalogHandler: ActionHandler<CatalogSyncParams> = async (ctx)
// the retired report is made of, so it has to come back from this read.
const items = await engine.find(
'duly_catalog_item',
positionCode ? { where: { position_code: positionCode } } : {},
positionCode ? { position_code: positionCode } : {},
);
const byId = new Map<string, Record<string, unknown>>();
for (const item of items) {
if (positionCode && text(item?.position_code) !== positionCode) continue;
byId.set(recordId(item), item);
}

const duties = await engine.find('duly_duty', { where: { source: 'catalog' } });
const duties = await engine.find('duly_duty', { source: 'catalog' });

const changes: CatalogSyncChange[] = [];
const retired: CatalogSyncRetired[] = [];
Expand Down
90 changes: 22 additions & 68 deletions test/catalog-apply-cadence.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -23,7 +23,7 @@ import type { CatalogApplyResult } from '../src/actions/catalog.handlers.js';
* apply path unprotected is half a fix.
*
* ── Why a REAL booted engine and not `catalog-instantiate.test.ts`'s fake ──
* That suite's `FakeEngine` is a Map with a `where` matcher: it runs no
* That suite's `FakeEngine` is a Map with a filter matcher: it runs no
* validation rules and stamps no defaults, so every claim below would pass on
* it for the wrong reason. Validation and `applyFieldDefaults` are precisely
* what is under test here, so the handler is dispatched through the app's own
Expand DownExpand Up@@ -60,7 +60,7 @@ afterAll(async () => {
await kernel?.shutdown?.();
});

// ── The two engine facades ──────────────────────────────────────────────────
// ── The engine facade ───────────────────────────────────────────────────────

interface Facade {
insert(object: string, values: AnyRow): Promise<{ id: string }>;
Expand All@@ -70,15 +70,29 @@ interface Facade {
}

/**
* The facade `applyCatalogHandler` is WRITTEN against: `find(object, query)`
* takes ObjectQL's own query envelope, `where` and all. It is the convention
* `catalog-instantiate.test.ts`'s `FakeEngine` honours too, so this is the
* shape every existing assertion about the handler is made under.
* The facade the RUNTIME builds, reproduced line for line —
* `buildActionEngineFacade` in @objectstack/runtime 17.2.0:
*
* async find(object, query) {
* const where = query && Object.keys(query).length ? { where: query } : {};
* const rows = await ql.find(object, { ...where, context });
*
* `find(object, query)` therefore takes a BARE FILTER and wraps it here. This
* file used to carry TWO facades — this one honouring the handler's own
* `where` envelope, and a second reproducing the runtime — with a tripwire
* pinning the gap between them (#79). The handler now passes flat filters, so
* there is one convention and one facade.
*
* It is still a double, which is all it can be: the second test below has to
* put a row in front of the handler that `duly_catalog_item`'s own rules
* refuse, and only a hand-supplied facade can do that. What a double cannot do
* is prove the wire shape is right — it encodes the author's belief about it.
* That proof lives in `test/catalog-engine-facade.test.ts`, which dispatches
* through the real action route and lets the runtime build its own facade.
*
* `catalogItems`, when given, replaces the catalog read with rows handed
* straight to the handler (unfiltered — the handler re-applies its own
* `active` filter). That is how a row the object's own rules now REFUSE can
* still be put in front of the handler, which one test below needs.
* `active` filter).
*/
function handlerFacade(catalogItems?: AnyRow[]): Facade {
return {
Expand All@@ -94,28 +108,6 @@ function handlerFacade(catalogItems?: AnyRow[]): Facade {
},
find: async (object, query) => {
if (catalogItems && object === 'duly_catalog_item') return catalogItems.map((r) => ({ ...r }));
return data.find(object, query);
},
};
}

/**
* The facade the RUNTIME actually builds — `buildActionEngineFacade` in
* @objectstack/runtime 17.2.0, reproduced line for line:
*
* async find(object, query) {
* const where = query && Object.keys(query).length ? { where: query } : {};
* const rows = await ql.find(object, { ...where, context });
* ...
*
* It wraps whatever it is handed in a `where` of its own. Used by exactly one
* test, the tripwire at the bottom.
*/
function runtimeFacade(): Facade {
const base = handlerFacade();
return {
...base,
find: async (object, query) => {
const where = query && Object.keys(query).length ? { where: query } : {};
return data.find(object, { ...where });
},
Expand DownExpand Up@@ -227,41 +219,3 @@ describe('duly_catalog_apply — the cadence it replicates (#65)', () => {
for (const field of CADENCE_FIELDS) expect(duty?.[field] ?? null, field).toBeNull();
});
});

// ───────────────────────────────────────────────────────────────────────────
// A tripwire on a filed defect — NOT an assertion that this is correct
// ───────────────────────────────────────────────────────────────────────────
describe('the handler\'s query shape does not survive the runtime\'s own facade', () => {
/**
* Measured while covering the apply path for #65 and filed as #79 — a
* different defect from the missing validation rule, and not fixed here.
*
* `applyCatalogHandler` calls `engine.find('duly_catalog_item', { where: …
* })`. The runtime's `buildActionEngineFacade` wraps whatever it is given:
* `ql.find(object, { where: query })`. So through the real dispatcher the
* handler's own `where` becomes `{ where: { where: … } }`, no row has a
* field called `where`, and the read comes back EMPTY — with no error. The
* action then reports `{ created: 0 }` and a successful run.
*
* Pinned so the seam is visible rather than folklore. When #79 is fixed
* this goes red: delete this describe block — do not adjust it — and
* `handlerFacade` above becomes the only convention in the file.
*/
it('finds nothing, creates nothing, and reports success', async () => {
const position = 'apply_runtime_facade';
await insertItem({ position_code: position, form: 'recurring', frequency: 'weekly' });

// Same item, same params, the only difference being which facade.
const viaHandlerConvention = await apply(handlerFacade(), {
position_code: position,
users: ['u_f1'],
});
expect(viaHandlerConvention.catalog_items).toBe(1);
expect(viaHandlerConvention.created).toBe(1);

const viaRuntime = await apply(runtimeFacade(), { position_code: position, users: ['u_f2'] });
expect(viaRuntime.catalog_items).toBe(0);
expect(viaRuntime.created).toBe(0);
expect(await dutiesOf('u_f2')).toEqual([]);
});
});
Loading
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
55 changes: 50 additions & 5 deletions src/actions/catalog.handlers.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -110,6 +110,41 @@ export function resolveDutyTimezone(): string {
return DEFAULT_DUTY_TIMEZONE;
}

// ── The engine facade's query shape ───────────────────────────────────
//
// `ctx.engine.find(object, query)` takes a BARE FILTER, not an ObjectQL query
// envelope. The runtime builds the envelope itself — `buildActionEngineFacade`
// in @objectstack/runtime 17.2.0, read verbatim from its `dist/index.js`:
//
// async find(object, query) {
// const where = query && Object.keys(query).length ? { where: query } : {};
// const rows = await ql.find(object, { ...where, context });
//
// So every read in this file passes `{ field: value }`, never
// `{ where: { field: value } }`. Handing it an envelope produces
// `{ where: { where: { … } } }`; no row has a field called `where`, so the read
// comes back EMPTY WITH NO ERROR. That is the failure this file shipped with:
// `duly_catalog_apply` reported a successful run of zero, `duly_catalog_sync`
// scanned nothing and called every duty unchanged, and `resolveBusinessUnit`
// anchored no duty at all — silently, because "no position row" is a legitimate
// day-one state. The ONE unfiltered read survived, because
// `Object.keys({}).length === 0` skips the wrapping entirely, which is exactly
// what made the handler look partially alive.
//
// `ActionEngineFacade.find` in @objectstack/spec types `query` as a plain
// record of string to unknown and says nothing about which of the two shapes it
// is — the runtime's implementation is the only thing that decides, and this
// app read it the other way. Filed upstream as
// **objectstack-ai/objectstack#14175** so the shape is DECLARED rather than
// discovered; until that lands this comment is the contract.
//
// ⛔ Do NOT add a tolerant `query.where ?? query` rung — not here, not in a
// test double. A consumer that accepts both shapes is precisely what let the
// wrong one ship green: `test/catalog-instantiate.test.ts`'s fake honoured the
// envelope, so 78 assertions passed against a shape production never produces.
// The test that can see this is one that dispatches through the REAL route and
// lets the runtime build its own facade — `test/catalog-engine-facade.test.ts`.

// ── Shapes ──────────────────────────────────────────────────────────────────

export interface CatalogApplyParams extends Record<string, unknown> {
Expand DownExpand Up@@ -256,12 +291,21 @@ export function pairKey(catalogItem: unknown, owner: unknown): string {
* exists on the platform. It is NOT read here — the issue names the
* assignment-level anchor, and adding a fallback rung is a product decision,
* reported rather than taken.)
*
* ⚠️ That tolerance is why this read's query shape matters more than the other
* three. The other reads fail into a visibly empty report — zero items, zero
* scanned — but this one fails into a state the handler is WRITTEN to accept:
* an envelope-shaped filter returned nothing, "nothing" reads as "not yet
* modelled", and every duty was created unanchored with no error anywhere. The
* rollups that the business unit exists to feed were simply empty. See the
* facade-shape note above; the end-to-end coverage is in
* `test/catalog-engine-facade.test.ts`.
*/
async function resolveBusinessUnit(
engine: ActionEngineFacade,
userId: string,
): Promise<string | undefined> {
const rows = await engine.find('sys_user_position', { where: { user_id: userId } });
const rows = await engine.find('sys_user_position', { user_id: userId });
for (const row of rows) {
// A person can hold several positions; take the first anchored one.
// Unanchored rows (`null`) are legacy/tenant-wide and carry no depth.
Expand All@@ -282,13 +326,14 @@ export const applyCatalogHandler: ActionHandler<CatalogApplyParams> = async (ctx
// stopped asking for; handing it to a new hire on their first day is the
// opposite of what deactivating it meant.
const items = await engine.find('duly_catalog_item', {
where: { position_code: positionCode, active: true },
position_code: positionCode,
active: true,
});
const activeItems = items.filter((item) => item?.active !== false);

// One probe for the whole run, not one per (item, user). The pair set is
// what makes a second apply create nothing.
const existing = await engine.find('duly_duty', { where: { owner: { $in: users } } });
const existing = await engine.find('duly_duty', { owner: { $in: users } });
const taken = new Set<string>();
for (const duty of existing) {
// Any duty already pointing at this catalog item for this person counts —
Expand DownExpand Up@@ -384,15 +429,15 @@ export const syncCatalogHandler: ActionHandler<CatalogSyncParams> = async (ctx)
// the retired report is made of, so it has to come back from this read.
const items = await engine.find(
'duly_catalog_item',
positionCode ? { where: { position_code: positionCode } } : {},
positionCode ? { position_code: positionCode } : {},
);
const byId = new Map<string, Record<string, unknown>>();
for (const item of items) {
if (positionCode && text(item?.position_code) !== positionCode) continue;
byId.set(recordId(item), item);
}

const duties = await engine.find('duly_duty', { where: { source: 'catalog' } });
const duties = await engine.find('duly_duty', { source: 'catalog' });

const changes: CatalogSyncChange[] = [];
const retired: CatalogSyncRetired[] = [];
Expand Down
90 changes: 22 additions & 68 deletions test/catalog-apply-cadence.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -23,7 +23,7 @@ import type { CatalogApplyResult } from '../src/actions/catalog.handlers.js';
* apply path unprotected is half a fix.
*
* ── Why a REAL booted engine and not `catalog-instantiate.test.ts`'s fake ──
* That suite's `FakeEngine` is a Map with a `where` matcher: it runs no
* That suite's `FakeEngine` is a Map with a filter matcher: it runs no
* validation rules and stamps no defaults, so every claim below would pass on
* it for the wrong reason. Validation and `applyFieldDefaults` are precisely
* what is under test here, so the handler is dispatched through the app's own
Expand DownExpand Up@@ -60,7 +60,7 @@ afterAll(async () => {
await kernel?.shutdown?.();
});

// ── The two engine facades ──────────────────────────────────────────────────
// ── The engine facade ───────────────────────────────────────────────────────

interface Facade {
insert(object: string, values: AnyRow): Promise<{ id: string }>;
Expand All@@ -70,15 +70,29 @@ interface Facade {
}

/**
* The facade `applyCatalogHandler` is WRITTEN against: `find(object, query)`
* takes ObjectQL's own query envelope, `where` and all. It is the convention
* `catalog-instantiate.test.ts`'s `FakeEngine` honours too, so this is the
* shape every existing assertion about the handler is made under.
* The facade the RUNTIME builds, reproduced line for line —
* `buildActionEngineFacade` in @objectstack/runtime 17.2.0:
*
* async find(object, query) {
* const where = query && Object.keys(query).length ? { where: query } : {};
* const rows = await ql.find(object, { ...where, context });
*
* `find(object, query)` therefore takes a BARE FILTER and wraps it here. This
* file used to carry TWO facades — this one honouring the handler's own
* `where` envelope, and a second reproducing the runtime — with a tripwire
* pinning the gap between them (#79). The handler now passes flat filters, so
* there is one convention and one facade.
*
* It is still a double, which is all it can be: the second test below has to
* put a row in front of the handler that `duly_catalog_item`'s own rules
* refuse, and only a hand-supplied facade can do that. What a double cannot do
* is prove the wire shape is right — it encodes the author's belief about it.
* That proof lives in `test/catalog-engine-facade.test.ts`, which dispatches
* through the real action route and lets the runtime build its own facade.
*
* `catalogItems`, when given, replaces the catalog read with rows handed
* straight to the handler (unfiltered — the handler re-applies its own
* `active` filter). That is how a row the object's own rules now REFUSE can
* still be put in front of the handler, which one test below needs.
* `active` filter).
*/
function handlerFacade(catalogItems?: AnyRow[]): Facade {
return {
Expand All@@ -94,28 +108,6 @@ function handlerFacade(catalogItems?: AnyRow[]): Facade {
},
find: async (object, query) => {
if (catalogItems && object === 'duly_catalog_item') return catalogItems.map((r) => ({ ...r }));
return data.find(object, query);
},
};
}

/**
* The facade the RUNTIME actually builds — `buildActionEngineFacade` in
* @objectstack/runtime 17.2.0, reproduced line for line:
*
* async find(object, query) {
* const where = query && Object.keys(query).length ? { where: query } : {};
* const rows = await ql.find(object, { ...where, context });
* ...
*
* It wraps whatever it is handed in a `where` of its own. Used by exactly one
* test, the tripwire at the bottom.
*/
function runtimeFacade(): Facade {
const base = handlerFacade();
return {
...base,
find: async (object, query) => {
const where = query && Object.keys(query).length ? { where: query } : {};
return data.find(object, { ...where });
},
Expand DownExpand Up@@ -227,41 +219,3 @@ describe('duly_catalog_apply — the cadence it replicates (#65)', () => {
for (const field of CADENCE_FIELDS) expect(duty?.[field] ?? null, field).toBeNull();
});
});

// ───────────────────────────────────────────────────────────────────────────
// A tripwire on a filed defect — NOT an assertion that this is correct
// ───────────────────────────────────────────────────────────────────────────
describe('the handler\'s query shape does not survive the runtime\'s own facade', () => {
/**
* Measured while covering the apply path for #65 and filed as #79 — a
* different defect from the missing validation rule, and not fixed here.
*
* `applyCatalogHandler` calls `engine.find('duly_catalog_item', { where: …
* })`. The runtime's `buildActionEngineFacade` wraps whatever it is given:
* `ql.find(object, { where: query })`. So through the real dispatcher the
* handler's own `where` becomes `{ where: { where: … } }`, no row has a
* field called `where`, and the read comes back EMPTY — with no error. The
* action then reports `{ created: 0 }` and a successful run.
*
* Pinned so the seam is visible rather than folklore. When #79 is fixed
* this goes red: delete this describe block — do not adjust it — and
* `handlerFacade` above becomes the only convention in the file.
*/
it('finds nothing, creates nothing, and reports success', async () => {
const position = 'apply_runtime_facade';
await insertItem({ position_code: position, form: 'recurring', frequency: 'weekly' });

// Same item, same params, the only difference being which facade.
const viaHandlerConvention = await apply(handlerFacade(), {
position_code: position,
users: ['u_f1'],
});
expect(viaHandlerConvention.catalog_items).toBe(1);
expect(viaHandlerConvention.created).toBe(1);

const viaRuntime = await apply(runtimeFacade(), { position_code: position, users: ['u_f2'] });
expect(viaRuntime.catalog_items).toBe(0);
expect(viaRuntime.created).toBe(0);
expect(await dutiesOf('u_f2')).toEqual([]);
});
});
Loading
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 > 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
55 changes: 50 additions & 5 deletions src/actions/catalog.handlers.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -110,6 +110,41 @@ export function resolveDutyTimezone(): string {
return DEFAULT_DUTY_TIMEZONE;
}

// ── The engine facade's query shape ───────────────────────────────────
//
// `ctx.engine.find(object, query)` takes a BARE FILTER, not an ObjectQL query
// envelope. The runtime builds the envelope itself — `buildActionEngineFacade`
// in @objectstack/runtime 17.2.0, read verbatim from its `dist/index.js`:
//
// async find(object, query) {
// const where = query && Object.keys(query).length ? { where: query } : {};
// const rows = await ql.find(object, { ...where, context });
//
// So every read in this file passes `{ field: value }`, never
// `{ where: { field: value } }`. Handing it an envelope produces
// `{ where: { where: { … } } }`; no row has a field called `where`, so the read
// comes back EMPTY WITH NO ERROR. That is the failure this file shipped with:
// `duly_catalog_apply` reported a successful run of zero, `duly_catalog_sync`
// scanned nothing and called every duty unchanged, and `resolveBusinessUnit`
// anchored no duty at all — silently, because "no position row" is a legitimate
// day-one state. The ONE unfiltered read survived, because
// `Object.keys({}).length === 0` skips the wrapping entirely, which is exactly
// what made the handler look partially alive.
//
// `ActionEngineFacade.find` in @objectstack/spec types `query` as a plain
// record of string to unknown and says nothing about which of the two shapes it
// is — the runtime's implementation is the only thing that decides, and this
// app read it the other way. Filed upstream as
// **objectstack-ai/objectstack#14175** so the shape is DECLARED rather than
// discovered; until that lands this comment is the contract.
//
// ⛔ Do NOT add a tolerant `query.where ?? query` rung — not here, not in a
// test double. A consumer that accepts both shapes is precisely what let the
// wrong one ship green: `test/catalog-instantiate.test.ts`'s fake honoured the
// envelope, so 78 assertions passed against a shape production never produces.
// The test that can see this is one that dispatches through the REAL route and
// lets the runtime build its own facade — `test/catalog-engine-facade.test.ts`.

// ── Shapes ──────────────────────────────────────────────────────────────────

export interface CatalogApplyParams extends Record<string, unknown> {
Expand DownExpand Up@@ -256,12 +291,21 @@ export function pairKey(catalogItem: unknown, owner: unknown): string {
* exists on the platform. It is NOT read here — the issue names the
* assignment-level anchor, and adding a fallback rung is a product decision,
* reported rather than taken.)
*
* ⚠️ That tolerance is why this read's query shape matters more than the other
* three. The other reads fail into a visibly empty report — zero items, zero
* scanned — but this one fails into a state the handler is WRITTEN to accept:
* an envelope-shaped filter returned nothing, "nothing" reads as "not yet
* modelled", and every duty was created unanchored with no error anywhere. The
* rollups that the business unit exists to feed were simply empty. See the
* facade-shape note above; the end-to-end coverage is in
* `test/catalog-engine-facade.test.ts`.
*/
async function resolveBusinessUnit(
engine: ActionEngineFacade,
userId: string,
): Promise<string | undefined> {
const rows = await engine.find('sys_user_position', { where: { user_id: userId } });
const rows = await engine.find('sys_user_position', { user_id: userId });
for (const row of rows) {
// A person can hold several positions; take the first anchored one.
// Unanchored rows (`null`) are legacy/tenant-wide and carry no depth.
Expand All@@ -282,13 +326,14 @@ export const applyCatalogHandler: ActionHandler<CatalogApplyParams> = async (ctx
// stopped asking for; handing it to a new hire on their first day is the
// opposite of what deactivating it meant.
const items = await engine.find('duly_catalog_item', {
where: { position_code: positionCode, active: true },
position_code: positionCode,
active: true,
});
const activeItems = items.filter((item) => item?.active !== false);

// One probe for the whole run, not one per (item, user). The pair set is
// what makes a second apply create nothing.
const existing = await engine.find('duly_duty', { where: { owner: { $in: users } } });
const existing = await engine.find('duly_duty', { owner: { $in: users } });
const taken = new Set<string>();
for (const duty of existing) {
// Any duty already pointing at this catalog item for this person counts —
Expand DownExpand Up@@ -384,15 +429,15 @@ export const syncCatalogHandler: ActionHandler<CatalogSyncParams> = async (ctx)
// the retired report is made of, so it has to come back from this read.
const items = await engine.find(
'duly_catalog_item',
positionCode ? { where: { position_code: positionCode } } : {},
positionCode ? { position_code: positionCode } : {},
);
const byId = new Map<string, Record<string, unknown>>();
for (const item of items) {
if (positionCode && text(item?.position_code) !== positionCode) continue;
byId.set(recordId(item), item);
}

const duties = await engine.find('duly_duty', { where: { source: 'catalog' } });
const duties = await engine.find('duly_duty', { source: 'catalog' });

const changes: CatalogSyncChange[] = [];
const retired: CatalogSyncRetired[] = [];
Expand Down
90 changes: 22 additions & 68 deletions test/catalog-apply-cadence.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -23,7 +23,7 @@ import type { CatalogApplyResult } from '../src/actions/catalog.handlers.js';
* apply path unprotected is half a fix.
*
* ── Why a REAL booted engine and not `catalog-instantiate.test.ts`'s fake ──
* That suite's `FakeEngine` is a Map with a `where` matcher: it runs no
* That suite's `FakeEngine` is a Map with a filter matcher: it runs no
* validation rules and stamps no defaults, so every claim below would pass on
* it for the wrong reason. Validation and `applyFieldDefaults` are precisely
* what is under test here, so the handler is dispatched through the app's own
Expand DownExpand Up@@ -60,7 +60,7 @@ afterAll(async () => {
await kernel?.shutdown?.();
});

// ── The two engine facades ──────────────────────────────────────────────────
// ── The engine facade ───────────────────────────────────────────────────────

interface Facade {
insert(object: string, values: AnyRow): Promise<{ id: string }>;
Expand All@@ -70,15 +70,29 @@ interface Facade {
}

/**
* The facade `applyCatalogHandler` is WRITTEN against: `find(object, query)`
* takes ObjectQL's own query envelope, `where` and all. It is the convention
* `catalog-instantiate.test.ts`'s `FakeEngine` honours too, so this is the
* shape every existing assertion about the handler is made under.
* The facade the RUNTIME builds, reproduced line for line —
* `buildActionEngineFacade` in @objectstack/runtime 17.2.0:
*
* async find(object, query) {
* const where = query && Object.keys(query).length ? { where: query } : {};
* const rows = await ql.find(object, { ...where, context });
*
* `find(object, query)` therefore takes a BARE FILTER and wraps it here. This
* file used to carry TWO facades — this one honouring the handler's own
* `where` envelope, and a second reproducing the runtime — with a tripwire
* pinning the gap between them (#79). The handler now passes flat filters, so
* there is one convention and one facade.
*
* It is still a double, which is all it can be: the second test below has to
* put a row in front of the handler that `duly_catalog_item`'s own rules
* refuse, and only a hand-supplied facade can do that. What a double cannot do
* is prove the wire shape is right — it encodes the author's belief about it.
* That proof lives in `test/catalog-engine-facade.test.ts`, which dispatches
* through the real action route and lets the runtime build its own facade.
*
* `catalogItems`, when given, replaces the catalog read with rows handed
* straight to the handler (unfiltered — the handler re-applies its own
* `active` filter). That is how a row the object's own rules now REFUSE can
* still be put in front of the handler, which one test below needs.
* `active` filter).
*/
function handlerFacade(catalogItems?: AnyRow[]): Facade {
return {
Expand All@@ -94,28 +108,6 @@ function handlerFacade(catalogItems?: AnyRow[]): Facade {
},
find: async (object, query) => {
if (catalogItems && object === 'duly_catalog_item') return catalogItems.map((r) => ({ ...r }));
return data.find(object, query);
},
};
}

/**
* The facade the RUNTIME actually builds — `buildActionEngineFacade` in
* @objectstack/runtime 17.2.0, reproduced line for line:
*
* async find(object, query) {
* const where = query && Object.keys(query).length ? { where: query } : {};
* const rows = await ql.find(object, { ...where, context });
* ...
*
* It wraps whatever it is handed in a `where` of its own. Used by exactly one
* test, the tripwire at the bottom.
*/
function runtimeFacade(): Facade {
const base = handlerFacade();
return {
...base,
find: async (object, query) => {
const where = query && Object.keys(query).length ? { where: query } : {};
return data.find(object, { ...where });
},
Expand DownExpand Up@@ -227,41 +219,3 @@ describe('duly_catalog_apply — the cadence it replicates (#65)', () => {
for (const field of CADENCE_FIELDS) expect(duty?.[field] ?? null, field).toBeNull();
});
});

// ───────────────────────────────────────────────────────────────────────────
// A tripwire on a filed defect — NOT an assertion that this is correct
// ───────────────────────────────────────────────────────────────────────────
describe('the handler\'s query shape does not survive the runtime\'s own facade', () => {
/**
* Measured while covering the apply path for #65 and filed as #79 — a
* different defect from the missing validation rule, and not fixed here.
*
* `applyCatalogHandler` calls `engine.find('duly_catalog_item', { where: …
* })`. The runtime's `buildActionEngineFacade` wraps whatever it is given:
* `ql.find(object, { where: query })`. So through the real dispatcher the
* handler's own `where` becomes `{ where: { where: … } }`, no row has a
* field called `where`, and the read comes back EMPTY — with no error. The
* action then reports `{ created: 0 }` and a successful run.
*
* Pinned so the seam is visible rather than folklore. When #79 is fixed
* this goes red: delete this describe block — do not adjust it — and
* `handlerFacade` above becomes the only convention in the file.
*/
it('finds nothing, creates nothing, and reports success', async () => {
const position = 'apply_runtime_facade';
await insertItem({ position_code: position, form: 'recurring', frequency: 'weekly' });

// Same item, same params, the only difference being which facade.
const viaHandlerConvention = await apply(handlerFacade(), {
position_code: position,
users: ['u_f1'],
});
expect(viaHandlerConvention.catalog_items).toBe(1);
expect(viaHandlerConvention.created).toBe(1);

const viaRuntime = await apply(runtimeFacade(), { position_code: position, users: ['u_f2'] });
expect(viaRuntime.catalog_items).toBe(0);
expect(viaRuntime.created).toBe(0);
expect(await dutiesOf('u_f2')).toEqual([]);
});
});
Loading
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
55 changes: 50 additions & 5 deletions src/actions/catalog.handlers.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -110,6 +110,41 @@ export function resolveDutyTimezone(): string {
return DEFAULT_DUTY_TIMEZONE;
}

// ── The engine facade's query shape ───────────────────────────────────
//
// `ctx.engine.find(object, query)` takes a BARE FILTER, not an ObjectQL query
// envelope. The runtime builds the envelope itself — `buildActionEngineFacade`
// in @objectstack/runtime 17.2.0, read verbatim from its `dist/index.js`:
//
// async find(object, query) {
// const where = query && Object.keys(query).length ? { where: query } : {};
// const rows = await ql.find(object, { ...where, context });
//
// So every read in this file passes `{ field: value }`, never
// `{ where: { field: value } }`. Handing it an envelope produces
// `{ where: { where: { … } } }`; no row has a field called `where`, so the read
// comes back EMPTY WITH NO ERROR. That is the failure this file shipped with:
// `duly_catalog_apply` reported a successful run of zero, `duly_catalog_sync`
// scanned nothing and called every duty unchanged, and `resolveBusinessUnit`
// anchored no duty at all — silently, because "no position row" is a legitimate
// day-one state. The ONE unfiltered read survived, because
// `Object.keys({}).length === 0` skips the wrapping entirely, which is exactly
// what made the handler look partially alive.
//
// `ActionEngineFacade.find` in @objectstack/spec types `query` as a plain
// record of string to unknown and says nothing about which of the two shapes it
// is — the runtime's implementation is the only thing that decides, and this
// app read it the other way. Filed upstream as
// **objectstack-ai/objectstack#14175** so the shape is DECLARED rather than
// discovered; until that lands this comment is the contract.
//
// ⛔ Do NOT add a tolerant `query.where ?? query` rung — not here, not in a
// test double. A consumer that accepts both shapes is precisely what let the
// wrong one ship green: `test/catalog-instantiate.test.ts`'s fake honoured the
// envelope, so 78 assertions passed against a shape production never produces.
// The test that can see this is one that dispatches through the REAL route and
// lets the runtime build its own facade — `test/catalog-engine-facade.test.ts`.

// ── Shapes ──────────────────────────────────────────────────────────────────

export interface CatalogApplyParams extends Record<string, unknown> {
Expand DownExpand Up@@ -256,12 +291,21 @@ export function pairKey(catalogItem: unknown, owner: unknown): string {
* exists on the platform. It is NOT read here — the issue names the
* assignment-level anchor, and adding a fallback rung is a product decision,
* reported rather than taken.)
*
* ⚠️ That tolerance is why this read's query shape matters more than the other
* three. The other reads fail into a visibly empty report — zero items, zero
* scanned — but this one fails into a state the handler is WRITTEN to accept:
* an envelope-shaped filter returned nothing, "nothing" reads as "not yet
* modelled", and every duty was created unanchored with no error anywhere. The
* rollups that the business unit exists to feed were simply empty. See the
* facade-shape note above; the end-to-end coverage is in
* `test/catalog-engine-facade.test.ts`.
*/
async function resolveBusinessUnit(
engine: ActionEngineFacade,
userId: string,
): Promise<string | undefined> {
const rows = await engine.find('sys_user_position', { where: { user_id: userId } });
const rows = await engine.find('sys_user_position', { user_id: userId });
for (const row of rows) {
// A person can hold several positions; take the first anchored one.
// Unanchored rows (`null`) are legacy/tenant-wide and carry no depth.
Expand All@@ -282,13 +326,14 @@ export const applyCatalogHandler: ActionHandler<CatalogApplyParams> = async (ctx
// stopped asking for; handing it to a new hire on their first day is the
// opposite of what deactivating it meant.
const items = await engine.find('duly_catalog_item', {
where: { position_code: positionCode, active: true },
position_code: positionCode,
active: true,
});
const activeItems = items.filter((item) => item?.active !== false);

// One probe for the whole run, not one per (item, user). The pair set is
// what makes a second apply create nothing.
const existing = await engine.find('duly_duty', { where: { owner: { $in: users } } });
const existing = await engine.find('duly_duty', { owner: { $in: users } });
const taken = new Set<string>();
for (const duty of existing) {
// Any duty already pointing at this catalog item for this person counts —
Expand DownExpand Up@@ -384,15 +429,15 @@ export const syncCatalogHandler: ActionHandler<CatalogSyncParams> = async (ctx)
// the retired report is made of, so it has to come back from this read.
const items = await engine.find(
'duly_catalog_item',
positionCode ? { where: { position_code: positionCode } } : {},
positionCode ? { position_code: positionCode } : {},
);
const byId = new Map<string, Record<string, unknown>>();
for (const item of items) {
if (positionCode && text(item?.position_code) !== positionCode) continue;
byId.set(recordId(item), item);
}

const duties = await engine.find('duly_duty', { where: { source: 'catalog' } });
const duties = await engine.find('duly_duty', { source: 'catalog' });

const changes: CatalogSyncChange[] = [];
const retired: CatalogSyncRetired[] = [];
Expand Down
90 changes: 22 additions & 68 deletions test/catalog-apply-cadence.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -23,7 +23,7 @@ import type { CatalogApplyResult } from '../src/actions/catalog.handlers.js';
* apply path unprotected is half a fix.
*
* ── Why a REAL booted engine and not `catalog-instantiate.test.ts`'s fake ──
* That suite's `FakeEngine` is a Map with a `where` matcher: it runs no
* That suite's `FakeEngine` is a Map with a filter matcher: it runs no
* validation rules and stamps no defaults, so every claim below would pass on
* it for the wrong reason. Validation and `applyFieldDefaults` are precisely
* what is under test here, so the handler is dispatched through the app's own
Expand DownExpand Up@@ -60,7 +60,7 @@ afterAll(async () => {
await kernel?.shutdown?.();
});

// ── The two engine facades ──────────────────────────────────────────────────
// ── The engine facade ───────────────────────────────────────────────────────

interface Facade {
insert(object: string, values: AnyRow): Promise<{ id: string }>;
Expand All@@ -70,15 +70,29 @@ interface Facade {
}

/**
* The facade `applyCatalogHandler` is WRITTEN against: `find(object, query)`
* takes ObjectQL's own query envelope, `where` and all. It is the convention
* `catalog-instantiate.test.ts`'s `FakeEngine` honours too, so this is the
* shape every existing assertion about the handler is made under.
* The facade the RUNTIME builds, reproduced line for line —
* `buildActionEngineFacade` in @objectstack/runtime 17.2.0:
*
* async find(object, query) {
* const where = query && Object.keys(query).length ? { where: query } : {};
* const rows = await ql.find(object, { ...where, context });
*
* `find(object, query)` therefore takes a BARE FILTER and wraps it here. This
* file used to carry TWO facades — this one honouring the handler's own
* `where` envelope, and a second reproducing the runtime — with a tripwire
* pinning the gap between them (#79). The handler now passes flat filters, so
* there is one convention and one facade.
*
* It is still a double, which is all it can be: the second test below has to
* put a row in front of the handler that `duly_catalog_item`'s own rules
* refuse, and only a hand-supplied facade can do that. What a double cannot do
* is prove the wire shape is right — it encodes the author's belief about it.
* That proof lives in `test/catalog-engine-facade.test.ts`, which dispatches
* through the real action route and lets the runtime build its own facade.
*
* `catalogItems`, when given, replaces the catalog read with rows handed
* straight to the handler (unfiltered — the handler re-applies its own
* `active` filter). That is how a row the object's own rules now REFUSE can
* still be put in front of the handler, which one test below needs.
* `active` filter).
*/
function handlerFacade(catalogItems?: AnyRow[]): Facade {
return {
Expand All@@ -94,28 +108,6 @@ function handlerFacade(catalogItems?: AnyRow[]): Facade {
},
find: async (object, query) => {
if (catalogItems && object === 'duly_catalog_item') return catalogItems.map((r) => ({ ...r }));
return data.find(object, query);
},
};
}

/**
* The facade the RUNTIME actually builds — `buildActionEngineFacade` in
* @objectstack/runtime 17.2.0, reproduced line for line:
*
* async find(object, query) {
* const where = query && Object.keys(query).length ? { where: query } : {};
* const rows = await ql.find(object, { ...where, context });
* ...
*
* It wraps whatever it is handed in a `where` of its own. Used by exactly one
* test, the tripwire at the bottom.
*/
function runtimeFacade(): Facade {
const base = handlerFacade();
return {
...base,
find: async (object, query) => {
const where = query && Object.keys(query).length ? { where: query } : {};
return data.find(object, { ...where });
},
Expand DownExpand Up@@ -227,41 +219,3 @@ describe('duly_catalog_apply — the cadence it replicates (#65)', () => {
for (const field of CADENCE_FIELDS) expect(duty?.[field] ?? null, field).toBeNull();
});
});

// ───────────────────────────────────────────────────────────────────────────
// A tripwire on a filed defect — NOT an assertion that this is correct
// ───────────────────────────────────────────────────────────────────────────
describe('the handler\'s query shape does not survive the runtime\'s own facade', () => {
/**
* Measured while covering the apply path for #65 and filed as #79 — a
* different defect from the missing validation rule, and not fixed here.
*
* `applyCatalogHandler` calls `engine.find('duly_catalog_item', { where: …
* })`. The runtime's `buildActionEngineFacade` wraps whatever it is given:
* `ql.find(object, { where: query })`. So through the real dispatcher the
* handler's own `where` becomes `{ where: { where: … } }`, no row has a
* field called `where`, and the read comes back EMPTY — with no error. The
* action then reports `{ created: 0 }` and a successful run.
*
* Pinned so the seam is visible rather than folklore. When #79 is fixed
* this goes red: delete this describe block — do not adjust it — and
* `handlerFacade` above becomes the only convention in the file.
*/
it('finds nothing, creates nothing, and reports success', async () => {
const position = 'apply_runtime_facade';
await insertItem({ position_code: position, form: 'recurring', frequency: 'weekly' });

// Same item, same params, the only difference being which facade.
const viaHandlerConvention = await apply(handlerFacade(), {
position_code: position,
users: ['u_f1'],
});
expect(viaHandlerConvention.catalog_items).toBe(1);
expect(viaHandlerConvention.created).toBe(1);

const viaRuntime = await apply(runtimeFacade(), { position_code: position, users: ['u_f2'] });
expect(viaRuntime.catalog_items).toBe(0);
expect(viaRuntime.created).toBe(0);
expect(await dutiesOf('u_f2')).toEqual([]);
});
});
Loading
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
55 changes: 50 additions & 5 deletions src/actions/catalog.handlers.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -110,6 +110,41 @@ export function resolveDutyTimezone(): string {
return DEFAULT_DUTY_TIMEZONE;
}

// ── The engine facade's query shape ───────────────────────────────────
//
// `ctx.engine.find(object, query)` takes a BARE FILTER, not an ObjectQL query
// envelope. The runtime builds the envelope itself — `buildActionEngineFacade`
// in @objectstack/runtime 17.2.0, read verbatim from its `dist/index.js`:
//
// async find(object, query) {
// const where = query && Object.keys(query).length ? { where: query } : {};
// const rows = await ql.find(object, { ...where, context });
//
// So every read in this file passes `{ field: value }`, never
// `{ where: { field: value } }`. Handing it an envelope produces
// `{ where: { where: { … } } }`; no row has a field called `where`, so the read
// comes back EMPTY WITH NO ERROR. That is the failure this file shipped with:
// `duly_catalog_apply` reported a successful run of zero, `duly_catalog_sync`
// scanned nothing and called every duty unchanged, and `resolveBusinessUnit`
// anchored no duty at all — silently, because "no position row" is a legitimate
// day-one state. The ONE unfiltered read survived, because
// `Object.keys({}).length === 0` skips the wrapping entirely, which is exactly
// what made the handler look partially alive.
//
// `ActionEngineFacade.find` in @objectstack/spec types `query` as a plain
// record of string to unknown and says nothing about which of the two shapes it
// is — the runtime's implementation is the only thing that decides, and this
// app read it the other way. Filed upstream as
// **objectstack-ai/objectstack#14175** so the shape is DECLARED rather than
// discovered; until that lands this comment is the contract.
//
// ⛔ Do NOT add a tolerant `query.where ?? query` rung — not here, not in a
// test double. A consumer that accepts both shapes is precisely what let the
// wrong one ship green: `test/catalog-instantiate.test.ts`'s fake honoured the
// envelope, so 78 assertions passed against a shape production never produces.
// The test that can see this is one that dispatches through the REAL route and
// lets the runtime build its own facade — `test/catalog-engine-facade.test.ts`.

// ── Shapes ──────────────────────────────────────────────────────────────────

export interface CatalogApplyParams extends Record<string, unknown> {
Expand DownExpand Up@@ -256,12 +291,21 @@ export function pairKey(catalogItem: unknown, owner: unknown): string {
* exists on the platform. It is NOT read here — the issue names the
* assignment-level anchor, and adding a fallback rung is a product decision,
* reported rather than taken.)
*
* ⚠️ That tolerance is why this read's query shape matters more than the other
* three. The other reads fail into a visibly empty report — zero items, zero
* scanned — but this one fails into a state the handler is WRITTEN to accept:
* an envelope-shaped filter returned nothing, "nothing" reads as "not yet
* modelled", and every duty was created unanchored with no error anywhere. The
* rollups that the business unit exists to feed were simply empty. See the
* facade-shape note above; the end-to-end coverage is in
* `test/catalog-engine-facade.test.ts`.
*/
async function resolveBusinessUnit(
engine: ActionEngineFacade,
userId: string,
): Promise<string | undefined> {
const rows = await engine.find('sys_user_position', { where: { user_id: userId } });
const rows = await engine.find('sys_user_position', { user_id: userId });
for (const row of rows) {
// A person can hold several positions; take the first anchored one.
// Unanchored rows (`null`) are legacy/tenant-wide and carry no depth.
Expand All@@ -282,13 +326,14 @@ export const applyCatalogHandler: ActionHandler<CatalogApplyParams> = async (ctx
// stopped asking for; handing it to a new hire on their first day is the
// opposite of what deactivating it meant.
const items = await engine.find('duly_catalog_item', {
where: { position_code: positionCode, active: true },
position_code: positionCode,
active: true,
});
const activeItems = items.filter((item) => item?.active !== false);

// One probe for the whole run, not one per (item, user). The pair set is
// what makes a second apply create nothing.
const existing = await engine.find('duly_duty', { where: { owner: { $in: users } } });
const existing = await engine.find('duly_duty', { owner: { $in: users } });
const taken = new Set<string>();
for (const duty of existing) {
// Any duty already pointing at this catalog item for this person counts —
Expand DownExpand Up@@ -384,15 +429,15 @@ export const syncCatalogHandler: ActionHandler<CatalogSyncParams> = async (ctx)
// the retired report is made of, so it has to come back from this read.
const items = await engine.find(
'duly_catalog_item',
positionCode ? { where: { position_code: positionCode } } : {},
positionCode ? { position_code: positionCode } : {},
);
const byId = new Map<string, Record<string, unknown>>();
for (const item of items) {
if (positionCode && text(item?.position_code) !== positionCode) continue;
byId.set(recordId(item), item);
}

const duties = await engine.find('duly_duty', { where: { source: 'catalog' } });
const duties = await engine.find('duly_duty', { source: 'catalog' });

const changes: CatalogSyncChange[] = [];
const retired: CatalogSyncRetired[] = [];
Expand Down
90 changes: 22 additions & 68 deletions test/catalog-apply-cadence.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -23,7 +23,7 @@ import type { CatalogApplyResult } from '../src/actions/catalog.handlers.js';
* apply path unprotected is half a fix.
*
* ── Why a REAL booted engine and not `catalog-instantiate.test.ts`'s fake ──
* That suite's `FakeEngine` is a Map with a `where` matcher: it runs no
* That suite's `FakeEngine` is a Map with a filter matcher: it runs no
* validation rules and stamps no defaults, so every claim below would pass on
* it for the wrong reason. Validation and `applyFieldDefaults` are precisely
* what is under test here, so the handler is dispatched through the app's own
Expand DownExpand Up@@ -60,7 +60,7 @@ afterAll(async () => {
await kernel?.shutdown?.();
});

// ── The two engine facades ──────────────────────────────────────────────────
// ── The engine facade ───────────────────────────────────────────────────────

interface Facade {
insert(object: string, values: AnyRow): Promise<{ id: string }>;
Expand All@@ -70,15 +70,29 @@ interface Facade {
}

/**
* The facade `applyCatalogHandler` is WRITTEN against: `find(object, query)`
* takes ObjectQL's own query envelope, `where` and all. It is the convention
* `catalog-instantiate.test.ts`'s `FakeEngine` honours too, so this is the
* shape every existing assertion about the handler is made under.
* The facade the RUNTIME builds, reproduced line for line —
* `buildActionEngineFacade` in @objectstack/runtime 17.2.0:
*
* async find(object, query) {
* const where = query && Object.keys(query).length ? { where: query } : {};
* const rows = await ql.find(object, { ...where, context });
*
* `find(object, query)` therefore takes a BARE FILTER and wraps it here. This
* file used to carry TWO facades — this one honouring the handler's own
* `where` envelope, and a second reproducing the runtime — with a tripwire
* pinning the gap between them (#79). The handler now passes flat filters, so
* there is one convention and one facade.
*
* It is still a double, which is all it can be: the second test below has to
* put a row in front of the handler that `duly_catalog_item`'s own rules
* refuse, and only a hand-supplied facade can do that. What a double cannot do
* is prove the wire shape is right — it encodes the author's belief about it.
* That proof lives in `test/catalog-engine-facade.test.ts`, which dispatches
* through the real action route and lets the runtime build its own facade.
*
* `catalogItems`, when given, replaces the catalog read with rows handed
* straight to the handler (unfiltered — the handler re-applies its own
* `active` filter). That is how a row the object's own rules now REFUSE can
* still be put in front of the handler, which one test below needs.
* `active` filter).
*/
function handlerFacade(catalogItems?: AnyRow[]): Facade {
return {
Expand All@@ -94,28 +108,6 @@ function handlerFacade(catalogItems?: AnyRow[]): Facade {
},
find: async (object, query) => {
if (catalogItems && object === 'duly_catalog_item') return catalogItems.map((r) => ({ ...r }));
return data.find(object, query);
},
};
}

/**
* The facade the RUNTIME actually builds — `buildActionEngineFacade` in
* @objectstack/runtime 17.2.0, reproduced line for line:
*
* async find(object, query) {
* const where = query && Object.keys(query).length ? { where: query } : {};
* const rows = await ql.find(object, { ...where, context });
* ...
*
* It wraps whatever it is handed in a `where` of its own. Used by exactly one
* test, the tripwire at the bottom.
*/
function runtimeFacade(): Facade {
const base = handlerFacade();
return {
...base,
find: async (object, query) => {
const where = query && Object.keys(query).length ? { where: query } : {};
return data.find(object, { ...where });
},
Expand DownExpand Up@@ -227,41 +219,3 @@ describe('duly_catalog_apply — the cadence it replicates (#65)', () => {
for (const field of CADENCE_FIELDS) expect(duty?.[field] ?? null, field).toBeNull();
});
});

// ───────────────────────────────────────────────────────────────────────────
// A tripwire on a filed defect — NOT an assertion that this is correct
// ───────────────────────────────────────────────────────────────────────────
describe('the handler\'s query shape does not survive the runtime\'s own facade', () => {
/**
* Measured while covering the apply path for #65 and filed as #79 — a
* different defect from the missing validation rule, and not fixed here.
*
* `applyCatalogHandler` calls `engine.find('duly_catalog_item', { where: …
* })`. The runtime's `buildActionEngineFacade` wraps whatever it is given:
* `ql.find(object, { where: query })`. So through the real dispatcher the
* handler's own `where` becomes `{ where: { where: … } }`, no row has a
* field called `where`, and the read comes back EMPTY — with no error. The
* action then reports `{ created: 0 }` and a successful run.
*
* Pinned so the seam is visible rather than folklore. When #79 is fixed
* this goes red: delete this describe block — do not adjust it — and
* `handlerFacade` above becomes the only convention in the file.
*/
it('finds nothing, creates nothing, and reports success', async () => {
const position = 'apply_runtime_facade';
await insertItem({ position_code: position, form: 'recurring', frequency: 'weekly' });

// Same item, same params, the only difference being which facade.
const viaHandlerConvention = await apply(handlerFacade(), {
position_code: position,
users: ['u_f1'],
});
expect(viaHandlerConvention.catalog_items).toBe(1);
expect(viaHandlerConvention.created).toBe(1);

const viaRuntime = await apply(runtimeFacade(), { position_code: position, users: ['u_f2'] });
expect(viaRuntime.catalog_items).toBe(0);
expect(viaRuntime.created).toBe(0);
expect(await dutiesOf('u_f2')).toEqual([]);
});
});
Loading
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
55 changes: 50 additions & 5 deletions src/actions/catalog.handlers.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -110,6 +110,41 @@ export function resolveDutyTimezone(): string {
return DEFAULT_DUTY_TIMEZONE;
}

// ── The engine facade's query shape ───────────────────────────────────
//
// `ctx.engine.find(object, query)` takes a BARE FILTER, not an ObjectQL query
// envelope. The runtime builds the envelope itself — `buildActionEngineFacade`
// in @objectstack/runtime 17.2.0, read verbatim from its `dist/index.js`:
//
// async find(object, query) {
// const where = query && Object.keys(query).length ? { where: query } : {};
// const rows = await ql.find(object, { ...where, context });
//
// So every read in this file passes `{ field: value }`, never
// `{ where: { field: value } }`. Handing it an envelope produces
// `{ where: { where: { … } } }`; no row has a field called `where`, so the read
// comes back EMPTY WITH NO ERROR. That is the failure this file shipped with:
// `duly_catalog_apply` reported a successful run of zero, `duly_catalog_sync`
// scanned nothing and called every duty unchanged, and `resolveBusinessUnit`
// anchored no duty at all — silently, because "no position row" is a legitimate
// day-one state. The ONE unfiltered read survived, because
// `Object.keys({}).length === 0` skips the wrapping entirely, which is exactly
// what made the handler look partially alive.
//
// `ActionEngineFacade.find` in @objectstack/spec types `query` as a plain
// record of string to unknown and says nothing about which of the two shapes it
// is — the runtime's implementation is the only thing that decides, and this
// app read it the other way. Filed upstream as
// **objectstack-ai/objectstack#14175** so the shape is DECLARED rather than
// discovered; until that lands this comment is the contract.
//
// ⛔ Do NOT add a tolerant `query.where ?? query` rung — not here, not in a
// test double. A consumer that accepts both shapes is precisely what let the
// wrong one ship green: `test/catalog-instantiate.test.ts`'s fake honoured the
// envelope, so 78 assertions passed against a shape production never produces.
// The test that can see this is one that dispatches through the REAL route and
// lets the runtime build its own facade — `test/catalog-engine-facade.test.ts`.

// ── Shapes ──────────────────────────────────────────────────────────────────

export interface CatalogApplyParams extends Record<string, unknown> {
Expand DownExpand Up@@ -256,12 +291,21 @@ export function pairKey(catalogItem: unknown, owner: unknown): string {
* exists on the platform. It is NOT read here — the issue names the
* assignment-level anchor, and adding a fallback rung is a product decision,
* reported rather than taken.)
*
* ⚠️ That tolerance is why this read's query shape matters more than the other
* three. The other reads fail into a visibly empty report — zero items, zero
* scanned — but this one fails into a state the handler is WRITTEN to accept:
* an envelope-shaped filter returned nothing, "nothing" reads as "not yet
* modelled", and every duty was created unanchored with no error anywhere. The
* rollups that the business unit exists to feed were simply empty. See the
* facade-shape note above; the end-to-end coverage is in
* `test/catalog-engine-facade.test.ts`.
*/
async function resolveBusinessUnit(
engine: ActionEngineFacade,
userId: string,
): Promise<string | undefined> {
const rows = await engine.find('sys_user_position', { where: { user_id: userId } });
const rows = await engine.find('sys_user_position', { user_id: userId });
for (const row of rows) {
// A person can hold several positions; take the first anchored one.
// Unanchored rows (`null`) are legacy/tenant-wide and carry no depth.
Expand All@@ -282,13 +326,14 @@ export const applyCatalogHandler: ActionHandler<CatalogApplyParams> = async (ctx
// stopped asking for; handing it to a new hire on their first day is the
// opposite of what deactivating it meant.
const items = await engine.find('duly_catalog_item', {
where: { position_code: positionCode, active: true },
position_code: positionCode,
active: true,
});
const activeItems = items.filter((item) => item?.active !== false);

// One probe for the whole run, not one per (item, user). The pair set is
// what makes a second apply create nothing.
const existing = await engine.find('duly_duty', { where: { owner: { $in: users } } });
const existing = await engine.find('duly_duty', { owner: { $in: users } });
const taken = new Set<string>();
for (const duty of existing) {
// Any duty already pointing at this catalog item for this person counts —
Expand DownExpand Up@@ -384,15 +429,15 @@ export const syncCatalogHandler: ActionHandler<CatalogSyncParams> = async (ctx)
// the retired report is made of, so it has to come back from this read.
const items = await engine.find(
'duly_catalog_item',
positionCode ? { where: { position_code: positionCode } } : {},
positionCode ? { position_code: positionCode } : {},
);
const byId = new Map<string, Record<string, unknown>>();
for (const item of items) {
if (positionCode && text(item?.position_code) !== positionCode) continue;
byId.set(recordId(item), item);
}

const duties = await engine.find('duly_duty', { where: { source: 'catalog' } });
const duties = await engine.find('duly_duty', { source: 'catalog' });

const changes: CatalogSyncChange[] = [];
const retired: CatalogSyncRetired[] = [];
Expand Down
90 changes: 22 additions & 68 deletions test/catalog-apply-cadence.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -23,7 +23,7 @@ import type { CatalogApplyResult } from '../src/actions/catalog.handlers.js';
* apply path unprotected is half a fix.
*
* ── Why a REAL booted engine and not `catalog-instantiate.test.ts`'s fake ──
* That suite's `FakeEngine` is a Map with a `where` matcher: it runs no
* That suite's `FakeEngine` is a Map with a filter matcher: it runs no
* validation rules and stamps no defaults, so every claim below would pass on
* it for the wrong reason. Validation and `applyFieldDefaults` are precisely
* what is under test here, so the handler is dispatched through the app's own
Expand DownExpand Up@@ -60,7 +60,7 @@ afterAll(async () => {
await kernel?.shutdown?.();
});

// ── The two engine facades ──────────────────────────────────────────────────
// ── The engine facade ───────────────────────────────────────────────────────

interface Facade {
insert(object: string, values: AnyRow): Promise<{ id: string }>;
Expand All@@ -70,15 +70,29 @@ interface Facade {
}

/**
* The facade `applyCatalogHandler` is WRITTEN against: `find(object, query)`
* takes ObjectQL's own query envelope, `where` and all. It is the convention
* `catalog-instantiate.test.ts`'s `FakeEngine` honours too, so this is the
* shape every existing assertion about the handler is made under.
* The facade the RUNTIME builds, reproduced line for line —
* `buildActionEngineFacade` in @objectstack/runtime 17.2.0:
*
* async find(object, query) {
* const where = query && Object.keys(query).length ? { where: query } : {};
* const rows = await ql.find(object, { ...where, context });
*
* `find(object, query)` therefore takes a BARE FILTER and wraps it here. This
* file used to carry TWO facades — this one honouring the handler's own
* `where` envelope, and a second reproducing the runtime — with a tripwire
* pinning the gap between them (#79). The handler now passes flat filters, so
* there is one convention and one facade.
*
* It is still a double, which is all it can be: the second test below has to
* put a row in front of the handler that `duly_catalog_item`'s own rules
* refuse, and only a hand-supplied facade can do that. What a double cannot do
* is prove the wire shape is right — it encodes the author's belief about it.
* That proof lives in `test/catalog-engine-facade.test.ts`, which dispatches
* through the real action route and lets the runtime build its own facade.
*
* `catalogItems`, when given, replaces the catalog read with rows handed
* straight to the handler (unfiltered — the handler re-applies its own
* `active` filter). That is how a row the object's own rules now REFUSE can
* still be put in front of the handler, which one test below needs.
* `active` filter).
*/
function handlerFacade(catalogItems?: AnyRow[]): Facade {
return {
Expand All@@ -94,28 +108,6 @@ function handlerFacade(catalogItems?: AnyRow[]): Facade {
},
find: async (object, query) => {
if (catalogItems && object === 'duly_catalog_item') return catalogItems.map((r) => ({ ...r }));
return data.find(object, query);
},
};
}

/**
* The facade the RUNTIME actually builds — `buildActionEngineFacade` in
* @objectstack/runtime 17.2.0, reproduced line for line:
*
* async find(object, query) {
* const where = query && Object.keys(query).length ? { where: query } : {};
* const rows = await ql.find(object, { ...where, context });
* ...
*
* It wraps whatever it is handed in a `where` of its own. Used by exactly one
* test, the tripwire at the bottom.
*/
function runtimeFacade(): Facade {
const base = handlerFacade();
return {
...base,
find: async (object, query) => {
const where = query && Object.keys(query).length ? { where: query } : {};
return data.find(object, { ...where });
},
Expand DownExpand Up@@ -227,41 +219,3 @@ describe('duly_catalog_apply — the cadence it replicates (#65)', () => {
for (const field of CADENCE_FIELDS) expect(duty?.[field] ?? null, field).toBeNull();
});
});

// ───────────────────────────────────────────────────────────────────────────
// A tripwire on a filed defect — NOT an assertion that this is correct
// ───────────────────────────────────────────────────────────────────────────
describe('the handler\'s query shape does not survive the runtime\'s own facade', () => {
/**
* Measured while covering the apply path for #65 and filed as #79 — a
* different defect from the missing validation rule, and not fixed here.
*
* `applyCatalogHandler` calls `engine.find('duly_catalog_item', { where: …
* })`. The runtime's `buildActionEngineFacade` wraps whatever it is given:
* `ql.find(object, { where: query })`. So through the real dispatcher the
* handler's own `where` becomes `{ where: { where: … } }`, no row has a
* field called `where`, and the read comes back EMPTY — with no error. The
* action then reports `{ created: 0 }` and a successful run.
*
* Pinned so the seam is visible rather than folklore. When #79 is fixed
* this goes red: delete this describe block — do not adjust it — and
* `handlerFacade` above becomes the only convention in the file.
*/
it('finds nothing, creates nothing, and reports success', async () => {
const position = 'apply_runtime_facade';
await insertItem({ position_code: position, form: 'recurring', frequency: 'weekly' });

// Same item, same params, the only difference being which facade.
const viaHandlerConvention = await apply(handlerFacade(), {
position_code: position,
users: ['u_f1'],
});
expect(viaHandlerConvention.catalog_items).toBe(1);
expect(viaHandlerConvention.created).toBe(1);

const viaRuntime = await apply(runtimeFacade(), { position_code: position, users: ['u_f2'] });
expect(viaRuntime.catalog_items).toBe(0);
expect(viaRuntime.created).toBe(0);
expect(await dutiesOf('u_f2')).toEqual([]);
});
});
Loading
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
55 changes: 50 additions & 5 deletions src/actions/catalog.handlers.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -110,6 +110,41 @@ export function resolveDutyTimezone(): string {
return DEFAULT_DUTY_TIMEZONE;
}

// ── The engine facade's query shape ───────────────────────────────────
//
// `ctx.engine.find(object, query)` takes a BARE FILTER, not an ObjectQL query
// envelope. The runtime builds the envelope itself — `buildActionEngineFacade`
// in @objectstack/runtime 17.2.0, read verbatim from its `dist/index.js`:
//
// async find(object, query) {
// const where = query && Object.keys(query).length ? { where: query } : {};
// const rows = await ql.find(object, { ...where, context });
//
// So every read in this file passes `{ field: value }`, never
// `{ where: { field: value } }`. Handing it an envelope produces
// `{ where: { where: { … } } }`; no row has a field called `where`, so the read
// comes back EMPTY WITH NO ERROR. That is the failure this file shipped with:
// `duly_catalog_apply` reported a successful run of zero, `duly_catalog_sync`
// scanned nothing and called every duty unchanged, and `resolveBusinessUnit`
// anchored no duty at all — silently, because "no position row" is a legitimate
// day-one state. The ONE unfiltered read survived, because
// `Object.keys({}).length === 0` skips the wrapping entirely, which is exactly
// what made the handler look partially alive.
//
// `ActionEngineFacade.find` in @objectstack/spec types `query` as a plain
// record of string to unknown and says nothing about which of the two shapes it
// is — the runtime's implementation is the only thing that decides, and this
// app read it the other way. Filed upstream as
// **objectstack-ai/objectstack#14175** so the shape is DECLARED rather than
// discovered; until that lands this comment is the contract.
//
// ⛔ Do NOT add a tolerant `query.where ?? query` rung — not here, not in a
// test double. A consumer that accepts both shapes is precisely what let the
// wrong one ship green: `test/catalog-instantiate.test.ts`'s fake honoured the
// envelope, so 78 assertions passed against a shape production never produces.
// The test that can see this is one that dispatches through the REAL route and
// lets the runtime build its own facade — `test/catalog-engine-facade.test.ts`.

// ── Shapes ──────────────────────────────────────────────────────────────────

export interface CatalogApplyParams extends Record<string, unknown> {
Expand DownExpand Up@@ -256,12 +291,21 @@ export function pairKey(catalogItem: unknown, owner: unknown): string {
* exists on the platform. It is NOT read here — the issue names the
* assignment-level anchor, and adding a fallback rung is a product decision,
* reported rather than taken.)
*
* ⚠️ That tolerance is why this read's query shape matters more than the other
* three. The other reads fail into a visibly empty report — zero items, zero
* scanned — but this one fails into a state the handler is WRITTEN to accept:
* an envelope-shaped filter returned nothing, "nothing" reads as "not yet
* modelled", and every duty was created unanchored with no error anywhere. The
* rollups that the business unit exists to feed were simply empty. See the
* facade-shape note above; the end-to-end coverage is in
* `test/catalog-engine-facade.test.ts`.
*/
async function resolveBusinessUnit(
engine: ActionEngineFacade,
userId: string,
): Promise<string | undefined> {
const rows = await engine.find('sys_user_position', { where: { user_id: userId } });
const rows = await engine.find('sys_user_position', { user_id: userId });
for (const row of rows) {
// A person can hold several positions; take the first anchored one.
// Unanchored rows (`null`) are legacy/tenant-wide and carry no depth.
Expand All@@ -282,13 +326,14 @@ export const applyCatalogHandler: ActionHandler<CatalogApplyParams> = async (ctx
// stopped asking for; handing it to a new hire on their first day is the
// opposite of what deactivating it meant.
const items = await engine.find('duly_catalog_item', {
where: { position_code: positionCode, active: true },
position_code: positionCode,
active: true,
});
const activeItems = items.filter((item) => item?.active !== false);

// One probe for the whole run, not one per (item, user). The pair set is
// what makes a second apply create nothing.
const existing = await engine.find('duly_duty', { where: { owner: { $in: users } } });
const existing = await engine.find('duly_duty', { owner: { $in: users } });
const taken = new Set<string>();
for (const duty of existing) {
// Any duty already pointing at this catalog item for this person counts —
Expand DownExpand Up@@ -384,15 +429,15 @@ export const syncCatalogHandler: ActionHandler<CatalogSyncParams> = async (ctx)
// the retired report is made of, so it has to come back from this read.
const items = await engine.find(
'duly_catalog_item',
positionCode ? { where: { position_code: positionCode } } : {},
positionCode ? { position_code: positionCode } : {},
);
const byId = new Map<string, Record<string, unknown>>();
for (const item of items) {
if (positionCode && text(item?.position_code) !== positionCode) continue;
byId.set(recordId(item), item);
}

const duties = await engine.find('duly_duty', { where: { source: 'catalog' } });
const duties = await engine.find('duly_duty', { source: 'catalog' });

const changes: CatalogSyncChange[] = [];
const retired: CatalogSyncRetired[] = [];
Expand Down
90 changes: 22 additions & 68 deletions test/catalog-apply-cadence.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -23,7 +23,7 @@ import type { CatalogApplyResult } from '../src/actions/catalog.handlers.js';
* apply path unprotected is half a fix.
*
* ── Why a REAL booted engine and not `catalog-instantiate.test.ts`'s fake ──
* That suite's `FakeEngine` is a Map with a `where` matcher: it runs no
* That suite's `FakeEngine` is a Map with a filter matcher: it runs no
* validation rules and stamps no defaults, so every claim below would pass on
* it for the wrong reason. Validation and `applyFieldDefaults` are precisely
* what is under test here, so the handler is dispatched through the app's own
Expand DownExpand Up@@ -60,7 +60,7 @@ afterAll(async () => {
await kernel?.shutdown?.();
});

// ── The two engine facades ──────────────────────────────────────────────────
// ── The engine facade ───────────────────────────────────────────────────────

interface Facade {
insert(object: string, values: AnyRow): Promise<{ id: string }>;
Expand All@@ -70,15 +70,29 @@ interface Facade {
}

/**
* The facade `applyCatalogHandler` is WRITTEN against: `find(object, query)`
* takes ObjectQL's own query envelope, `where` and all. It is the convention
* `catalog-instantiate.test.ts`'s `FakeEngine` honours too, so this is the
* shape every existing assertion about the handler is made under.
* The facade the RUNTIME builds, reproduced line for line —
* `buildActionEngineFacade` in @objectstack/runtime 17.2.0:
*
* async find(object, query) {
* const where = query && Object.keys(query).length ? { where: query } : {};
* const rows = await ql.find(object, { ...where, context });
*
* `find(object, query)` therefore takes a BARE FILTER and wraps it here. This
* file used to carry TWO facades — this one honouring the handler's own
* `where` envelope, and a second reproducing the runtime — with a tripwire
* pinning the gap between them (#79). The handler now passes flat filters, so
* there is one convention and one facade.
*
* It is still a double, which is all it can be: the second test below has to
* put a row in front of the handler that `duly_catalog_item`'s own rules
* refuse, and only a hand-supplied facade can do that. What a double cannot do
* is prove the wire shape is right — it encodes the author's belief about it.
* That proof lives in `test/catalog-engine-facade.test.ts`, which dispatches
* through the real action route and lets the runtime build its own facade.
*
* `catalogItems`, when given, replaces the catalog read with rows handed
* straight to the handler (unfiltered — the handler re-applies its own
* `active` filter). That is how a row the object's own rules now REFUSE can
* still be put in front of the handler, which one test below needs.
* `active` filter).
*/
function handlerFacade(catalogItems?: AnyRow[]): Facade {
return {
Expand All@@ -94,28 +108,6 @@ function handlerFacade(catalogItems?: AnyRow[]): Facade {
},
find: async (object, query) => {
if (catalogItems && object === 'duly_catalog_item') return catalogItems.map((r) => ({ ...r }));
return data.find(object, query);
},
};
}

/**
* The facade the RUNTIME actually builds — `buildActionEngineFacade` in
* @objectstack/runtime 17.2.0, reproduced line for line:
*
* async find(object, query) {
* const where = query && Object.keys(query).length ? { where: query } : {};
* const rows = await ql.find(object, { ...where, context });
* ...
*
* It wraps whatever it is handed in a `where` of its own. Used by exactly one
* test, the tripwire at the bottom.
*/
function runtimeFacade(): Facade {
const base = handlerFacade();
return {
...base,
find: async (object, query) => {
const where = query && Object.keys(query).length ? { where: query } : {};
return data.find(object, { ...where });
},
Expand DownExpand Up@@ -227,41 +219,3 @@ describe('duly_catalog_apply — the cadence it replicates (#65)', () => {
for (const field of CADENCE_FIELDS) expect(duty?.[field] ?? null, field).toBeNull();
});
});

// ───────────────────────────────────────────────────────────────────────────
// A tripwire on a filed defect — NOT an assertion that this is correct
// ───────────────────────────────────────────────────────────────────────────
describe('the handler\'s query shape does not survive the runtime\'s own facade', () => {
/**
* Measured while covering the apply path for #65 and filed as #79 — a
* different defect from the missing validation rule, and not fixed here.
*
* `applyCatalogHandler` calls `engine.find('duly_catalog_item', { where: …
* })`. The runtime's `buildActionEngineFacade` wraps whatever it is given:
* `ql.find(object, { where: query })`. So through the real dispatcher the
* handler's own `where` becomes `{ where: { where: … } }`, no row has a
* field called `where`, and the read comes back EMPTY — with no error. The
* action then reports `{ created: 0 }` and a successful run.
*
* Pinned so the seam is visible rather than folklore. When #79 is fixed
* this goes red: delete this describe block — do not adjust it — and
* `handlerFacade` above becomes the only convention in the file.
*/
it('finds nothing, creates nothing, and reports success', async () => {
const position = 'apply_runtime_facade';
await insertItem({ position_code: position, form: 'recurring', frequency: 'weekly' });

// Same item, same params, the only difference being which facade.
const viaHandlerConvention = await apply(handlerFacade(), {
position_code: position,
users: ['u_f1'],
});
expect(viaHandlerConvention.catalog_items).toBe(1);
expect(viaHandlerConvention.created).toBe(1);

const viaRuntime = await apply(runtimeFacade(), { position_code: position, users: ['u_f2'] });
expect(viaRuntime.catalog_items).toBe(0);
expect(viaRuntime.created).toBe(0);
expect(await dutiesOf('u_f2')).toEqual([]);
});
});
Loading
Loading