Skip to content

Commit fce14ab

Browse files
os-zhuangclaude
andauthored
fix(runtime): callData('query') fallback serves the caller's query instead of dropping it (#4386) (#4390)
The protocol-unavailable fallback passed only { context } to ql.find — the caller's where/orderBy/limit never left the function, and the ENTIRE table came back as an ordinary-looking { records, total }. The sibling get/update/ delete fallbacks all built a proper where; query was the only verb whose fallback forgot the request. Forward the canonical QueryAST keys both possible recipients execute (where/fields/orderBy/limit/offset — engine option bag and raw-driver QueryAST are aligned by design), drop caller-supplied context (server-derived only, matching findData's unconditional delete), and refuse 501 on anything the fallback cannot reproduce without the protocol layer: wire spellings needing fold/lowering (sort/select/skip/populate — folding here would re-implement the protocol's lowering per reader, the #3795 condition) and capabilities a raw driver would silently drop (search/expand). A fallback that cannot reproduce the query's semantics must not pretend to serve it (route-ownership rule 3). Protocol path unchanged. Fixes #4386. Found during the #4371 call-site survey. Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent af5b96b commit fce14ab

3 files changed

Lines changed: 172 additions & 6 deletions

File tree

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
---
2+
"@objectstack/runtime": patch
3+
---
4+
5+
fix(runtime): the `callData('query')` ObjectQL fallback serves the caller's query instead of dropping it (#4386)
6+
7+
When the protocol service is unavailable (lean assemblies, MCP multi-env with
8+
a raw driver), the fallback passed only `{ context }` to `ql.find` — the
9+
caller's `where`/`orderBy`/`limit` never left the function, and the ENTIRE
10+
table came back as an ordinary-looking `{ records, total }`. The sibling
11+
`get`/`update`/`delete` fallbacks all built a proper `where`; `query` was the
12+
only verb whose fallback forgot the request.
13+
14+
The fallback now forwards the canonical QueryAST keys both possible
15+
recipients execute (`where`, `fields`, `orderBy`, `limit`, `offset` — engine
16+
option bag and raw-driver QueryAST are aligned by design), drops a
17+
caller-supplied `context` (server-derived only, matching `findData`), and
18+
refuses with 501 anything it cannot reproduce without the protocol layer —
19+
wire spellings needing fold/lowering (`sort`, `select`, `skip`, `populate`)
20+
and capabilities a raw driver would silently drop (`search`, `expand`). The
21+
protocol path is unchanged and keeps accepting wire spellings.
Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* #4386 — the `callData('query')` ObjectQL fallback serves the caller's query
5+
* instead of dropping it.
6+
*
7+
* Regression: when the protocol service is unavailable (lean assemblies, MCP
8+
* multi-env with a raw driver), the fallback passed only `{ context }` to
9+
* `ql.find` — no `where`, no `orderBy`, no `limit` — and answered with an
10+
* ordinary-looking `{ records, total }` of the ENTIRE table. The sibling
11+
* `get`/`update`/`delete` fallbacks all built a proper `where`; `query` was
12+
* the only verb whose fallback forgot the request.
13+
*
14+
* The fallback now forwards the canonical QueryAST keys both possible
15+
* recipients execute (engine option bag / raw-driver QueryAST are aligned by
16+
* design), and REFUSES (501) anything it cannot reproduce without the
17+
* protocol layer — wire spellings needing fold/lowering (`sort`, `select`),
18+
* and capabilities a raw driver would silently drop (`search`, `expand`).
19+
* A fallback that cannot reproduce the query's semantics must not pretend to.
20+
*/
21+
22+
import { describe, it, expect, beforeEach } from 'vitest';
23+
import { callData, type ActionExecutionDeps } from './action-execution.js';
24+
25+
const EC = { userId: 'u1', isSystem: false, positions: [], permissions: [] } as any;
26+
27+
function makeHarness(opts: { withProtocol?: boolean } = {}) {
28+
const finds: any[] = [];
29+
const findData: any[] = [];
30+
const ql = {
31+
find: async (_o: string, bag: any) => { finds.push(bag); return [{ id: 'r1' }, { id: 'r2' }]; },
32+
};
33+
const protocol = opts.withProtocol
34+
? { findData: async (req: any) => { findData.push(req); return { object: req.object, records: [], total: 0, hasMore: false }; } }
35+
: undefined;
36+
const services: Record<string, any> = {
37+
metadata: { getObject: async () => ({ name: 'task', fields: {} }) },
38+
objectql: ql,
39+
...(protocol ? { protocol } : {}),
40+
};
41+
const deps: ActionExecutionDeps = {
42+
resolveService: (async (name: string) => services[name]) as any,
43+
getObjectQL: async () => ql,
44+
};
45+
return { deps, finds, findData };
46+
}
47+
48+
describe("callData('query') fallback serves the query it was given (#4386)", () => {
49+
let h: ReturnType<typeof makeHarness>;
50+
51+
beforeEach(() => { h = makeHarness(); });
52+
53+
it('forwards where/orderBy/limit/offset/fields to ql.find, with the server context', async () => {
54+
const query = {
55+
where: { status: 'open' },
56+
orderBy: [{ field: 'created_at', order: 'desc' }],
57+
limit: 5,
58+
offset: 10,
59+
fields: ['id', 'title'],
60+
};
61+
const out = await callData(h.deps, 'query', { object: 'task', query }, undefined, undefined, EC);
62+
expect(h.finds).toHaveLength(1);
63+
expect(h.finds[0]).toMatchObject({ ...query, context: EC });
64+
expect(out.records).toHaveLength(2);
65+
});
66+
67+
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);
69+
expect(h.finds[0]).toMatchObject({ where: { status: 'open' }, limit: 3 });
70+
});
71+
72+
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);
74+
expect(h.finds[0].context).toBe(EC);
75+
});
76+
77+
it.each(['sort', 'select', 'skip', 'populate', 'search', 'expand', '$filter'])(
78+
'refuses %s with 501 instead of part-serving — nothing reaches ql.find',
79+
async (key) => {
80+
await expect(
81+
callData(h.deps, 'query', { object: 'task', query: { where: { a: 1 }, [key]: 'x' } }, undefined, undefined, EC),
82+
).rejects.toMatchObject({ statusCode: 501 });
83+
expect(h.finds).toHaveLength(0);
84+
},
85+
);
86+
87+
it('names the unservable keys and the served set in the refusal', async () => {
88+
await expect(
89+
callData(h.deps, 'query', { object: 'task', query: { sort: '-x', select: 'id' } }, undefined, undefined, EC),
90+
).rejects.toMatchObject({ message: expect.stringMatching(/'sort', 'select'.*where, fields, orderBy, limit, offset/s) });
91+
});
92+
93+
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);
95+
expect(h.finds[0]).toMatchObject({ context: EC });
96+
expect(out.total).toBe(2);
97+
});
98+
99+
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);
101+
expect(h.finds[0]).toMatchObject({ where: { a: 1 } });
102+
});
103+
104+
it('with the protocol service present the fallback never runs — findData gets the query verbatim, wire spellings included', async () => {
105+
const withP = makeHarness({ withProtocol: true });
106+
await callData(withP.deps, 'query', { object: 'task', query: { sort: '-title', top: 5 } }, undefined, undefined, EC);
107+
expect(withP.findData).toHaveLength(1);
108+
expect(withP.findData[0].query).toEqual({ sort: '-title', top: 5 });
109+
expect(withP.finds).toHaveLength(0);
110+
});
111+
});

packages/runtime/src/action-execution.ts

Lines changed: 40 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -186,16 +186,50 @@ export async function callData(deps: ActionExecutionDeps,
186186
}
187187

188188
if (action === 'query' || action === 'find') {
189+
// Build query: use explicit params.query if provided, otherwise extract
190+
// query fields from params. Shared by both paths below — the fallback
191+
// must serve the SAME request the protocol path would have served.
192+
const query = params.query || (() => {
193+
const { object, ...rest } = params;
194+
return rest;
195+
})();
189196
if (protocol && typeof protocol.findData === 'function') {
190-
// Build query: use explicit params.query if provided, otherwise extract query fields from params
191-
const query = params.query || (() => {
192-
const { object, ...rest } = params;
193-
return rest;
194-
})();
195197
return await protocol.findData({ object: params.object, query, context: executionContext });
196198
}
197199
if (ql) {
198-
let all = await ql.find(params.object, qlOpts);
200+
// [#4386] This fallback used to pass only `{ context }` — the
201+
// caller's entire query (where/orderBy/limit/…) was dropped and the
202+
// FULL table came back as an ordinary-looking `{ records, total }`.
203+
// Serve the canonical QueryAST keys both possible recipients
204+
// actually execute (`ql` here is the engine, or on the MCP
205+
// multi-env path a RAW driver reading a QueryAST — same canonical
206+
// keys by design). Anything else — wire spellings (`sort`,
207+
// `select`, …) that need the protocol layer's fold/lowering, or
208+
// capabilities a raw driver would silently drop (`search`,
209+
// `expand`) — is refused loudly rather than part-served: a
210+
// fallback that cannot reproduce the query's semantics must not
211+
// pretend to (route-ownership rule 3).
212+
const FALLBACK_QUERY_KEYS = ['where', 'fields', 'orderBy', 'limit', 'offset'];
213+
const bag: any = {};
214+
const unservable: string[] = [];
215+
for (const [k, v] of Object.entries((query ?? {}) as Record<string, unknown>)) {
216+
if (v == null) continue;
217+
// `context` is SERVER-derived on this path, same as findData's
218+
// unconditional `delete options.context` — a caller-supplied
219+
// one is dropped, never an error and never honoured.
220+
if (k === 'context') continue;
221+
if (FALLBACK_QUERY_KEYS.includes(k)) bag[k] = v;
222+
else unservable.push(k);
223+
}
224+
if (unservable.length > 0) {
225+
throw {
226+
statusCode: 501,
227+
message: `Data query fallback cannot serve ${unservable.map((k) => `'${k}'`).join(', ')}: ` +
228+
'the protocol service (metadata-protocol plugin) is not registered, and without its ' +
229+
`normalization this path serves only canonical QueryAST keys (${FALLBACK_QUERY_KEYS.join(', ')}).`,
230+
};
231+
}
232+
let all = await ql.find(params.object, findOpts(bag));
199233
if (!Array.isArray(all) && all && (all as any).value) all = (all as any).value;
200234
if (!all) all = [];
201235
return { object: params.object, records: all, total: all.length };

0 commit comments

Comments
 (0)