Skip to content

Commit 5cb5f96

Browse files
committed
test(trigger-schedule): pin the sweep's SELECTION scope, both directions
The existing pin read `expect(calls[0].context).toEqual({ isSystem: true })` — it was pinning the defect, so it is replaced rather than relaxed: exact equality against `{ isSystem: true, tenantId: <declared> }`, which reddens the moment the sweep asks for no scope. Four new pins: - every window query carries the scope, not just the first (offset mode issues one per offset), and `where` still holds only the date window, so no organization predicate was hand-built onto it; - DIFFERENTIAL — matching rows in TWO organizations, only the declared one's record launches a run. The double implements the driver's documented scope (`org = :tenant OR org IS NULL`) and, with no scope, no predicate at all, so its "absent" arm reproduces the defect instead of hiding it; - a store that cannot honour the scope (driver-memory's #16589 refusal) is reported at `error` naming the flow, never answered unscoped; - a `tenancy.enabled: false` object is disclosed at bind — the declaration cannot narrow that sweep — with a paired control that an ordinary object draws no such line. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37
1 parent 8b45571 commit 5cb5f96

1 file changed

Lines changed: 205 additions & 9 deletions

File tree

packages/triggers/trigger-schedule/src/time-relative-trigger.test.ts

Lines changed: 205 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ interface FindCall {
4949
objectName: string;
5050
where: Record<string, unknown>;
5151
limit?: number;
52-
context?: { isSystem?: boolean };
52+
context?: { isSystem?: boolean; tenantId?: string };
5353
}
5454

5555
/**
@@ -74,6 +74,39 @@ function fakeDataEngine(rows: Row[], knownObjects: string[] = ['contracts']) {
7474
return { engine, calls };
7575
}
7676

77+
/**
78+
* [#16659] A fake ObjectQL surface that HONOURS `context.tenantId`, so the
79+
* differential control can put matching rows in two organizations and observe
80+
* which ones come back.
81+
*
82+
* The scope it implements is the SQL driver's documented one — `tenantId`
83+
* present ⇒ `(organization_id = :tenant OR organization_id IS NULL)`; `tenantId`
84+
* absent ⇒ no predicate at all, which is exactly how the unfixed sweep read.
85+
* ⛔ Not a convenience double that filters whatever it is handed: the "absent"
86+
* arm has to reproduce the DEFECT, or ablating the fix would still look scoped.
87+
*/
88+
function tenantScopedDataEngine(rows: Row[], knownObjects: string[] = ['contracts']) {
89+
const calls: FindCall[] = [];
90+
const engine: TimeRelativeDataEngine = {
91+
async find(objectName, query) {
92+
const where = (query?.where ?? {}) as Record<string, unknown>;
93+
calls.push({ objectName, where, limit: query?.limit, context: query?.context });
94+
const tenant = query?.context?.tenantId;
95+
const scoped = rows.filter((row) => {
96+
if (tenant === undefined) return true;
97+
const org = row.organization_id;
98+
return org === tenant || org == null;
99+
});
100+
const out = scoped.filter((row) => matches(row, where));
101+
return typeof query?.limit === 'number' ? out.slice(0, query.limit) : out;
102+
},
103+
getObject(name) {
104+
return knownObjects.includes(name) ? { name } : undefined;
105+
},
106+
};
107+
return { engine, calls };
108+
}
109+
77110
/** Minimal where matcher: temporal range on the date field + scalar equality. */
78111
function matches(row: Row, where: Record<string, unknown>): boolean {
79112
for (const [key, cond] of Object.entries(where)) {
@@ -98,13 +131,21 @@ function silentLogger(): TriggerLogger {
98131
/** Fixed reference clock: 2026-07-18 (noon UTC). */
99132
const NOW = () => new Date('2026-07-18T12:00:00.000Z');
100133

134+
/**
135+
* [#16659] The organization every fixture binding declares. Named rather than
136+
* inlined because it is now asserted from two directions — the sweep's query
137+
* scope and the launched run's identity — and a literal repeated at both ends
138+
* of that pair can drift into agreeing with itself.
139+
*/
140+
const TEST_ORG = 'org_2mtx1w9d0k4bqf7v';
141+
101142
function binding(timeRelative: unknown, overrides: Partial<FlowTriggerBinding> = {}): FlowTriggerBinding {
102143
return {
103144
flowName: 'renewal_alert',
104145
object: 'contracts',
105146
config: { timeRelative },
106147
// [#16659] see the schedule trigger's fixture note.
107-
organization: 'org_2mtx1w9d0k4bqf7v',
148+
organization: TEST_ORG,
108149
...overrides,
109150
};
110151
}
@@ -227,8 +268,14 @@ describe('TimeRelativeTrigger', () => {
227268
// Context is record-shaped (so `{record.x}` + start conditions work).
228269
expect(seen[0]).toMatchObject({ object: 'contracts', event: 'time_relative' });
229270
expect(seen[0].record).toBe(seen[0].params);
230-
// The sweep queries as a system op (sees all rows, RLS-bypassing).
231-
expect(calls[0].context).toEqual({ isSystem: true });
271+
// The sweep queries as a system op (sees all rows, RLS-bypassing) AND
272+
// inside its declared organization. [#16659] This assertion used to
273+
// read `{ isSystem: true }` and it was pinning the defect: `isSystem`
274+
// is AUTHORIZATION and `tenantId` is TENANCY, and a sweep carrying only
275+
// the first selects across every tenant while its runs act as one.
276+
// ⛔ Do not relax it back to a subset match — the exact-equality is
277+
// what makes "the sweep asks for no scope" red.
278+
expect(calls[0].context).toEqual({ isSystem: true, tenantId: TEST_ORG });
232279
expect(calls[0].where).toEqual({
233280
status: 'active',
234281
end_date: { $gte: '2026-07-18T00:00:00.000Z', $lte: '2026-09-16T23:59:59.999Z' },
@@ -733,23 +780,29 @@ describe('TimeRelativeTriggerPlugin', () => {
733780
// ─── The acting-organization refusal (#16659) ───────────────────────
734781
//
735782
// The time-relative sweep is NOT the weaker case for carrying an organization,
736-
// it is the stronger one: it queries with `context: { isSystem: true }` on
737-
// purpose, so an org-less sweep selects across every tenant and then launches a
738-
// run that can write into none of them.
783+
// it is the stronger one: it runs ELEVATED on purpose (`isSystem` — a
784+
// background sweep must see every row, not the RLS-scoped subset), so the
785+
// declaration is the only thing keeping its SELECTION inside one organization.
786+
// An org-less sweep selects across every tenant and then launches a run that
787+
// can write into none of them; a sweep whose declaration reached only the run
788+
// selects across every tenant and launches runs that write into ONE, which is
789+
// worse. Both halves are pinned below.
739790

740791
describe('TimeRelativeTrigger — the acting-organization refusal (#16659)', () => {
741792
const DESC = { object: 'contracts', dateField: 'end_date', withinDays: 60 };
742793

743-
function recordingLogger(): { logger: TriggerLogger; errors: string[] } {
794+
function recordingLogger(): { logger: TriggerLogger; errors: string[]; warns: string[] } {
744795
const errors: string[] = [];
796+
const warns: string[] = [];
745797
return {
746798
logger: {
747799
info: () => {},
748800
debug: () => {},
749-
warn: () => {},
801+
warn: (msg: string) => void warns.push(String(msg)),
750802
error: (msg: string) => void errors.push(String(msg)),
751803
},
752804
errors,
805+
warns,
753806
};
754807
}
755808

@@ -787,4 +840,147 @@ describe('TimeRelativeTrigger — the acting-organization refusal (#16659)', ()
787840
await flush();
788841
expect(job.jobs.size).toBe(0);
789842
});
843+
844+
// ── the SELECTION half (#16659, F2) ───────────────────────────────────
845+
//
846+
// Declaring an organization bounded the RUN and left the QUERY unbounded,
847+
// so a sweep declared for A matched rows in every tenant and launched runs
848+
// stamped A about other organizations' records. These pins are about the
849+
// query.
850+
851+
it('EVERY window query carries the declared organization, not just the first', async () => {
852+
// Offset mode issues one query per offset — a scope threaded onto only
853+
// the first would leave the rest crossing organizations, and a pin that
854+
// read `calls[0]` alone would not notice.
855+
const job = fakeJobService();
856+
const { engine, calls } = fakeDataEngine([]);
857+
const trigger = new TimeRelativeTrigger(() => job.service, () => engine, silentLogger(), NOW);
858+
859+
trigger.start(
860+
binding({ object: 'contracts', dateField: 'end_date', offsetDays: [60, 30, 7] }),
861+
async () => {},
862+
);
863+
await flush();
864+
await job.fire('flow-time-relative:renewal_alert');
865+
866+
expect(calls.length, 'offset mode must issue one query per offset').toBe(3);
867+
expect(
868+
calls.map((c) => c.context?.tenantId ?? 'NO-SCOPE'),
869+
'an unscoped window query selects every organization\'s rows',
870+
).toEqual([TEST_ORG, TEST_ORG, TEST_ORG]);
871+
// The scope is the ONLY thing tenancy contributes: the author's filter
872+
// and the date window are untouched, so no organization predicate was
873+
// hand-built onto `where` (which would hardcode a column name the
874+
// object is free to rename, and select nothing where there is none).
875+
for (const call of calls) {
876+
expect(Object.keys(call.where)).toEqual(['end_date']);
877+
}
878+
});
879+
880+
it('DIFFERENTIAL: with matching rows in two organizations only the declared one is swept', async () => {
881+
// The discriminating shape. A pin that only proved "A's rows are found"
882+
// passes on the defect too — the defect FOUND them, alongside B's.
883+
//
884+
// The double implements the documented driver contract rather than a
885+
// convenient one: `DriverOptions.tenantId` scopes to
886+
// `(organization_id = :tenant OR organization_id IS NULL)`
887+
// (sql-driver's own `tenantFieldByTable` note), and an ABSENT scope
888+
// applies no predicate at all — which is precisely how the unfixed
889+
// sweep read.
890+
const ORG_B = 'org_beta_0000000000000';
891+
const rows: Row[] = [
892+
{ id: 'a1', end_date: '2026-07-25T00:00:00.000Z', organization_id: TEST_ORG },
893+
{ id: 'b1', end_date: '2026-07-25T00:00:00.000Z', organization_id: ORG_B },
894+
{ id: 'b2', end_date: '2026-07-26T00:00:00.000Z', organization_id: ORG_B },
895+
];
896+
const job = fakeJobService();
897+
const { engine, calls } = tenantScopedDataEngine(rows);
898+
const trigger = new TimeRelativeTrigger(() => job.service, () => engine, silentLogger(), NOW);
899+
const seen: AutomationContext[] = [];
900+
901+
trigger.start(
902+
binding({ object: 'contracts', dateField: 'end_date', withinDays: 60 }),
903+
async (ctx) => void seen.push(ctx),
904+
);
905+
await flush();
906+
await job.fire('flow-time-relative:renewal_alert');
907+
908+
expect(
909+
seen.map((c) => (c.record as Row).id),
910+
'the sweep launched a run for a record in an organization the flow never declared',
911+
).toEqual(['a1']);
912+
expect(
913+
seen.map((c) => c.tenantId),
914+
'and the run still acts as the declared organization',
915+
).toEqual([TEST_ORG]);
916+
expect(calls[0].context?.tenantId, 'the scope must reach the engine, not be applied afterwards').toBe(TEST_ORG);
917+
});
918+
919+
it('a store that CANNOT honour the scope is reported at `error`, never answered unscoped', async () => {
920+
// `driver-memory` refuses any call handed a tenant scope (#16589). A
921+
// sweep required to stay inside one organization, talking to a store
922+
// that cannot keep it there, must be LOUD — "selected nothing this
923+
// tick" and "cannot select at all" are different facts.
924+
const job = fakeJobService();
925+
const engine: TimeRelativeDataEngine = {
926+
async find(_objectName, query) {
927+
if (query?.context?.tenantId !== undefined) {
928+
throw Object.assign(new Error('[driver-memory] Refusing to answer: this driver has NO row-level tenant isolation.'), {
929+
code: 'MEMORY_MULTI_TENANT_UNSUPPORTED',
930+
});
931+
}
932+
return [];
933+
},
934+
getObject: () => ({ name: 'contracts' }),
935+
};
936+
const log = recordingLogger();
937+
const trigger = new TimeRelativeTrigger(() => job.service, () => engine, log.logger, NOW);
938+
939+
trigger.start(binding({ object: 'contracts', dateField: 'end_date', withinDays: 60 }), async () => {});
940+
await flush();
941+
await job.fire('flow-time-relative:renewal_alert');
942+
943+
const failure = log.errors.find((l) => l.includes('sweep failed'));
944+
expect(failure, `the sweep failed silently; errors seen: ${JSON.stringify(log.errors)}`).toBeTruthy();
945+
expect(failure, 'the failure must name the flow it belongs to').toContain('renewal_alert');
946+
expect(failure, "and carry the store's own reason").toContain('NO row-level tenant isolation');
947+
});
948+
949+
it('says so at bind when the swept object is one the engine will NOT scope', async () => {
950+
// ⚠️ The quiet direction. A `tenancy.enabled: false` object (ADR-0066)
951+
// is exempt from the engine's tenant scope, so the declaration cannot
952+
// narrow this sweep at all — it still selects across every
953+
// organization, while the flow's `organization` line makes it LOOK
954+
// contained. Nothing here changes which rows come back; the pin is that
955+
// the operator is TOLD.
956+
const job = fakeJobService();
957+
const { engine } = fakeDataEngine([]);
958+
engine.getObject = () => ({ name: 'contracts', tenancy: { enabled: false } });
959+
const log = recordingLogger();
960+
const trigger = new TimeRelativeTrigger(() => job.service, () => engine, log.logger, NOW);
961+
962+
trigger.start(binding({ object: 'contracts', dateField: 'end_date', withinDays: 60 }), async () => {});
963+
await flush();
964+
965+
const said = log.warns.find((l) => l.includes('tenancy'));
966+
expect(said, `nothing was said; warns seen: ${JSON.stringify(log.warns)}`).toBeTruthy();
967+
expect(said, 'the warning must name the object whose declaration makes the scope inert').toContain('contracts');
968+
expect(said, 'and state the consequence, not just the fact').toContain('does NOT narrow this sweep');
969+
expect(job.jobs.size, 'the sweep still binds — this is a disclosure, not a refusal').toBe(1);
970+
});
971+
972+
it('control: a tenant-scoped object gets NO such warning', async () => {
973+
const job = fakeJobService();
974+
const { engine } = fakeDataEngine([]);
975+
const log = recordingLogger();
976+
const trigger = new TimeRelativeTrigger(() => job.service, () => engine, log.logger, NOW);
977+
978+
trigger.start(binding({ object: 'contracts', dateField: 'end_date', withinDays: 60 }), async () => {});
979+
await flush();
980+
981+
expect(
982+
log.warns.filter((l) => l.includes('does NOT narrow this sweep')),
983+
'an ordinary object must not be warned about — that would train operators to ignore the line',
984+
).toHaveLength(0);
985+
});
790986
});

0 commit comments

Comments
 (0)