refactor(hooks): 摘掉最后三处 hook 上下文的 doc / previousDoc 防御性 alias 读 (#5906)

.changeset/hook-ctx-doc-alias-reads-removed.md

@@ -0,0 +1,25 @@
+---
+'@objectstack/service-storage': patch
+'@objectstack/plugin-sharing': patch
+'@objectstack/runtime': patch
+---
+
+hooks: drop the last three `doc` / `previousDoc` alias reads on a hook context — read the engine's own keys only
+
+Behaviour is unchanged: every one of these limbs guarded against a producer that
+has never existed, so none of them could be reached.
+
+- `service-storage` attachment lifecycle read `ctx.result ?? ctx.input.doc ?? ctx.input.data`
+- `plugin-sharing` primary-BU projection read `(ctx.input.data ?? ctx.input.doc).user_id`
+- `runtime`'s hook sandbox read `engineCtx.input ?? engineCtx.doc` and `engineCtx.previous ?? engineCtx.previousDoc`
+
+Every ObjectQL write context spells the payload `data` — measured and pinned by
+`hook-input-shape-contract.test.ts` in `@objectstack/objectql` ("insert carries
+`data` — never `doc`", #5273). The top-level pair is the same family one level
+up: `HookContextSchema` declares `input` / `result` / `previous` and neither a
+`doc` nor a `previousDoc`, and `engine.ts` — the sole producer of a HookContext
+— builds neither. The limbs survived only because the old `HookContext.input`
+contract table documented insert as `{ doc, options }`; that table was corrected
+in #5668, and the same alias was removed from `trigger-record-change` in #5671.
+These are the remainder (#5906), removed rather than left as a second de-facto
+contract (PD #12).

packages/objectql/src/hook-input-shape-contract.test.ts

@@ -19,9 +19,14 @@
  *     filters bind the driver call where no handler can widen them. So the one
  *     field the docs pointed at resolved `undefined`.
  *   - `insert` was documented as `{ doc: Record, ... }`; the engine builds
- *     `{ data: row, ... }`. (`trigger-record-change` still carries a defensive
- *     `input.doc` alias read for that reason — filed separately, not fixed
- *     here.)
+ *     `{ data: row, ... }`. That wrong sentence was the whole reason consumers
+ *     carried defensive `input.doc` alias reads — branches guarding against a
+ *     producer that has never existed. All of them are now gone:
+ *     `trigger-record-change` in #5671, and the last three — service-storage's
+ *     attachment lifecycle, plugin-sharing's primary-BU projection, and the
+ *     runtime hook sandbox (which aliased the top-level `doc`/`previousDoc` of
+ *     the same family) — in #5906. No consumer defends against this key today,
+ *     so the assertions below are what keeps it that way.
  *
  * The table was also silent about #5038: since ADR-0058's bulk-write addendum
  * the `after*` events on a bulk write fire PER MATCHED ROW on a

packages/plugins/plugin-sharing/src/primary-bu-projection.ts

@@ -77,7 +77,12 @@ function collectUserIds(ctx: any): string[] {
   const add = (v: unknown) => { if (v != null && v !== '') ids.add(String(v)); };
   add(ctx?.result?.user_id);
   add(ctx?.previous?.user_id);
-  add((ctx?.input?.data ?? ctx?.input?.doc)?.user_id);
+  // `input.data` is the ONE key a write hook's payload arrives under — measured
+  // and pinned by objectql's `hook-input-shape-contract.test.ts` ("insert carries
+  // `data` — never `doc`", #5273). An `input.doc` alias limb sat below this read
+  // for a producer that never existed; removed in #5906 (same family as #5671)
+  // rather than left as a second de-facto contract (PD #12).
+  add(ctx?.input?.data?.user_id);
   add(ctx?.[STASH_KEY]);
   return [...ids];
 }

packages/runtime/src/sandbox/body-runner.test.ts

@@ -156,6 +156,52 @@ describe('hookBodyRunnerFactory', () => {
     await fn!(engineCtx);
     expect(backing.website).toBe('https://acme.com');
   });
+
+  // [#5906] `input` and `previous` are the engine's own spellings, and the only
+  // ones: `HookContextSchema` declares no top-level `doc`/`previousDoc`, and
+  // objectql's `engine.ts` — the sole producer of a HookContext — builds neither.
+  // Alias limbs for both used to sit in `buildSandboxContext`; these two pin that
+  // the sandbox now seeds from the truth keys and from nothing else. The NEGATIVE
+  // one carries the weight: the truth keys sit FIRST in both reads, so the
+  // positive case would stay green if either alias limb were put back.
+  describe('seeds ctx.input / ctx.previous from the engine keys only', () => {
+    /** A `ql` whose single write records what the body observed. */
+    const probingQl = (seen: Array<Record<string, unknown>>) => ({
+      object: () => ({ insert: async (data: any) => { seen.push(data); return data; } }),
+    });
+
+    const probeHook = (seen: Array<Record<string, unknown>>) =>
+      hookBodyRunnerFactory(runner, { ql: probingQl(seen), appId: 'crm' })({
+        name: 'probe',
+        object: 'contact',
+        events: ['beforeUpdate'],
+        body: {
+          language: 'js',
+          source:
+            "await ctx.api.object('probe').insert({"
+            + ' input: JSON.stringify(ctx.input),'
+            + ' previous: JSON.stringify(ctx.previous ?? null) });',
+          capabilities: ['api.write'],
+        },
+      } as any);
+
+    it('reads `input` and `previous`', async () => {
+      const seen: Array<Record<string, unknown>> = [];
+      await probeHook(seen)!({ input: { email: 'new@x.io' }, previous: { email: 'old@x.io' } } as any);
+      expect(seen[0]).toEqual({
+        input: '{"email":"new@x.io"}',
+        previous: '{"email":"old@x.io"}',
+      });
+    });
+
+    it('does NOT read a `doc` / `previousDoc` alias — no engine path produces either', async () => {
+      const seen: Array<Record<string, unknown>> = [];
+      // The spellings the deleted limbs defended, and nothing else on the context:
+      // with them unread the body sees an empty input and no previous at all.
+      await probeHook(seen)!({ doc: { email: 'new@x.io' }, previousDoc: { email: 'old@x.io' } } as any);
+      expect(seen[0]).toEqual({ input: '{}', previous: 'null' });
+    });
+  });
 });
 
 describe('actionBodyRunnerFactory', () => {

packages/runtime/src/sandbox/body-runner.ts

@@ -305,8 +305,14 @@ function buildSandboxApi(engineCtx: any, ql: any, errLabel: string) {
 }
 
 function buildSandboxContext(engineCtx: any, ql: any): ScriptContext {
-  const inputSnapshot = unwrapProxyToPlain(engineCtx?.input ?? engineCtx?.doc);
-  const previousRaw = engineCtx?.previous ?? engineCtx?.previousDoc;
+  // `input` and `previous` are the engine's own spellings, and the only ones:
+  // `HookContextSchema` (`packages/spec/src/data/hook.zod.ts`) declares neither a
+  // top-level `doc` nor a `previousDoc`, and objectql's `engine.ts` — the sole
+  // producer of a HookContext — builds neither. Alias limbs for both sat here for
+  // producers that never existed; removed in #5906 (same family as #5671) rather
+  // than left as a second de-facto contract (PD #12).
+  const inputSnapshot = unwrapProxyToPlain(engineCtx?.input);
+  const previousRaw = engineCtx?.previous;
   return {
     input: inputSnapshot ?? {},
     // Preserve `undefined` for `previous` on insert events so hooks can

packages/services/service-storage/src/attachment-lifecycle.test.ts

@@ -167,6 +167,52 @@ describe('installAttachmentLifecycleHooks — tombstoning', () => {
     expect(engine.updates[0].data).toMatchObject({ id: 'f1', status: 'committed', deleted_at: null });
   });
 
+  // [#5906] `input.data` is the ONE key an insert payload arrives under — measured
+  // on the real engine by objectql's `hook-input-shape-contract.test.ts` ("insert
+  // carries `data` — never `doc`", #5273). The fixture above cannot pin that: it
+  // supplies `result`, which sits FIRST in the handler's read, so it stays green
+  // whatever the limbs below it spell. These two carry the weight instead, and the
+  // NEGATIVE one is the load-bearing half — it goes red the moment the deleted
+  // `input.doc` alias limb is put back (that limb sat ahead of `data`, so a
+  // `doc`-only context would be read again).
+  const tombstonedFile = () => ({
+    id: 'f1',
+    key: 'attachments/f1.bin',
+    scope: 'attachments',
+    status: 'deleted',
+    deleted_at: '2026-01-01T00:00:00Z',
+  });
+
+  it('un-tombstones from input.data when the context carries no result', async () => {
+    const engine = fakeEngine({ attachments: [], files: [tombstonedFile()] });
+    installAttachmentLifecycleHooks(engine, silentLogger());
+
+    await engine.trigger('afterInsert', {
+      object: 'sys_attachment',
+      event: 'afterInsert',
+      input: { data: { file_id: 'f1' } },
+    });
+
+    expect(engine.updates).toHaveLength(1);
+    expect(engine.updates[0].data).toMatchObject({ id: 'f1', status: 'committed', deleted_at: null });
+  });
+
+  it('does NOT read an `input.doc` alias — no engine path produces that key', async () => {
+    const engine = fakeEngine({ attachments: [], files: [tombstonedFile()] });
+    installAttachmentLifecycleHooks(engine, silentLogger());
+
+    await engine.trigger('afterInsert', {
+      object: 'sys_attachment',
+      event: 'afterInsert',
+      // The spelling the deleted limb defended. With it gone the handler finds no
+      // `file_id` at all, so the tombstone stands.
+      input: { doc: { file_id: 'f1' } },
+    });
+
+    expect(engine.updates).toHaveLength(0);
+    expect(engine.tables.sys_file[0].status).toBe('deleted');
+  });
+
   it('a failing lookup never blocks the delete (best-effort)', async () => {
     const engine = fakeEngine({ attachments: [], files: [] });
     engine.findOne = async () => {

packages/services/service-storage/src/attachment-lifecycle.ts

@@ -171,7 +171,14 @@ export function installAttachmentLifecycleHooks(
     'afterInsert',
     async (ctx: any) => {
       try {
-        const row: any = ctx?.result ?? ctx?.input?.doc ?? ctx?.input?.data;
+        // An after-insert context carries the stored row on `ctx.result`, and the
+        // written payload under `input.data` — `data` is the ONLY spelling any
+        // engine path produces, measured and pinned by objectql's
+        // `hook-input-shape-contract.test.ts` ("insert carries `data` — never
+        // `doc`", #5273). An `input.doc` alias limb used to sit between these two
+        // for a producer that never existed; removed in #5906 (same family as
+        // #5671) rather than left as a second de-facto contract (PD #12).
+        const row: any = ctx?.result ?? ctx?.input?.data;
         const fileId = row?.file_id;
         if (!fileId) return;
         const file = await engine.findOne('sys_file', { where: { id: String(fileId) }, context: { ...SYSTEM_CTX } });