feat(objectql,runtime): 默认 runner setter 改为 first-wins —— 私有字段探测全数退场 (#4251)

.changeset/runner-setters-first-wins.md

@@ -0,0 +1,32 @@
+---
+"@objectstack/objectql": minor
+"@objectstack/runtime": patch
+---
+
+feat(objectql,runtime): the default-runner setters are first-wins, and the private-field probes that used to enforce that are gone (#4251)
+
+`setDefaultBodyRunner` / `setDefaultActionRunner` now enforce their own
+documented contract — "the runtime layer sets this once per engine" — by
+keeping the first runner and returning `false` for any later call. Public
+accessors `getDefaultBodyRunner()` / `getDefaultActionRunner()` join them, and
+the fields become real `private` members instead of `(this as any)` attachments.
+
+Before this, the invariant lived in the CALLERS: AppPlugin probed the engine's
+private `_defaultBodyRunner` / `_defaultActionRunner` fields through `any` to
+avoid clobbering another AppPlugin's runner on a shared kernel — an invariant
+owned by every caller and enforced by none, and a private reach that a field
+rename would have broken silently (the guard reads `undefined`, every AppPlugin
+reinstalls). The engine's own `bindHooks` fallback and ObjectQLPlugin's
+authored-action re-sync read the same fields the same way. All three read the
+public accessors now; the only remaining `_default*` mentions in the repo are
+comments and test doubles.
+
+Caller audit before the semantics change: every setter call site either owns a
+fresh engine (the sandbox and hook-binder tests) or wants exactly
+keep-the-first (AppPlugin) — nobody replaces a runner on a live engine. Return
+type `void` → `boolean` is additive; AppPlugin uses it to keep its "Installed
+default … runner" log truthful (skipped when the engine kept an earlier one).
+
+Pinned in hook-binder tests: second install refused end-to-end (the first
+runner is the one that executes) and the accessors expose exactly what was
+kept.

packages/objectql/src/engine.ts

@@ -690,8 +690,8 @@ export class ObjectQL implements IDataEngine {
     metrics?: any;
   }): void {
     const merged = { ...(opts ?? {}), logger: this.logger } as any;
-    if (!merged.bodyRunner && (this as any)._defaultBodyRunner) {
-      merged.bodyRunner = (this as any)._defaultBodyRunner;
+    if (!merged.bodyRunner && this._defaultBodyRunner) {
+      merged.bodyRunner = this._defaultBodyRunner;
     }
     if (merged.strict === undefined && (this as any)._strictHookBinding) {
       merged.strict = true;
@@ -705,14 +705,41 @@ export class ObjectQL implements IDataEngine {
     bindHooksToEngine(this, hooks, merged);
   }
 
+  /** Default hook body-runner — see {@link setDefaultBodyRunner}. */
+  private _defaultBodyRunner?: any;
+  /** Default action body-runner factory — see {@link setDefaultActionRunner}. */
+  private _defaultActionRunner?: (actionDef: any) => ((ctx: any) => Promise<unknown>) | undefined;
+
   /**
    * Install a default body-runner used when `bindHooks` is called without
    * an explicit one. The runtime layer sets this once on each per-project
    * engine so every binding path (template seed, metadata sync, AppPlugin)
    * can execute hook `body.source` consistently.
+   *
+   * FIRST-WINS (#4251): "set once per engine" is this method's own contract,
+   * so the method enforces it — a second call is ignored and returns `false`.
+   * Callers used to implement the guard themselves by probing the private
+   * `_defaultBodyRunner` field through `any` (multiple AppPlugin instances on
+   * one kernel must not clobber each other's runner), which meant the
+   * invariant lived in every caller and belonged to none. Nobody replaces a
+   * runner on a live engine: every setter call site either owns a fresh
+   * engine or wants exactly this keep-the-first behaviour.
+   *
+   * @returns `true` when this call installed the runner, `false` when one was
+   * already present (kept unchanged).
    */
-  setDefaultBodyRunner(runner: any): void {
-    (this as any)._defaultBodyRunner = runner;
+  setDefaultBodyRunner(runner: any): boolean {
+    if (this._defaultBodyRunner) {
+      this.logger.debug('Default body runner already installed — keeping the first');
+      return false;
+    }
+    this._defaultBodyRunner = runner;
+    return true;
+  }
+
+  /** The installed default body-runner, if any — the public read the first-wins guard implies. */
+  getDefaultBodyRunner(): any {
+    return this._defaultBodyRunner;
   }
 
   /**
@@ -724,9 +751,25 @@ export class ObjectQL implements IDataEngine {
    * `body` into an executable `registerAction` handler. The factory returns
    * `undefined` for actions it cannot run (no `body`, invalid shape), which
    * callers must treat as "skip", not an error.
+   *
+   * FIRST-WINS (#4251) — same contract and rationale as
+   * {@link setDefaultBodyRunner}.
+   *
+   * @returns `true` when this call installed the runner, `false` when one was
+   * already present (kept unchanged).
    */
-  setDefaultActionRunner(runner: (actionDef: any) => ((ctx: any) => Promise<unknown>) | undefined): void {
-    (this as any)._defaultActionRunner = runner;
+  setDefaultActionRunner(runner: (actionDef: any) => ((ctx: any) => Promise<unknown>) | undefined): boolean {
+    if (this._defaultActionRunner) {
+      this.logger.debug('Default action runner already installed — keeping the first');
+      return false;
+    }
+    this._defaultActionRunner = runner;
+    return true;
+  }
+
+  /** The installed default action-runner factory, if any. */
+  getDefaultActionRunner(): ((actionDef: any) => ((ctx: any) => Promise<unknown>) | undefined) | undefined {
+    return this._defaultActionRunner;
   }
 
   /**

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

@@ -227,6 +227,35 @@ describe('bindHooksToEngine', () => {
     expect(calls).toEqual(['body-default-runner']);
   });
 
+  it('keeps the FIRST default body runner — the setter is first-wins (#4251)', async () => {
+    const engine = makeEngine();
+    const calls: string[] = [];
+    expect(engine.setDefaultBodyRunner(() => async () => { calls.push('first'); })).toBe(true);
+    // A second install (another AppPlugin sharing the kernel) is refused —
+    // the invariant used to be enforced by callers probing the private
+    // `_defaultBodyRunner` field; it is the engine's own now.
+    expect(engine.setDefaultBodyRunner(() => async () => { calls.push('second'); })).toBe(false);
+    const hook: Hook = {
+      name: 'body-first-wins', object: 'account', events: ['beforeInsert'], priority: 100,
+      body: { language: 'js', source: 'ctx.input.x = 1;' },
+    } as unknown as Hook;
+    engine.bindHooks([hook], { packageId: 'app:first-wins' });
+    await engine.triggerHooks('beforeInsert', makeCtx());
+    // Execution proves the first runner stayed installed.
+    expect(calls).toEqual(['first']);
+  });
+
+  it('keeps the FIRST default action runner and exposes both via public accessors (#4251)', () => {
+    const engine = makeEngine();
+    const first = () => undefined;
+    const second = () => undefined;
+    expect(engine.setDefaultActionRunner(first)).toBe(true);
+    expect(engine.setDefaultActionRunner(second)).toBe(false);
+    // The accessors are the public read the old private-field probes become.
+    expect(engine.getDefaultActionRunner()).toBe(first);
+    expect(engine.getDefaultBodyRunner()).toBeUndefined();
+  });
+
   it('explicit bodyRunner wins over the engine default', async () => {
     const engine = makeEngine();
     const calls: string[] = [];

packages/objectql/src/hook-binder.ts

@@ -259,7 +259,9 @@ function resolveHandler(
   if (body && typeof body === 'object') {
     let runner = opts.bodyRunner;
     if (typeof runner !== 'function') {
-      const fallback = (engine as any)?._defaultBodyRunner;
+      // [#4251] The public accessor — this read used to reach the private
+      // `_defaultBodyRunner` field through `as any`.
+      const fallback = engine?.getDefaultBodyRunner?.();
       if (typeof fallback === 'function') runner = fallback;
     }
     if (typeof runner !== 'function') {

packages/objectql/src/plugin-authored-actions.test.ts

@@ -30,8 +30,13 @@ function makeQlMock(overrides: AnyRecord = {}) {
       getArtifactItem: vi.fn(() => undefined),
     },
     // Default action runner as the runtime installs it: body → handler.
+    // The plugin reads it through `getDefaultActionRunner()` (#4251); the
+    // field stays so tests can swap or clear the runner directly.
     _defaultActionRunner: vi.fn((action: AnyRecord) =>
       action?.body ? async () => `ran:${action.name}` : undefined),
+    getDefaultActionRunner(): unknown {
+      return (this as AnyRecord)._defaultActionRunner;
+    },
     ...overrides,
   };
 }

packages/objectql/src/plugin.ts

@@ -1665,7 +1665,12 @@ export class ObjectQLPlugin implements Plugin {
     // deleting the last authored action must unregister it.
     ql.removeActionsByPackage('metadata-service');
 
-    const runner: any = ql._defaultActionRunner;
+    // [#4251] The public accessor — this read used to reach the private
+    // `_defaultActionRunner` field directly. Probed because `ql` can be a
+    // test double that predates the accessor.
+    const runner: any = typeof ql.getDefaultActionRunner === 'function'
+      ? ql.getDefaultActionRunner()
+      : undefined;
     let registered = 0;
     let skippedNoHandler = 0;
     for (const action of bindable) {

packages/runtime/src/app-plugin.test.ts

@@ -436,11 +436,19 @@ describe('AppPlugin', () => {
             expect(mockManifest.register).not.toHaveBeenCalled();
         });
 
-        it('does not replace an already-installed default runner', async () => {
-            mockQL._defaultBodyRunner = () => undefined;
+        it('delegates keep-the-first to the engine — the setter is called unconditionally (#4251)', async () => {
+            // The caller-side guard (probing the engine's private
+            // `_defaultBodyRunner` field) is gone; the ENGINE's first-wins
+            // setter owns the invariant, pinned in objectql's hook-binder
+            // tests. This side only skips the "Installed" log when the engine
+            // answers `false`.
+            mockQL.setDefaultBodyRunner = vi.fn(() => false); // engine: already installed
             const plugin = new AppPlugin({ id: 'com.test.app' });
             await plugin.init(mockContext);
-            expect(mockQL.setDefaultBodyRunner).not.toHaveBeenCalled();
+            expect(mockQL.setDefaultBodyRunner).toHaveBeenCalledTimes(1);
+            expect(vi.mocked(mockContext.logger.info).mock.calls.map((c) => c[0])).not.toContain(
+                '[AppPlugin] Installed default hook body runner (runtime-authored hooks can execute)',
+            );
         });
 
         it('honours the OS_DISABLE_AUTHORED_HOOKS=1 opt-out', async () => {

packages/runtime/src/app-plugin.ts

@@ -55,26 +55,22 @@ interface AppEngineSurface {
     registerAction(objectKey: string, name: string, handler: unknown, packageId: string): void;
     registerDriver(driver: unknown): void;
     setDatasourceMapping(rules: unknown): void;
-    setDefaultBodyRunner(runner: unknown): void;
+    /**
+     * FIRST-WINS setters (#4251): the engine keeps the first runner and
+     * returns whether this call installed it. The idempotence guard used to
+     * live HERE, as a probe of the engine's private `_defaultBodyRunner` /
+     * `_defaultActionRunner` fields through this surface — an invariant owned
+     * by every caller and enforced by none. It is the engine's now.
+     * `boolean | void` because pre-first-wins engines (and bare `vi.fn()`
+     * doubles) return undefined — treated as installed.
+     */
+    setDefaultBodyRunner(runner: unknown): boolean | void;
     setDefaultActionRunner(
         runner: (actionDef: any) => ((ctx: any) => Promise<unknown>) | undefined,
-    ): void;
+    ): boolean | void;
     setHookMetricsRecorder(recorder: unknown): void;
     getHookMetricsRecorder(): any;
     readonly registry?: { setInitialDisabledPackageIds?: (ids: Iterable<string>) => void };
-    /**
-     * The idempotence guards for the two runners above, read directly.
-     *
-     * These are NOT part of any declared API: the engine attaches them with
-     * `(this as any)._defaultBodyRunner = runner` and publishes no reader, so
-     * "has another AppPlugin already installed one?" has no answer except
-     * reaching for the field. Declared here to keep that reach visible instead
-     * of laundering it through `any` — the fix is a public accessor on the
-     * engine beside `getHookMetricsRecorder`, which already exists for exactly
-     * this question about the metrics recorder. Filed on #4251.
-     */
-    _defaultBodyRunner?: unknown;
-    _defaultActionRunner?: unknown;
 }
 
 /** The engine as AppPlugin uses it: the data contract plus those seams. */
@@ -326,13 +322,17 @@ export class AppPlugin implements Plugin {
             return; // no engine on this kernel — nothing to wire
         }
         if (!ql || typeof ql.setDefaultBodyRunner !== 'function') return;
-        if (ql._defaultBodyRunner) return; // another AppPlugin already installed one
-        ql.setDefaultBodyRunner(hookBodyRunnerFactory(new QuickJSScriptRunner(), {
+        // [#4251] The setter is first-wins — the engine keeps the first runner
+        // when several AppPlugins share one kernel; this caller no longer
+        // probes the engine's private `_defaultBodyRunner` field to find out.
+        const installed = ql.setDefaultBodyRunner(hookBodyRunnerFactory(new QuickJSScriptRunner(), {
             ql,
             logger: ctx.logger,
             appId: 'runtime-authored',
         }));
-        ctx.logger.info('[AppPlugin] Installed default hook body runner (runtime-authored hooks can execute)');
+        if (installed !== false) {
+            ctx.logger.info('[AppPlugin] Installed default hook body runner (runtime-authored hooks can execute)');
+        }
     }
 
     /**
@@ -364,13 +364,15 @@ export class AppPlugin implements Plugin {
             return; // no engine on this kernel — nothing to wire
         }
         if (!ql || typeof ql.setDefaultActionRunner !== 'function') return;
-        if (ql._defaultActionRunner) return; // another AppPlugin already installed one
-        ql.setDefaultActionRunner(actionBodyRunnerFactory(new QuickJSScriptRunner(), {
+        // [#4251] First-wins setter — same as the hook body runner above.
+        const installed = ql.setDefaultActionRunner(actionBodyRunnerFactory(new QuickJSScriptRunner(), {
             ql,
             logger: ctx.logger,
             appId: 'runtime-authored',
         }));
-        ctx.logger.info('[AppPlugin] Installed default action body runner (runtime-authored actions can execute)');
+        if (installed !== false) {
+            ctx.logger.info('[AppPlugin] Installed default action body runner (runtime-authored actions can execute)');
+        }
     }
 
     /**