Skip to content

Commit 6c7512d

Browse files
os-zhuangclaude
andcommitted
feat(objectql): accept execution context via trailing options arg on read methods
The engine's read methods (find/findOne/count/aggregate) took execution context INSIDE the query (`query.context`), while the write methods (insert/update) took it in a trailing `options.context`. That split is a footgun: the same `{ context }` object is correct as the 3rd arg to `insert` but was SILENTLY DROPPED as the 3rd arg to `find` — a recurring class of bugs where an intended `isSystem` bypass just vanished (e.g. control-plane reads coming back empty once org-scoping hooks were added; same shape as the staging "all environments invisible" incident). This is the AI-error root cause: the read API fought the universal convention (and AI's instinct) of "context/options goes in the trailing argument", so people and agents kept passing it there and losing it. Fix: read methods now ALSO accept `options.context` (3rd arg), merged into the effective context (options wins when both given). `query.context` stays fully supported. One rule now spans reads and writes: execution context goes in the trailing options argument. This additively fixes the existing dropped-context call sites (plugin-security/auth/org-scoping, objectos auth-proxy, cloud environment-crud, and several tests) with zero call-site edits. Tests: +5 engine cases (options.context honored on find/findOne/count, legacy query.context still works, options wins on conflict). objectql 621, plugin-security 87, plugin-auth 118, plugin-org-scoping 15 — all green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 48e4eb4 commit 6c7512d

3 files changed

Lines changed: 99 additions & 8 deletions

File tree

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
---
2+
"@objectstack/objectql": minor
3+
---
4+
5+
engine: accept execution context via the trailing `options` argument on the read
6+
methods (`find` / `findOne` / `count` / `aggregate`), aligning them with the
7+
write methods (`insert` / `update`).
8+
9+
Previously reads took context only inside the query (`query.context`) while
10+
writes took it in a trailing `options.context`. The same `{ context }` object was
11+
therefore correct as the 3rd argument to `insert` but **silently dropped** as the
12+
3rd argument to `find` — a recurring footgun where an intended `isSystem` bypass
13+
just vanished (e.g. control-plane reads returning empty once org-scoping hooks
14+
were added). Now "execution context goes in the trailing `options` argument" is a
15+
single rule across reads and writes. `query.context` remains fully supported; when
16+
both are supplied, `options.context` wins.

packages/objectql/src/engine.test.ts

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -245,6 +245,53 @@ describe('ObjectQL Engine', () => {
245245
});
246246
});
247247

248+
describe('execution context via the trailing options arg (read methods)', () => {
249+
// Regression: reads took context inside the query while writes took it in
250+
// a trailing options arg — so `find(obj, q, { context })` silently dropped
251+
// the context (e.g. an intended isSystem bypass), a real source of
252+
// "empty result once scoping was added" bugs. Reads now ALSO accept the
253+
// trailing options.context, aligning with insert/update. `tenantId` is a
254+
// convenient observable: buildDriverOptions forwards it to the driver.
255+
beforeEach(async () => {
256+
engine.registerDriver(mockDriver, true);
257+
await engine.init();
258+
vi.mocked(SchemaRegistry.getObject).mockReturnValue({ name: 'task', fields: {} });
259+
});
260+
261+
const lastFindOpts = () => (mockDriver.find as any).mock.calls.at(-1)?.[2];
262+
263+
it('find: context from the trailing options arg reaches the driver', async () => {
264+
await engine.find('task', { filters: [] }, { context: { tenantId: 't-opts' } as any });
265+
expect(lastFindOpts()).toMatchObject({ tenantId: 't-opts' });
266+
});
267+
268+
it('find: context inside the query still works (legacy form)', async () => {
269+
await engine.find('task', { filters: [], context: { tenantId: 't-query' } as any });
270+
expect(lastFindOpts()).toMatchObject({ tenantId: 't-query' });
271+
});
272+
273+
it('find: when both are given, the trailing options.context wins', async () => {
274+
await engine.find(
275+
'task',
276+
{ context: { tenantId: 't-query' } as any },
277+
{ context: { tenantId: 't-opts' } as any },
278+
);
279+
expect(lastFindOpts()).toMatchObject({ tenantId: 't-opts' });
280+
});
281+
282+
it('findOne accepts context via the trailing options arg', async () => {
283+
(mockDriver.findOne as any).mockResolvedValue({ id: '1' });
284+
await engine.findOne('task', { filters: [] }, { context: { tenantId: 't-fo' } as any });
285+
expect((mockDriver.findOne as any).mock.calls.at(-1)?.[2]).toMatchObject({ tenantId: 't-fo' });
286+
});
287+
288+
it('count accepts context via the trailing options arg', async () => {
289+
(mockDriver.count as any).mockResolvedValue(0);
290+
await engine.count('task', {}, { context: { tenantId: 't-cnt' } as any });
291+
expect((mockDriver.count as any).mock.calls.at(-1)?.[2]).toMatchObject({ tenantId: 't-cnt' });
292+
});
293+
});
294+
248295
describe('Update hooks — previous-record snapshot', () => {
249296
beforeEach(async () => {
250297
engine.registerDriver(mockDriver, true);

packages/objectql/src/engine.ts

Lines changed: 36 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,34 @@ export interface OperationContext {
135135
result?: any;
136136
}
137137

138+
/**
139+
* Trailing options for the READ methods (find / findOne / count / aggregate).
140+
*
141+
* Historically the read methods took their execution context INSIDE the query
142+
* (`query.context`), while the WRITE methods (insert / update) took it in a
143+
* trailing `options.context`. That split was a footgun: the same `{ context }`
144+
* object is correct as the 3rd arg to `insert` but was SILENTLY DROPPED as the
145+
* 3rd arg to `find` — a class of bugs where an intended `isSystem` bypass just
146+
* vanished (e.g. control-plane reads coming back empty once org-scoping hooks
147+
* were added). We now ALSO accept `context` via this trailing options arg on the
148+
* read methods, so "execution context goes in the trailing options argument" is
149+
* one rule across reads and writes. `query.context` remains supported; when both
150+
* are given, `options.context` wins (it is the explicit channel).
151+
*/
152+
export interface EngineReadOptions {
153+
context?: ExecutionContext;
154+
}
155+
156+
/** Merge read-path execution context from the query and the trailing options. */
157+
function mergeReadContext(
158+
fromQuery?: ExecutionContext,
159+
fromOptions?: ExecutionContext,
160+
): ExecutionContext | undefined {
161+
if (fromOptions == null) return fromQuery;
162+
if (fromQuery == null) return fromOptions;
163+
return { ...fromQuery, ...fromOptions };
164+
}
165+
138166
/**
139167
* Engine Middleware (Onion model)
140168
*/
@@ -1766,7 +1794,7 @@ export class ObjectQL implements IDataEngine {
17661794
// Data Access Methods (IDataEngine Interface)
17671795
// ============================================
17681796

1769-
async find(object: string, query?: EngineQueryOptions): Promise<any[]> {
1797+
async find(object: string, query?: EngineQueryOptions, options?: EngineReadOptions): Promise<any[]> {
17701798
object = this.resolveObjectName(object);
17711799
this.logger.debug('Find operation starting', { object, query });
17721800
const driver = this.getDriver(object);
@@ -1818,7 +1846,7 @@ export class ObjectQL implements IDataEngine {
18181846
operation: 'find',
18191847
ast,
18201848
options: query,
1821-
context: query?.context,
1849+
context: mergeReadContext(query?.context, options?.context),
18221850
};
18231851

18241852
await this.executeWithMiddleware(opCtx, async () => {
@@ -1864,7 +1892,7 @@ export class ObjectQL implements IDataEngine {
18641892
return opCtx.result as any[];
18651893
}
18661894

1867-
async findOne(objectName: string, query?: EngineQueryOptions): Promise<any> {
1895+
async findOne(objectName: string, query?: EngineQueryOptions, options?: EngineReadOptions): Promise<any> {
18681896
objectName = this.resolveObjectName(objectName);
18691897
this.logger.debug('FindOne operation', { objectName });
18701898
const driver = this.getDriver(objectName);
@@ -1896,7 +1924,7 @@ export class ObjectQL implements IDataEngine {
18961924
operation: 'findOne',
18971925
ast,
18981926
options: query,
1899-
context: query?.context,
1927+
context: mergeReadContext(query?.context, options?.context),
19001928
};
19011929

19021930
await this.executeWithMiddleware(opCtx, async () => {
@@ -2341,15 +2369,15 @@ export class ObjectQL implements IDataEngine {
23412369
return opCtx.result;
23422370
}
23432371

2344-
async count(object: string, query?: EngineCountOptions): Promise<number> {
2372+
async count(object: string, query?: EngineCountOptions, options?: EngineReadOptions): Promise<number> {
23452373
object = this.resolveObjectName(object);
23462374
const driver = this.getDriver(object);
23472375

23482376
const opCtx: OperationContext = {
23492377
object,
23502378
operation: 'count',
23512379
options: query,
2352-
context: query?.context,
2380+
context: mergeReadContext(query?.context, options?.context),
23532381
};
23542382

23552383
await this.executeWithMiddleware(opCtx, async () => {
@@ -2366,7 +2394,7 @@ export class ObjectQL implements IDataEngine {
23662394
return opCtx.result as number;
23672395
}
23682396

2369-
async aggregate(object: string, query: EngineAggregateOptions): Promise<any[]> {
2397+
async aggregate(object: string, query: EngineAggregateOptions, options?: EngineReadOptions): Promise<any[]> {
23702398
object = this.resolveObjectName(object);
23712399
const driver = this.getDriver(object);
23722400
this.logger.debug(`Aggregate on ${object} using ${driver.name}`, query);
@@ -2375,7 +2403,7 @@ export class ObjectQL implements IDataEngine {
23752403
object,
23762404
operation: 'aggregate',
23772405
options: query,
2378-
context: query?.context,
2406+
context: mergeReadContext(query?.context, options?.context),
23792407
};
23802408

23812409
await this.executeWithMiddleware(opCtx, async () => {

0 commit comments

Comments
 (0)