Skip to content

Commit 004ea11

Browse files
hotlongclaude
andauthored
refactor(hooks): 摘掉最后三处 hook 上下文的 doc / previousDoc 防御性 alias 读 (#5906) (#6269)
#5671(PR #5908)清掉 trigger-record-change 那条 `input.doc` alias 后,全仓 grep 剩下的三处同族兜底一并收官。三条限支都在为一个从不存在的生产者兜底,任何引擎路径 都不可达,行为零变化: - `service-storage` 附件生命周期:`ctx.result ?? ctx.input.doc ?? ctx.input.data` —— 注意 `doc` 排在 `data` 前面,但恒为 undefined,`??` 每次都穿到 `data`。 - `plugin-sharing` primary-BU 投影:`(ctx.input.data ?? ctx.input.doc).user_id`。 - `runtime` hook 沙箱:`engineCtx.input ?? engineCtx.doc` 与 `engineCtx.previous ?? engineCtx.previousDoc` —— 读的是顶层键,同一族上一层。 真值:ObjectQL 的写事件上下文一律把载荷拼在 `data` 上,由 objectql 的 `hook-input-shape-contract.test.ts` 钉住(「insert carries `data` — never `doc`」, #5273 / PR #5668);顶层那对同理 —— `HookContextSchema` 只声明 `input` / `result` / `previous`,`engine.ts` 作为 HookContext 的唯一生产者两个都不构造。 依据消失后按 PD #12 删掉,而不是继续养成第二套事实契约。 承重 pin(各自「正向 + 反向」一对,反向那条才承重 —— 真值键都在读取链首位, alias 限支复活时正向用例仍绿): - `attachment-lifecycle.test.ts`:既有 insert fixture 同时给了 `result`,链首命中, 对本次清理不承重;另配「无 `result` 只有 `input.data`」的正向用例,以及 「只拼 `input.doc`」的反向用例(限支复活即红)。 - `body-runner.test.ts`:新增一对,反向用例喂一个只拼 `doc`/`previousDoc` 的上下文, 断言沙箱看到空 input、无 previous。 另:`hook-input-shape-contract.test.ts` 文件头那句「trigger-record-change still carries a defensive `input.doc` alias read — filed separately, not fixed here」在 #5908 合入后已成假话,改写为收官表述(仅改注释,不动该包任何生产代码)。 Claude-Session: https://claude.ai/code/session_015a5qkLzpGXhLL2F5gvJ7dD Co-authored-by: Claude <noreply@anthropic.com>
1 parent cca11e9 commit 004ea11

7 files changed

Lines changed: 147 additions & 7 deletions

File tree

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

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

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

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

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

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

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,52 @@ describe('hookBodyRunnerFactory', () => {
156156
await fn!(engineCtx);
157157
expect(backing.website).toBe('https://acme.com');
158158
});
159+
160+
// [#5906] `input` and `previous` are the engine's own spellings, and the only
161+
// ones: `HookContextSchema` declares no top-level `doc`/`previousDoc`, and
162+
// objectql's `engine.ts` — the sole producer of a HookContext — builds neither.
163+
// Alias limbs for both used to sit in `buildSandboxContext`; these two pin that
164+
// the sandbox now seeds from the truth keys and from nothing else. The NEGATIVE
165+
// one carries the weight: the truth keys sit FIRST in both reads, so the
166+
// positive case would stay green if either alias limb were put back.
167+
describe('seeds ctx.input / ctx.previous from the engine keys only', () => {
168+
/** A `ql` whose single write records what the body observed. */
169+
const probingQl = (seen: Array<Record<string, unknown>>) => ({
170+
object: () => ({ insert: async (data: any) => { seen.push(data); return data; } }),
171+
});
172+
173+
const probeHook = (seen: Array<Record<string, unknown>>) =>
174+
hookBodyRunnerFactory(runner, { ql: probingQl(seen), appId: 'crm' })({
175+
name: 'probe',
176+
object: 'contact',
177+
events: ['beforeUpdate'],
178+
body: {
179+
language: 'js',
180+
source:
181+
"await ctx.api.object('probe').insert({"
182+
+ ' input: JSON.stringify(ctx.input),'
183+
+ ' previous: JSON.stringify(ctx.previous ?? null) });',
184+
capabilities: ['api.write'],
185+
},
186+
} as any);
187+
188+
it('reads `input` and `previous`', async () => {
189+
const seen: Array<Record<string, unknown>> = [];
190+
await probeHook(seen)!({ input: { email: 'new@x.io' }, previous: { email: 'old@x.io' } } as any);
191+
expect(seen[0]).toEqual({
192+
input: '{"email":"new@x.io"}',
193+
previous: '{"email":"old@x.io"}',
194+
});
195+
});
196+
197+
it('does NOT read a `doc` / `previousDoc` alias — no engine path produces either', async () => {
198+
const seen: Array<Record<string, unknown>> = [];
199+
// The spellings the deleted limbs defended, and nothing else on the context:
200+
// with them unread the body sees an empty input and no previous at all.
201+
await probeHook(seen)!({ doc: { email: 'new@x.io' }, previousDoc: { email: 'old@x.io' } } as any);
202+
expect(seen[0]).toEqual({ input: '{}', previous: 'null' });
203+
});
204+
});
159205
});
160206

161207
describe('actionBodyRunnerFactory', () => {

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

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -305,8 +305,14 @@ function buildSandboxApi(engineCtx: any, ql: any, errLabel: string) {
305305
}
306306

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

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

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,52 @@ describe('installAttachmentLifecycleHooks — tombstoning', () => {
167167
expect(engine.updates[0].data).toMatchObject({ id: 'f1', status: 'committed', deleted_at: null });
168168
});
169169

170+
// [#5906] `input.data` is the ONE key an insert payload arrives under — measured
171+
// on the real engine by objectql's `hook-input-shape-contract.test.ts` ("insert
172+
// carries `data` — never `doc`", #5273). The fixture above cannot pin that: it
173+
// supplies `result`, which sits FIRST in the handler's read, so it stays green
174+
// whatever the limbs below it spell. These two carry the weight instead, and the
175+
// NEGATIVE one is the load-bearing half — it goes red the moment the deleted
176+
// `input.doc` alias limb is put back (that limb sat ahead of `data`, so a
177+
// `doc`-only context would be read again).
178+
const tombstonedFile = () => ({
179+
id: 'f1',
180+
key: 'attachments/f1.bin',
181+
scope: 'attachments',
182+
status: 'deleted',
183+
deleted_at: '2026-01-01T00:00:00Z',
184+
});
185+
186+
it('un-tombstones from input.data when the context carries no result', async () => {
187+
const engine = fakeEngine({ attachments: [], files: [tombstonedFile()] });
188+
installAttachmentLifecycleHooks(engine, silentLogger());
189+
190+
await engine.trigger('afterInsert', {
191+
object: 'sys_attachment',
192+
event: 'afterInsert',
193+
input: { data: { file_id: 'f1' } },
194+
});
195+
196+
expect(engine.updates).toHaveLength(1);
197+
expect(engine.updates[0].data).toMatchObject({ id: 'f1', status: 'committed', deleted_at: null });
198+
});
199+
200+
it('does NOT read an `input.doc` alias — no engine path produces that key', async () => {
201+
const engine = fakeEngine({ attachments: [], files: [tombstonedFile()] });
202+
installAttachmentLifecycleHooks(engine, silentLogger());
203+
204+
await engine.trigger('afterInsert', {
205+
object: 'sys_attachment',
206+
event: 'afterInsert',
207+
// The spelling the deleted limb defended. With it gone the handler finds no
208+
// `file_id` at all, so the tombstone stands.
209+
input: { doc: { file_id: 'f1' } },
210+
});
211+
212+
expect(engine.updates).toHaveLength(0);
213+
expect(engine.tables.sys_file[0].status).toBe('deleted');
214+
});
215+
170216
it('a failing lookup never blocks the delete (best-effort)', async () => {
171217
const engine = fakeEngine({ attachments: [], files: [] });
172218
engine.findOne = async () => {

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

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

0 commit comments

Comments
 (0)