Skip to content

Commit a07d1ff

Browse files
committed
fix(runtime): per-request kernel lives on the request, not on the dispatcher (#5155)
One HttpDispatcher serves a whole host, but the kernel a request resolves to is per request. That answer was stored on the instance field `this.kernel`, written once per request by `resolveRequestScope()` and read by every service lookup afterwards — each behind at least one `await`. Two interleaved requests on a multi-tenant host therefore swapped data sources under each other: A resolved env-1, yielded, B resolved env-2, and A resumed reading env-2. `HttpProtocolContext` now carries `kernel`, written by `resolveRequestScope()` next to the `environmentId` / `dataDriver` / `executionContext` it already writes there. `this.kernel` is gone. Every kernel-reading member of `DomainHandlerDeps` / `ActionExecutionDeps` takes the request as its first parameter, so the dependency is visible at the call site and the compiler asks for it — chosen over AsyncLocalStorage, which would have reintroduced implicit mutable ambient context, the same defect in a new costume. Three host-level readers (`/ready`, its driver-health probe, the memoized `default-project` lookup) now name `defaultKernel` explicitly instead of reading whichever tenant resolved most recently. Covered by a deterministic interleaving regression test: request A parks inside its own identity resolution, request B runs to completion, A resumes. On the old code A is served env-2's i18n bundle. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VkPSGsX9o17MsGv3Lbxu2w
1 parent 2f6516e commit a07d1ff

26 files changed

Lines changed: 608 additions & 179 deletions
Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
---
2+
"@objectstack/runtime": minor
3+
---
4+
5+
fix(runtime): the HTTP dispatcher serves each request from its OWN resolved kernel — two tenants can no longer swap data sources under each other (#5155)
6+
7+
A host constructs exactly **one** `HttpDispatcher` (`dispatcher-plugin.ts`
8+
`start()`), and every route it serves shares that instance. The kernel a request
9+
resolves to, however, is per request: on a multi-tenant host the injected
10+
`kernelResolver` (ADR-0006) picks a different one per environment.
11+
12+
That per-request answer was being stored on a dispatcher **instance field**,
13+
`this.kernel`, written once per request by `resolveRequestScope()` and then read
14+
by `resolveService()` / `getService()` / `getObjectQL()` /
15+
`getRequestKernelService()` / `announceKernelEvent()` / `getRegisteredAiRoutes()`
16+
— every one of them behind at least one `await`. Node's single thread is no
17+
protection here: what it protects is code that does **not** hold mutable shared
18+
state across an `await`, and this held it across several.
19+
20+
So two interleaved requests on two environments produced this:
21+
22+
1. request A resolves, `this.kernel` = env-1's kernel;
23+
2. A yields at an `await` (session lookup, driver query);
24+
3. request B resolves, `this.kernel` = env-2's kernel;
25+
4. A resumes and resolves `objectql` / `metadata` / `automation` off **env-2**.
26+
27+
One tenant's request reading another tenant's data source — a correctness and
28+
isolation defect, not a performance one. Single-environment deployments were
29+
never affected (`this.kernel === defaultKernel` always, so the write was
30+
idempotent), which is exactly why no local run or CI job ever showed it. It is
31+
now covered by a deterministic interleaving regression test
32+
(`http-dispatcher.multi-tenant-concurrency.test.ts`), which fails on the old
33+
code with request A being served env-2's data.
34+
35+
**The fix: the resolved kernel travels on the request, and every facility that
36+
reads a kernel takes the request explicitly.** `HttpProtocolContext` gains a
37+
`kernel` field, written by `resolveRequestScope()` alongside the
38+
`environmentId` / `dataDriver` / `executionContext` it already writes there.
39+
There is no longer any `this.kernel` to rewrite. An `AsyncLocalStorage` carrier
40+
was deliberately **not** used: it would have reintroduced implicit mutable
41+
ambient context, which is the shape of this bug in a new costume.
42+
43+
Three host-level readers moved to the host kernel explicitly, where they had
44+
been reading whichever tenant resolved most recently: `/ready` (readiness is a
45+
property of the replica), its driver-health probe, and the memoized
46+
single-environment `default-project` lookup.
47+
48+
**Migration — `DomainHandlerDeps` and `ActionExecutionDeps`.** Every
49+
kernel-reading member now takes the request as its **first** parameter. If you
50+
implement or call either contract (both are exported from
51+
`@objectstack/runtime`; nothing in this monorepo or the sibling distributions
52+
did):
53+
54+
- `deps.resolveService(name, envId)` becomes `deps.resolveService(context, name, envId)`
55+
- `deps.getService(name)` becomes `deps.getService(context, name)`
56+
- `deps.getObjectQL(envId)` becomes `deps.getObjectQL(context, envId)`
57+
- `deps.getRequestKernelService(name)` becomes `deps.getRequestKernelService(context, name)`
58+
- `deps.announceKernelEvent(event, payload)` becomes `deps.announceKernelEvent(context, event, payload)`
59+
- `deps.getRegisteredAiRoutes()` becomes `deps.getRegisteredAiRoutes(context)`
60+
61+
`context` is the `HttpProtocolContext` the domain handler already receives. The
62+
same rule applies to the `action-execution` helpers, which take it right after
63+
`deps`: `callData`, `resolveAutomationService`, `dispatchFlowAction`,
64+
`invokeBusinessAction`, `resolveRouteActionDeclaration`.
65+
66+
`HttpDispatcher.getDiscoveryInfo(prefix)` gains an **optional** second argument,
67+
the request context. Callers that serve `/discovery` straight off the host (the
68+
adapters, the dispatcher plugin) need no change and now describe the host kernel
69+
deterministically instead of whichever tenant asked last.
70+
71+
`resolveProjectKernelObjectQL(context)` keeps its direct-caller kernel swap;
72+
the swap is now written onto that context, so it stays visible to the rest of
73+
that request and to nothing else.

packages/runtime/src/action-body-identity.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,7 @@ describe('#3914 — MCP run_action dispatch binds ctx.api and ctx.engine', () =>
246246
};
247247
const ec = { userId: 'user_42', tenantId: 'org_acme', positions: [], permissions: [] };
248248

249-
await invokeBusinessAction(mcpDeps, 'close_case', { recordId: 'case_1' }, {
249+
await invokeBusinessAction(mcpDeps, { request: {} } as any, 'close_case', { recordId: 'case_1' }, {
250250
driver: undefined,
251251
envId: 'platform',
252252
ec,

packages/runtime/src/action-execution-calldata-query.test.ts

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,16 @@
2121

2222
import { describe, it, expect, beforeEach } from 'vitest';
2323
import { callData, type ActionExecutionDeps } from './action-execution.js';
24+
import type { HttpProtocolContext } from './http-dispatcher.js';
2425

2526
const EC = { userId: 'u1', isSystem: false, positions: [], permissions: [] } as any;
27+
/**
28+
* The request `callData` is serving. [#5155] Every service lookup resolves off
29+
* `context.kernel`, so the request has to be named at the call — a fake that
30+
* ignored it would be modelling the shared-field shape this suite's subject no
31+
* longer has.
32+
*/
33+
const REQ = { request: {} } as HttpProtocolContext;
2634

2735
function makeHarness(opts: { withProtocol?: boolean } = {}) {
2836
const finds: any[] = [];
@@ -39,7 +47,7 @@ function makeHarness(opts: { withProtocol?: boolean } = {}) {
3947
...(protocol ? { protocol } : {}),
4048
};
4149
const deps: ActionExecutionDeps = {
42-
resolveService: (async (name: string) => services[name]) as any,
50+
resolveService: (async (_ctx: HttpProtocolContext, name: string) => services[name]) as any,
4351
getObjectQL: async () => ql,
4452
};
4553
return { deps, finds, findData };
@@ -58,52 +66,52 @@ describe("callData('query') fallback serves the query it was given (#4386)", ()
5866
offset: 10,
5967
fields: ['id', 'title'],
6068
};
61-
const out = await callData(h.deps, 'query', { object: 'task', query }, undefined, undefined, EC);
69+
const out = await callData(h.deps, REQ, 'query', { object: 'task', query }, undefined, undefined, EC);
6270
expect(h.finds).toHaveLength(1);
6371
expect(h.finds[0]).toMatchObject({ ...query, context: EC });
6472
expect(out.records).toHaveLength(2);
6573
});
6674

6775
it('extracts query fields from bare params when params.query is absent — same source as the protocol path', async () => {
68-
await callData(h.deps, 'query', { object: 'task', where: { status: 'open' }, limit: 3 }, undefined, undefined, EC);
76+
await callData(h.deps, REQ, 'query', { object: 'task', where: { status: 'open' }, limit: 3 }, undefined, undefined, EC);
6977
expect(h.finds[0]).toMatchObject({ where: { status: 'open' }, limit: 3 });
7078
});
7179

7280
it('a caller-supplied context is dropped, never honoured — server-derived only, matching findData', async () => {
73-
await callData(h.deps, 'query', { object: 'task', query: { where: { a: 1 }, context: { isSystem: true } } }, undefined, undefined, EC);
81+
await callData(h.deps, REQ, 'query', { object: 'task', query: { where: { a: 1 }, context: { isSystem: true } } }, undefined, undefined, EC);
7482
expect(h.finds[0].context).toBe(EC);
7583
});
7684

7785
it.each(['sort', 'select', 'skip', 'populate', 'search', 'expand', '$filter'])(
7886
'refuses %s with 501 instead of part-serving — nothing reaches ql.find',
7987
async (key) => {
8088
await expect(
81-
callData(h.deps, 'query', { object: 'task', query: { where: { a: 1 }, [key]: 'x' } }, undefined, undefined, EC),
89+
callData(h.deps, REQ, 'query', { object: 'task', query: { where: { a: 1 }, [key]: 'x' } }, undefined, undefined, EC),
8290
).rejects.toMatchObject({ statusCode: 501 });
8391
expect(h.finds).toHaveLength(0);
8492
},
8593
);
8694

8795
it('names the unservable keys and the served set in the refusal', async () => {
8896
await expect(
89-
callData(h.deps, 'query', { object: 'task', query: { sort: '-x', select: 'id' } }, undefined, undefined, EC),
97+
callData(h.deps, REQ, 'query', { object: 'task', query: { sort: '-x', select: 'id' } }, undefined, undefined, EC),
9098
).rejects.toMatchObject({ message: expect.stringMatching(/'sort', 'select'.*where, fields, orderBy, limit, offset/s) });
9199
});
92100

93101
it('an empty query still lists (the protocol path lists too) — no refusal, no predicate', async () => {
94-
const out = await callData(h.deps, 'query', { object: 'task' }, undefined, undefined, EC);
102+
const out = await callData(h.deps, REQ, 'query', { object: 'task' }, undefined, undefined, EC);
95103
expect(h.finds[0]).toMatchObject({ context: EC });
96104
expect(out.total).toBe(2);
97105
});
98106

99107
it('null-valued keys are withdrawals, not unservable', async () => {
100-
await callData(h.deps, 'query', { object: 'task', query: { sort: null, where: { a: 1 } } }, undefined, undefined, EC);
108+
await callData(h.deps, REQ, 'query', { object: 'task', query: { sort: null, where: { a: 1 } } }, undefined, undefined, EC);
101109
expect(h.finds[0]).toMatchObject({ where: { a: 1 } });
102110
});
103111

104112
it('with the protocol service present the fallback never runs — findData gets the query verbatim, wire spellings included', async () => {
105113
const withP = makeHarness({ withProtocol: true });
106-
await callData(withP.deps, 'query', { object: 'task', query: { sort: '-title', top: 5 } }, undefined, undefined, EC);
114+
await callData(withP.deps, REQ, 'query', { object: 'task', query: { sort: '-title', top: 5 } }, undefined, undefined, EC);
107115
expect(withP.findData).toHaveLength(1);
108116
expect(withP.findData[0].query).toEqual({ sort: '-title', top: 5 });
109117
expect(withP.finds).toHaveLength(0);

packages/runtime/src/action-execution.ts

Lines changed: 32 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import { validateActionParams, type ResolvedActionParam } from '@objectstack/spe
1919
import type { ExecutionContext } from '@objectstack/spec/kernel';
2020
import type { IObjectQLEngine, ServiceSlotContract, ServiceSlotContracts } from '@objectstack/spec/contracts';
2121
import { checkApiExposure } from './api-exposure.js';
22+
import type { HttpProtocolContext } from './http-dispatcher.js';
2223
import {
2324
GLOBAL_ACTION_OBJECT_KEY,
2425
actionHandlerObjectKeys,
@@ -78,21 +79,31 @@ function warnActionParamsOnce(key: string, message: string): void {
7879
* the pattern, alongside `ResolveOptions` in security/resolve-execution-context.
7980
* A lookup facade has to be typed everywhere it is re-declared, or the copy
8081
* that still says `any` becomes the way around all the others.
82+
*
83+
* [#5155] Both lookups take the REQUEST as their first parameter, for the
84+
* reason spelled out on `DomainHandlerDeps` (of which this is the narrow view
85+
* `HttpDispatcher.actionExecutionDeps` hands out): the object is shared by
86+
* every request the host serves, so the kernel to resolve against is the
87+
* request's, never the facade's.
8188
*/
8289
export interface ActionExecutionDeps {
83-
resolveService<K extends keyof ServiceSlotContracts>(name: K, environmentId?: string): Promise<ServiceSlotContract<K> | undefined>;
84-
resolveService(name: string, environmentId?: string): any;
85-
getObjectQL(environmentId?: string): Promise<IObjectQLEngine | null>;
90+
resolveService<K extends keyof ServiceSlotContracts>(context: HttpProtocolContext, name: K, environmentId?: string): Promise<ServiceSlotContract<K> | undefined>;
91+
resolveService(context: HttpProtocolContext, name: string, environmentId?: string): any;
92+
getObjectQL(context: HttpProtocolContext, environmentId?: string): Promise<IObjectQLEngine | null>;
8693
}
8794

8895
/**
8996
* Direct data service dispatch — replaces broker.call('data.*').
9097
* Tries protocol service first (supports expand/populate), falls back to ObjectQL.
9198
*
99+
* @param requestContext - The request being served (#5155). Carries the kernel
100+
* every service lookup below resolves against; see
101+
* {@link HttpProtocolContext.kernel}.
92102
* @param dataDriver - Optional environment-scoped driver to use instead of kernel default
93103
* @param scopeId - Optional project ID for scoped service resolution (SharedProjectPlugin mode)
94104
*/
95-
export async function callData(deps: ActionExecutionDeps,
105+
export async function callData(deps: ActionExecutionDeps,
106+
requestContext: HttpProtocolContext,
96107
action: string,
97108
params: any,
98109
dataDriver?: any,
@@ -106,7 +117,7 @@ export async function callData(deps: ActionExecutionDeps,
106117
if (!executionContext?.isSystem && params?.object) {
107118
let def: any;
108119
try {
109-
const meta = await deps.resolveService('metadata', scopeId);
120+
const meta = await deps.resolveService(requestContext, 'metadata', scopeId);
110121
def = await (meta as any)?.getObject?.(params.object);
111122
} catch {
112123
def = undefined; // fall open to schema defaults (apiEnabled=true)
@@ -117,9 +128,9 @@ export async function callData(deps: ActionExecutionDeps,
117128
}
118129
}
119130

120-
const protocol = await deps.resolveService('protocol', scopeId);
121-
const qlService = dataDriver ?? await deps.getObjectQL(scopeId);
122-
const ql = qlService ?? await deps.resolveService('objectql', scopeId);
131+
const protocol = await deps.resolveService(requestContext, 'protocol', scopeId);
132+
const qlService = dataDriver ?? await deps.getObjectQL(requestContext, scopeId);
133+
const ql = qlService ?? await deps.resolveService(requestContext, 'objectql', scopeId);
123134
const qlOpts = executionContext ? { context: executionContext } : undefined;
124135
const findOpts = (extra?: any) => {
125136
const base = qlOpts ? { ...qlOpts } : {};
@@ -252,8 +263,8 @@ export async function callData(deps: ActionExecutionDeps,
252263
if (!Array.isArray(params.aggregations) || params.aggregations.length === 0) {
253264
throw { statusCode: 400, message: 'aggregate requires at least one aggregation' };
254265
}
255-
const engine = (await deps.getObjectQL(scopeId))
256-
?? await deps.resolveService('objectql', scopeId).catch(() => null);
266+
const engine = (await deps.getObjectQL(requestContext, scopeId))
267+
?? await deps.resolveService(requestContext, 'objectql', scopeId).catch(() => null);
257268
if (engine && typeof engine.aggregate === 'function') {
258269
const rows = await engine.aggregate(
259270
params.object,
@@ -383,13 +394,13 @@ export function headlessActionTypeError(_deps: ActionExecutionDeps, action: any,
383394
* the single availability probe behind `type: 'flow'` dispatch (both the
384395
* headless-invokability filter and the two invoke paths ask through it).
385396
*/
386-
export async function resolveAutomationService(deps: ActionExecutionDeps, envId?: string): Promise<any | null> {
397+
export async function resolveAutomationService(deps: ActionExecutionDeps, requestContext: HttpProtocolContext, envId?: string): Promise<any | null> {
387398
try {
388399
// [#4127 batch 4] Was `: any`, which voided the gate here. `execute` is
389400
// declared on IAutomationService, so this needed no contract work — only
390401
// for someone to notice, and three grep sweeps over `domains/*.ts` never
391402
// reached this file. The lint rule did.
392-
const svc = await deps.resolveService('automation', envId);
403+
const svc = await deps.resolveService(requestContext, 'automation', envId);
393404
return svc && typeof svc.execute === 'function' ? svc : null;
394405
} catch {
395406
return null; // no automation service on this kernel
@@ -483,6 +494,7 @@ export function seedFlowActionParams(_deps: ActionExecutionDeps,
483494
* doesn't keep.
484495
*/
485496
export async function dispatchFlowAction(deps: ActionExecutionDeps,
497+
requestContext: HttpProtocolContext,
486498
action: any,
487499
wiring: {
488500
objectName: string;
@@ -494,7 +506,7 @@ export async function dispatchFlowAction(deps: ActionExecutionDeps,
494506
},
495507
): Promise<any> {
496508
const { objectName, record, params, recordId, ec, envId } = wiring;
497-
const automation = await resolveAutomationService(deps, envId);
509+
const automation = await resolveAutomationService(deps, requestContext, envId);
498510
if (!automation) {
499511
throw new Error(flowActionUnavailableError(action));
500512
}
@@ -790,7 +802,8 @@ export function buildActionEngineFacade(_deps: ActionExecutionDeps, ql: any, ec?
790802
* attributable and org-scoped. Flow actions differ: the flow engine receives
791803
* the caller's identity below and honours `runAs` (ADR-0049).
792804
*/
793-
export async function invokeBusinessAction(deps: ActionExecutionDeps,
805+
export async function invokeBusinessAction(deps: ActionExecutionDeps,
806+
requestContext: HttpProtocolContext,
794807
name: string,
795808
input: { objectName?: string; recordId?: string; params?: Record<string, unknown> },
796809
wiring: {
@@ -821,7 +834,7 @@ export async function invokeBusinessAction(deps: ActionExecutionDeps,
821834
if (isSystemObjectName(objectName)) {
822835
throw new Error(`Action '${name}' is on a system object and is not exposed via MCP`);
823836
}
824-
const hasAutomation = Boolean(await resolveAutomationService(deps, envId));
837+
const hasAutomation = Boolean(await resolveAutomationService(deps, requestContext, envId));
825838
if (!isHeadlessInvokableAction(deps, action, hasAutomation)) {
826839
throw new Error(
827840
`Action '${name}' (type='${action?.type ?? 'script'}') cannot be invoked via MCP`,
@@ -873,15 +886,15 @@ export async function invokeBusinessAction(deps: ActionExecutionDeps,
873886

874887
// ── flow dispatch ── (shared with the REST /actions route, #3915)
875888
if (action.type === 'flow') {
876-
const result = await dispatchFlowAction(deps, action, { objectName, record, params, recordId, ec, envId });
889+
const result = await dispatchFlowAction(deps, requestContext, action, { objectName, record, params, recordId, ec, envId });
877890
return { ok: true, action: action.name, objectName, ...(recordId ? { recordId } : {}), result };
878891
}
879892

880893
// ── script/body dispatch via the engine's executeAction ──
881894
// [#4127] `executeAction` is
882895
// ObjectQL's own surface, outside IDataEngine; `getObjectQL` exists to reach
883896
// exactly that. Closing this needs ObjectQL's contract written, not a cast.
884-
const ql: any = await deps.getObjectQL(envId);
897+
const ql: any = await deps.getObjectQL(requestContext, envId);
885898
if (!ql || typeof ql.executeAction !== 'function') {
886899
throw new Error('Data engine not available for action dispatch');
887900
}
@@ -1093,6 +1106,7 @@ export async function executeRegisteredAction(_deps: ActionExecutionDeps,
10931106
* lookup.
10941107
*/
10951108
export async function resolveRouteActionDeclaration(deps: ActionExecutionDeps,
1109+
requestContext: HttpProtocolContext,
10961110
args: { ql: any; objectName: string; actionName: string; envId?: string },
10971111
): Promise<{ action: any; obj: any; degraded?: boolean; reason?: string }> {
10981112
const { ql, objectName, actionName, envId } = args;
@@ -1140,7 +1154,7 @@ export async function resolveRouteActionDeclaration(deps: ActionExecutionDeps,
11401154
// and belongs in the batch that adds the four undeclared auth members.
11411155
// [#4127 batch 4] `loadDiagnosed` is on IMetadataService now, so this
11421156
// reads the contract instead of guessing at it.
1143-
const meta = await deps.resolveService('metadata', envId);
1157+
const meta = await deps.resolveService(requestContext, 'metadata', envId);
11441158
if (meta && typeof meta.loadDiagnosed === 'function') {
11451159
const diag: any = await meta.loadDiagnosed('action', actionName);
11461160
if (diag?.data && ownsRoute(diag.data)) return { action: diag.data, obj };

0 commit comments

Comments
 (0)