test(objectql): update the two condition-gate pins fail loud invalidates (#4775)

`hook-binder.test.ts` pinned the OLD uncompilable behaviour and explicitly
accepted either outcome ("ignored at compile time (handler runs) or evaluated
false (skipped) … just assert we didn't crash"). That latitude was the defect:
"condition ignored" deleted the gate, so a hook declared to run conditionally
ran on every write. It now asserts the rejection, the `uncompilable` reason and
that the handler never ran.

`hook-metrics.test.ts` used a BARE `name == "skipme"` condition. Hook
conditions are `record`-scoped, so the bare identifier resolved to nothing and
the expression faulted on every call — the "skip" the test asserted came from
the old swallow (fault → warn → false), not from a condition that answered NO.
This is precisely the never-actually-enforced condition #4775 was expected to
flush out. Rewritten as `record.name == "skipme"`, which expresses the same
intent and is actually evaluated.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Br2xsJsczFsTR9bvbh2Ny

Author: claude

packages/objectql/src/hook-binder.test.ts

@@ -3,7 +3,7 @@
 import { describe, it, expect, vi } from 'vitest';
 import { ObjectQL } from './engine.js';
 import { bindHooksToEngine } from './hook-binder.js';
-import { wrapDeclarativeHook } from './hook-wrappers.js';
+import { wrapDeclarativeHook, HookConditionError } from './hook-wrappers.js';
 import type { Hook, HookContext } from '@objectstack/spec/data';
 
 function makeEngine() {
@@ -377,21 +377,33 @@ describe('wrapDeclarativeHook', () => {
     expect(calls).toEqual(['done']); // awaited despite async=true
   });
 
-  it('logs and treats invalid condition formulas as skipping', async () => {
-    const warn = vi.fn();
+  it('rejects the operation when the condition formula does not compile', async () => {
+    // [#4775] This test used to accept EITHER outcome ("ignored at compile time
+    // (handler runs) or evaluated false (skipped) … just assert we didn't
+    // crash"). That latitude was the defect: "condition ignored" DELETED the
+    // gate, so a hook declared to run conditionally ran on every write, and the
+    // only trace was a `warn`. A condition that cannot compile can never be
+    // evaluated, so the hook can neither run nor be skipped honestly — the
+    // operation is rejected instead, naming the hook.
+    const error = vi.fn();
     const calls: string[] = [];
     const meta: Hook = {
       name: 'badcond', object: 'a', events: ['beforeInsert'], priority: 100,
       condition: '(((not valid syntax',
       handler: () => { calls.push('ran'); },
     };
     const wrapped = wrapDeclarativeHook(meta, meta.handler as any, {
-      logger: { debug: () => {}, info: () => {}, warn, error: () => {} },
+      logger: { debug: () => {}, info: () => {}, warn: () => {}, error },
     });
-    await wrapped(makeCtx());
-    expect(warn).toHaveBeenCalled();
-    // Either ignored at compile time (handler runs) or evaluated false
-    // (skipped). Both are valid; just assert we didn't crash.
-    expect(calls.length === 0 || calls[0] === 'ran').toBe(true);
+
+    const err = await wrapped(makeCtx()).then(() => null, (e) => e);
+
+    expect(err).toBeInstanceOf(HookConditionError);
+    expect(err.reason).toBe('uncompilable');
+    expect(err.message).toContain("Hook 'badcond'");
+    expect(calls).toEqual([]);
+    // Reported at bind time too, at error level — an operator sees the broken
+    // hook before the first write trips over it.
+    expect(error).toHaveBeenCalled();
   });
 });

packages/objectql/src/hook-metrics.test.ts

@@ -82,7 +82,13 @@ describe('hook metrics', () => {
             object: 'account',
             events: ['beforeInsert'],
             priority: 100,
-            condition: 'name == "skipme"',
+            // [#4775] Was a bare `name == "skipme"`. Hook conditions are
+            // `record`-scoped, so the bare identifier resolved to nothing and
+            // the expression FAULTED on every call — the "skip" this test
+            // asserted came from the old swallow (fault → warn → false), not
+            // from a condition that answered NO. `record.name` is the same
+            // intent, actually evaluated: 'acme' != 'skipme' ⇒ FALSE ⇒ skip.
+            condition: 'record.name == "skipme"',
             handler: async () => { /* noop */ },
         };
         bindHooksToEngine(engine, [hook], { packageId: 'p', metrics });