Skip to content

Commit 38eb666

Browse files
committed
fix(17396): audit reports the recorded policy refusal, not a live env re-read
The reason was re-derived inside getTriggerBindingAudit() from a live resolveScheduledWorkPolicy() read. The audit is consumed long after the bind, so an environment that moved in between made it report 'binding failed' for a flow whose trigger was never called — the exact reading ruled item 6 forbids. Record the refusal at the gate; clear it the moment a flow gets past. Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KB5PFtxuy1x3dcR5gxudx6
1 parent c80021e commit 38eb666

3 files changed

Lines changed: 261 additions & 20 deletions

File tree

packages/qa/dogfood/test/schedule-sweep-organization-scope.dogfood.test.ts

Lines changed: 128 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ import { bootStack, type VerifyStack } from '@objectstack/verify';
3939
import { MessagingServicePlugin, INBOX_OBJECT, NOTIFICATION_EVENT_OBJECT } from '@objectstack/service-messaging';
4040
import { TimeRelativeTrigger, type JobServiceSurface, type TriggerLogger } from '@objectstack/trigger-schedule';
4141
import type { JobHandler, JobSchedule } from '@objectstack/spec/contracts';
42+
import { SCHEDULED_WORK_ENV, SCHEDULED_WORK_DISABLED_REASON } from '@objectstack/types';
4243
import {
4344
scheduleOrganizationStack,
4445
declaringTimeRelativeFlow,
@@ -119,6 +120,9 @@ for (const databaseDriver of ['sqlite-wasm', 'memory'] as const) {
119120
let orgB: string;
120121
let rowA: string;
121122
let rowsB: string[];
123+
let recipientId: string;
124+
let priorSwitch: string | undefined;
125+
let priorPosture: string | undefined;
122126

123127
beforeAll(async () => {
124128
stack = await bootStack(scheduleOrganizationStack as never, {
@@ -143,7 +147,7 @@ for (const databaseDriver of ['sqlite-wasm', 'memory'] as const) {
143147
expect(orgA).not.toBe(orgB);
144148

145149
const admin = await ql.findOne('sys_user', { where: { email: 'admin@objectos.ai' }, ...SYS });
146-
const recipientId = String(admin?.id ?? 'usr_system');
150+
recipientId = String(admin?.id ?? 'usr_system');
147151

148152
// ── the differential fixture ──────────────────────────────────────
149153
// One matching row in A, TWO in B. Every row is inside the window, so
@@ -165,6 +169,36 @@ for (const databaseDriver of ['sqlite-wasm', 'memory'] as const) {
165169
'precondition: the rows must actually carry the two organizations — a NULL-org row is visible under ANY scope (`org = :tenant OR org IS NULL`), so a fixture that failed to stamp them would make this suite pass unfixed',
166170
).toEqual([orgA, orgB, orgB].sort());
167171

172+
// ── [#17396] The DEPLOYMENT this suite is about ───────────────────
173+
//
174+
// Ruling G put two environment facts in front of every bind, and both
175+
// are set HERE, around the bind, rather than at boot:
176+
//
177+
// 1. `OS_AUTOMATION_SCHEDULED_WORK_ENABLED` — package-authored
178+
// scheduled work is OFF by default in every posture, so without it
179+
// NOTHING arms and the `precondition: the sweep BOUND` case below
180+
// fails, taking every assertion built on it with it. ⛔ It is a
181+
// PRECONDITION of this file's subject, not a convenience: what these
182+
// pins measure is which rows an ARMED sweep selects, and an unarmed
183+
// sweep selects nothing for a reason that has nothing to do with
184+
// tenancy.
185+
// 2. `OS_TENANCY_POSTURE=isolated` — the acting-organization
186+
// declaration this sweep carries is REQUIRED only behind a wall.
187+
// Under `single` the same flow arms while declaring nothing and
188+
// sweeps unscoped, which is a different subject with a different
189+
// correct answer.
190+
//
191+
// ⚠️ Set around the BIND, not around `bootStack`: both triggers read
192+
// these live at `start()`, while booting the STACK under a wall would
193+
// demand the enterprise organizations plugin this suite deliberately
194+
// does not install (ADR-0093 D5 refuses to boot a wall it cannot
195+
// enforce). Nothing the pins measure moves: which rows the sweep selects
196+
// is decided by the two `sys_organization` rows and the declaration.
197+
priorSwitch = process.env[SCHEDULED_WORK_ENV];
198+
priorPosture = process.env.OS_TENANCY_POSTURE;
199+
process.env[SCHEDULED_WORK_ENV] = 'true';
200+
process.env.OS_TENANCY_POSTURE = 'isolated';
201+
168202
automation.registerFlow(SWEEP_FLOW, declaringTimeRelativeFlow(orgA, recipientId));
169203

170204
job = fakeJobService();
@@ -174,6 +208,12 @@ for (const databaseDriver of ['sqlite-wasm', 'memory'] as const) {
174208
}, 120_000);
175209

176210
afterAll(async () => {
211+
// [#17396] Restore the PREVIOUS values rather than deleting the keys — a
212+
// CI box that exported either one must be left exactly as it was found.
213+
if (priorSwitch === undefined) delete process.env[SCHEDULED_WORK_ENV];
214+
else process.env[SCHEDULED_WORK_ENV] = priorSwitch;
215+
if (priorPosture === undefined) delete process.env.OS_TENANCY_POSTURE;
216+
else process.env.OS_TENANCY_POSTURE = priorPosture;
177217
await stack?.stop();
178218
});
179219

@@ -184,8 +224,94 @@ for (const databaseDriver of ['sqlite-wasm', 'memory'] as const) {
184224
it('precondition: the sweep BOUND', () => {
185225
expect(
186226
job.has(SWEEP_JOB),
187-
`the sweep did not bind — registered jobs: ${job.names().join(', ') || '(none)'}`,
227+
`the sweep did not bind — registered jobs: ${job.names().join(', ') || '(none)'}`
228+
+ ` (⚠️ #17396: this is also the case that fails when ${SCHEDULED_WORK_ENV} is not set —`
229+
+ ' package-authored scheduled work is off by default in every posture, and an unarmed'
230+
+ ' sweep selects nothing for a reason that has nothing to do with tenancy)',
231+
).toBe(true);
232+
});
233+
234+
// ── [#17396] The OTHER deployment state, which ruling G item 6 requires
235+
// and nothing measured before this card ────────────────────────────
236+
//
237+
// With the switch OFF neither trigger arms anything, and every such flow is
238+
// listed in `getTriggerBindingAudit()` — the surface the automation
239+
// plugin's `kernel:bootstrapped` warning, the CLI startup summary and
240+
// Studio all read — with a DISTINCT reason: *disabled by deployment
241+
// policy*, ⛔ NEVER "binding failed".
242+
//
243+
// ⭐ That distinction is the whole of the ruled item, and it is not
244+
// cosmetic: a binding failure is a defect with an engineering remedy, while
245+
// this is a deployment policy with an operator remedy, and the two send
246+
// whoever reads the boot summary to different places. It is pinned HERE,
247+
// on the real engine with a real registered trigger, because the engine's
248+
// own catch — the one that writes "binding failed" — is the thing that must
249+
// NOT be reached.
250+
it('[#17396] switch OFF: the sweep does not arm, and the audit says disabled by deployment policy', async () => {
251+
const OFF_FLOW = `${SWEEP_FLOW}_policy_off`;
252+
const OFF_JOB = `flow-time-relative:${OFF_FLOW}`;
253+
const restore = process.env[SCHEDULED_WORK_ENV];
254+
try {
255+
delete process.env[SCHEDULED_WORK_ENV];
256+
automation.registerFlow(OFF_FLOW, declaringTimeRelativeFlow(orgA, recipientId));
257+
await new Promise<void>((r) => setTimeout(r, 0));
258+
} finally {
259+
if (restore === undefined) delete process.env[SCHEDULED_WORK_ENV];
260+
else process.env[SCHEDULED_WORK_ENV] = restore;
261+
}
262+
263+
// ⛔ The flow is well-formed and DECLARES its organization — the same
264+
// fixture the armed sweep above uses. Nothing about it is wrong; the
265+
// deployment simply has not asked for scheduled work.
266+
expect(
267+
job.has(OFF_JOB),
268+
`a policy-disabled flow must have no job at all — registered: ${job.names().join(', ') || '(none)'}`,
269+
).toBe(false);
270+
271+
const states = automation.getFlowRuntimeStates() as Array<{ name: string; bound: boolean }>;
272+
expect(
273+
states.find((st: { name: string }) => st.name === OFF_FLOW)?.bound,
274+
"Studio's status badge must not report this flow as armed",
275+
).toBe(false);
276+
expect(
277+
states.find((st: { name: string }) => st.name === SWEEP_FLOW)?.bound,
278+
'control: the sweep armed while the switch was ON must still read as bound, or this pin would pass with everything broken',
188279
).toBe(true);
280+
281+
const audit = automation.getTriggerBindingAudit() as Array<{
282+
flowName: string;
283+
triggerType: string;
284+
reason: string;
285+
}>;
286+
const entry = audit.find((a: { flowName: string }) => a.flowName === OFF_FLOW);
287+
expect(
288+
entry,
289+
`ruled item 6: the flow must be LISTED, so the boot summary names it; audit: ${JSON.stringify(audit)}`,
290+
).toBeTruthy();
291+
expect(entry!.triggerType).toBe('time_relative');
292+
expect(
293+
entry!.reason,
294+
'the reason must be the one sentence every surface shares, so the audit, the CLI summary and Studio cannot drift',
295+
).toBe(SCHEDULED_WORK_DISABLED_REASON);
296+
expect(entry!.reason, 'and it must name the switch the operator has to set').toContain(SCHEDULED_WORK_ENV);
297+
// ⭐ The prohibition, pinned by absence because the branch it must not
298+
// take produces exactly this phrase.
299+
expect(
300+
entry!.reason,
301+
'ruled item 6: a policy-disabled flow is ⛔ NEVER reported as a binding failure',
302+
).not.toMatch(/binding failed/);
303+
expect(
304+
audit.map((a: { flowName: string }) => a.flowName),
305+
'control: the armed sweep must not be listed as a silent miss',
306+
).not.toContain(SWEEP_FLOW);
307+
308+
// And the trigger was never asked: with the switch off the engine does
309+
// not call `start()` at all, so nothing threw and nothing was logged as
310+
// a failure.
311+
expect(
312+
log.errors.filter((l) => l.includes(OFF_FLOW)),
313+
'a deployment running the configuration it asked for must not print an error',
314+
).toEqual([]);
189315
});
190316

191317
if (databaseDriver === 'memory') {

packages/services/service-automation/src/engine.test.ts

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3426,6 +3426,62 @@ describe('AutomationEngine - the deployment switch (#17396)', () => {
34263426
expect(audit[0].reason).not.toBe(SCHEDULED_WORK_DISABLED_REASON);
34273427
});
34283428

3429+
it('the audit reports what HAPPENED, not what the environment says when it is read', () => {
3430+
// ⭐ REGRESSION PIN. The first spelling re-derived the reason inside
3431+
// `getTriggerBindingAudit()` from a live `resolveScheduledWorkPolicy()`
3432+
// read. The audit is read long after the bind — `kernel:bootstrapped`,
3433+
// the CLI startup summary, every Studio poll — so an environment that
3434+
// moved in between made it report `binding failed — see earlier
3435+
// warnings` for a flow whose trigger was NEVER CALLED, pointing the
3436+
// reader at warnings that do not exist. That is precisely the reading
3437+
// ruled item 6 forbids, reached by a route the ruling's own words do
3438+
// not describe. Caught by the dogfood sweep suite, pinned here.
3439+
delete process.env[SCHEDULED_WORK_ENV];
3440+
const engine = new AutomationEngine(createTestLogger());
3441+
const rec = recordingTrigger('schedule');
3442+
engine.registerTrigger(rec.trigger);
3443+
engine.registerFlow('digest', scheduleFlow('digest'));
3444+
expect(rec.started, 'control: the flow really was refused by policy').toHaveLength(0);
3445+
3446+
// The environment moves, and nothing re-registers the flow.
3447+
process.env[SCHEDULED_WORK_ENV] = 'true';
3448+
3449+
const audit = engine.getTriggerBindingAudit();
3450+
expect(audit.map((a) => a.flowName)).toEqual(['digest']);
3451+
expect(
3452+
audit[0].reason,
3453+
'the flow is still unarmed because the policy refused it — the switch moving later does not turn that into a binding failure',
3454+
).toBe(SCHEDULED_WORK_DISABLED_REASON);
3455+
expect(audit[0].reason).not.toMatch(/binding failed/);
3456+
});
3457+
3458+
it('a flow that gets past the gate drops the record, so the reason is its own', () => {
3459+
// The other direction, and what keeps the record from becoming a
3460+
// permanent label: once the switch is on and the flow is registered
3461+
// again, whatever happens next owns the reason.
3462+
delete process.env[SCHEDULED_WORK_ENV];
3463+
const engine = new AutomationEngine(createTestLogger());
3464+
engine.registerFlow('digest', scheduleFlow('digest'));
3465+
expect(engine.getTriggerBindingAudit()[0]?.reason).toBe(SCHEDULED_WORK_DISABLED_REASON);
3466+
3467+
process.env[SCHEDULED_WORK_ENV] = 'true';
3468+
// Registering the trigger re-attempts activation for every flow.
3469+
engine.registerTrigger({
3470+
type: 'schedule',
3471+
start() {
3472+
throw new Error('the job service refused');
3473+
},
3474+
stop() {},
3475+
});
3476+
3477+
const audit = engine.getTriggerBindingAudit();
3478+
expect(audit).toHaveLength(1);
3479+
expect(
3480+
audit[0].reason,
3481+
'a real bind failure after the gate opened must read as one — the policy record must not outlive the policy',
3482+
).toMatch(/binding failed/);
3483+
});
3484+
34293485
it('is read at BIND, not cached, so flipping the switch changes the next registration', () => {
34303486
// The CLI's `--fresh` harness and any test that flips the switch
34313487
// between kernels in one process depend on this.

0 commit comments

Comments
 (0)