fix(hooks): query by where, not filter, on ctx.api reads

.changeset/hook-query-where-not-filter.md

@@ -0,0 +1,44 @@
+---
+'hotcrm': patch
+---
+
+Stop hook reads from silently querying the wrong record. Seventeen hook-side
+`ctx.api` calls — across eight `*.hook.ts` files and the shared
+`_line-item-price-fill.ts` — passed their predicate as `filter:`, a key the
+ObjectQL kernel does not accept on every read path, and drops without raising:
+
+- `find` normalizes `filter` → `where`, so those calls were correct by luck.
+- `findOne` spreads the query into the AST (`{ ...query, limit: 1 }`) and never
+  aliases, so the predicate vanished and the call returned **the object's first
+  row** — for any argument, including one matching nothing. It never returned
+  `null` and never threw.
+- `count` reads `query.where` explicitly, so the predicate vanished and the call
+  counted **the entire object**.
+
+Confirmed against a real ObjectQL 16.1.0 engine, not the test harness. The
+mis-reads were load-bearing: line-item pricing defaulted `list_price` /
+`unit_price` from the first product in the catalog rather than the chosen one;
+quote acceptance and won-opportunity / contract-activation account promotion
+evaluated their "already in the target state?" gate against an unrelated record
+while writing to the correct one; case escalation assigned the follow-up task to
+the first account's owner; the account and product delete guards counted every
+row in the object, so they blocked deletes that had no real references and
+reported an invented number; and campaign ROI snapshots recorded whole-table
+opportunity and lead counts as campaign attribution.
+
+`HookQuery` no longer declares `filter?`, so the compiler now rejects the
+spelling at authoring time instead of leaving it to be discovered in the data.
+
+The suite could not have caught this: `test/helpers/hook-harness.ts` resolved
+its predicate as `q.filter ?? q.where ?? {}`, making the stand-in more
+permissive than the kernel it stands in for, so every affected hook tested
+green. The harness now throws on `filter`, and `test/hook-query-predicate.test.ts`
+pins the contract against a real in-memory kernel — including the kernel's
+silent-drop behaviour, so the trap stays documented — plus a source scan over
+`src/objects/**.ts` that fails any file reintroducing the key.
+
+That scan deliberately covers the whole directory rather than the `*.hook.ts`
+glob: merging `main` brought in the extraction of the two price-fill hooks into
+`_line-item-price-fill.ts`, which carried the `filter:` along with it. A guard
+keyed to a filename convention would have missed the one instance most likely to
+survive a refactor.

src/actions/lead.actions.ts

@@ -115,7 +115,7 @@ export const CreateCampaignAction: Action = {
       // duplicates are skipped, and re-running the action on the same
       // selection must not double-count marketing touches.
       const raw = await ctx.api.object('crm_campaign_member').find({
-        filter: { crm_campaign: campaignId },
+        where: { crm_campaign: campaignId },
         fields: ['crm_lead'],
         top: 5000,
       });

src/objects/_hook-api.ts

@@ -16,9 +16,27 @@
 
 type Doc = Record<string, unknown>;
 
-/** Query options accepted by read operations. Drivers accept `filter` or `where`. */
+/**
+ * Query options accepted by read operations.
+ *
+ * The predicate key is `where` — and ONLY `where`. An earlier version of this
+ * type also allowed `filter`, on the belief that "drivers accept either". They
+ * do not, and the mismatch fails SILENTLY:
+ *
+ *   - `find`   normalizes `filter` → `where`, so it happens to work.
+ *   - `findOne` spreads the query straight into the AST (`{...query, limit: 1}`)
+ *     and never aliases. An unknown `filter` key is dropped by the driver, so
+ *     the query degrades to "first row of the object" — no error, no null, just
+ *     the wrong record.
+ *   - `count`  reads `query.where` explicitly, so `filter` is dropped and the
+ *     call counts the WHOLE object.
+ *
+ * `filter` is therefore deliberately absent: a hook that reaches for it must
+ * fail at compile time rather than silently compute against a stranger's row.
+ * (See `test/hook-query-predicate.test.ts`, which pins this against the real
+ * kernel rather than the test harness.)
+ */
 export interface HookQuery {
-  filter?: Doc;
   where?: Doc;
   fields?: string[];
   top?: number;

src/objects/_line-item-price-fill.ts

@@ -43,7 +43,7 @@ export function createLineItemPriceFill(objectName: string, hookName: string): H
       const productId = typeof input.crm_product === 'string' ? input.crm_product : undefined;
       if (!productId) return;
       const product = await api.object('crm_product').findOne({
-        filter: { id: productId }, fields: ['id', 'list_price'],
+        where: { id: productId }, fields: ['id', 'list_price'],
       });
       const listPrice = product && typeof product.list_price === 'number' ? product.list_price : undefined;
       if (listPrice === undefined) return;

src/objects/account.hook.ts

@@ -51,7 +51,7 @@ const accountHook: Hook = {
       const api = ctx.api as HookApi | undefined;
       if (!api) return;
       const openOpps = await api.object('crm_opportunity').count({
-        filter: {
+        where: {
           crm_account: previous.id,
           stage: { $nin: ['closed_won', 'closed_lost'] },
         },

src/objects/campaign.hook.ts

@@ -66,14 +66,14 @@ const campaignCompleted: Hook = {
     // metric snapshot was silently zero.
     const [memberRows, opportunities, wonOpps, wonOppRecords] = await Promise.all([
       api.object('crm_campaign_member').find({
-        filter: { crm_campaign: id },
+        where: { crm_campaign: id },
         fields: ['crm_lead', 'status'],
         top: 5000,
       }),
-      api.object('crm_opportunity').count({ filter: { crm_campaign: id } }),
-      api.object('crm_opportunity').count({ filter: { crm_campaign: id, stage: 'closed_won' } }),
+      api.object('crm_opportunity').count({ where: { crm_campaign: id } }),
+      api.object('crm_opportunity').count({ where: { crm_campaign: id, stage: 'closed_won' } }),
       api.object('crm_opportunity').find({
-        filter: { crm_campaign: id, stage: 'closed_won' },
+        where: { crm_campaign: id, stage: 'closed_won' },
         fields: ['amount'],
         top: 5000,
       }),
@@ -90,7 +90,7 @@ const campaignCompleted: Hook = {
     );
     const convertedLeads =
       leadIds.length > 0
-        ? await api.object('crm_lead').count({ filter: { id: { $in: leadIds }, is_converted: true } })
+        ? await api.object('crm_lead').count({ where: { id: { $in: leadIds }, is_converted: true } })
         : 0;
 
     const actualRevenue = wonOppRecords.reduce((sum, row) => {

src/objects/case.hook.ts

@@ -111,7 +111,7 @@ const caseSideEffects: Hook = {
 
     // Escalation: open task for account owner
     if (input.status === 'escalated' && previous.status !== 'escalated' && accountId) {
-      const account = await api.object('crm_account').findOne({ filter: { id: accountId } });
+      const account = await api.object('crm_account').findOne({ where: { id: accountId } });
       const ownerId = (account as { owner?: string } | null)?.owner ?? ctx.user?.id;
       const due = new Date();
       due.setDate(due.getDate() + 1);

src/objects/contract.hook.ts

@@ -106,7 +106,7 @@ const contractActivation: Hook = {
     }
 
     if (accountId) {
-      const account = await api.object('crm_account').findOne({ filter: { id: accountId } });
+      const account = await api.object('crm_account').findOne({ where: { id: accountId } });
       if (account && account.type !== 'customer') {
         await api.object('crm_account').update(accountId, { type: 'customer' });
       }

src/objects/opportunity.hook.ts

@@ -166,7 +166,7 @@ const opportunityWonHook: Hook = {
       undefined;
     if (!accountId) return;
 
-    const account = await api.object('crm_account').findOne({ filter: { id: accountId } });
+    const account = await api.object('crm_account').findOne({ where: { id: accountId } });
     if (account && account.type !== 'customer') {
       await api.object('crm_account').update(accountId, { type: 'customer' });
     }

src/objects/opportunity_line_item.hook.ts

@@ -67,13 +67,13 @@ const opportunityAmountRollup: Hook = {
       // editing a line item would silently rewrite a closed-won deal's recorded
       // amount. A closed deal's value is settled; leave it.
       const opp = await api.object('crm_opportunity').findOne({
-        filter: { id: oppId }, fields: ['id', 'stage'],
+        where: { id: oppId }, fields: ['id', 'stage'],
       });
       const stage = opp && typeof opp.stage === 'string' ? opp.stage : undefined;
       if (stage === 'closed_won' || stage === 'closed_lost') continue;
 
       const lines = await api.object('crm_opportunity_line_item').find({
-        filter: { crm_opportunity: oppId },
+        where: { crm_opportunity: oppId },
         fields: ['quantity', 'unit_price', 'discount'],
         top: 5000,
       });

src/objects/product.hook.ts

@@ -51,8 +51,8 @@ const productHook: Hook = {
       const id = previous?.id;
       if (!api || !id) return;
       const [oppRefs, quoteRefs] = await Promise.all([
-        api.object('crm_opportunity_line_item').count({ filter: { crm_product: id } }).catch(() => 0),
-        api.object('crm_quote_line_item').count({ filter: { crm_product: id } }).catch(() => 0),
+        api.object('crm_opportunity_line_item').count({ where: { crm_product: id } }).catch(() => 0),
+        api.object('crm_quote_line_item').count({ where: { crm_product: id } }).catch(() => 0),
       ]);
       const total = oppRefs + quoteRefs;
       if (total > 0) {

src/objects/quote.hook.ts

@@ -128,7 +128,7 @@ const quoteAccepted: Hook = {
     });
 
     if (opportunityId) {
-      const opp = await api.object('crm_opportunity').findOne({ filter: { id: opportunityId } });
+      const opp = await api.object('crm_opportunity').findOne({ where: { id: opportunityId } });
       if (opp && opp.stage !== 'closed_won' && opp.stage !== 'closed_lost') {
         await api.object('crm_opportunity').update(opportunityId, {
           stage: 'closed_won',

src/objects/quote_line_item.hook.ts

@@ -64,15 +64,15 @@ const quoteTotalRollup: Hook = {
 
     for (const quoteId of quoteIds) {
       const quote = await api.object('crm_quote').findOne({
-        filter: { id: quoteId },
+        where: { id: quoteId },
         fields: ['id', 'status', 'discount', 'tax', 'shipping_handling'],
       });
       if (!quote) continue;
       const status = typeof quote.status === 'string' ? quote.status : undefined;
       if (status === 'accepted' || status === 'expired') continue;
 
       const lines = await api.object('crm_quote_line_item').find({
-        filter: { crm_quote: quoteId },
+        where: { crm_quote: quoteId },
         fields: ['quantity', 'unit_price', 'discount'],
         top: 5000,
       });

test/helpers/hook-harness.ts

@@ -84,7 +84,26 @@ export function makeHarness(store: Record<string, Rec[]> = {}): Harness {
   const calls: RecordedCall[] = [];
   let seq = 0;
   const rows = (object: string): Rec[] => (store[object] ??= []);
-  const whereOf = (q: HookQuery = {}): Rec => (q.filter ?? q.where ?? {}) as Rec;
+  /**
+   * Extract the predicate — from `where`, and ONLY from `where`.
+   *
+   * This used to read `q.filter ?? q.where ?? {}`, which made the harness
+   * MORE permissive than the kernel and turned it into a liar: the real
+   * `findOne` ignores an unknown `filter` key and returns the object's first
+   * row, and the real `count` ignores it and counts everything, but every test
+   * here went green because the harness quietly honoured both spellings. A
+   * fake replacement that accepts inputs the real thing rejects cannot prove
+   * anything — so `filter` is now a loud failure, not a silent alias.
+   */
+  const whereOf = (q: HookQuery = {}): Rec => {
+    if ('filter' in (q as Rec)) {
+      throw new Error(
+        "hook-harness: query key \"filter\" is not a predicate — the kernel silently " +
+          'ignores it on findOne/count and reads the wrong record. Use "where".',
+      );
+    }
+    return (q.where ?? {}) as Rec;
+  };
 
   const api: HookApi = {
     object(name: string) {