diff --git a/.changeset/schedule-trigger-acting-organization.md b/.changeset/schedule-trigger-acting-organization.md new file mode 100644 index 0000000000..e64819ac1b --- /dev/null +++ b/.changeset/schedule-trigger-acting-organization.md @@ -0,0 +1,129 @@ +--- +"@objectstack/spec": minor +"@objectstack/service-automation": minor +"@objectstack/trigger-schedule": minor +"@objectstack/lint": minor +--- + +fix(triggers,spec,service-automation,lint)!: a time-triggered flow declares its acting organization, and both its query and its run are confined to it (#16659) + + + +**Registered as an ADR-0087 semantic migration** +(`schedule-flow-acting-organization-required`, protocol 18). Nothing authorable +is renamed, retired or re-typed — no `packages/spec` key changes its name, its +type or its optionality, no stored shape moves, and every flow, node and +start-node `config` that parses today parses byte-identically afterwards, +because the start node's `config` is an OPEN record (ADR-0018) and the new +`organization` key is an addition to a slot that already accepted anything. So +`objectstack migrate meta` has nothing MECHANICAL to prescribe: the remedy is a +value only the deployment holds, a `sys_organization.id` minted at runtime, with +no authored artifact and no stored representation a rewrite could act on — and +inventing one is precisely what the ruling forbids. ⚠️ That is the argument +against a CONVERSION, and it is not an argument for silence: ADR-0087 D3 says a +migration that cannot be expressed declaratively gets a structured TODO +(surface, reason, acceptance criteria) rather than nothing, and what follows IS +a prescription in that sense — declare `config.organization` once per +organization, no fan-out, then act on the three consequences of the split named +below. Direct precedent: `rest-requireauth-default-flip` (protocol 12) — +behaviour-only, no shape moved, a deployment judgement no transform can make, +registered anyway. Filed under protocol **18**, not 17: v17.0.0 was cut before +this narrowing landed, so the enforcement rides the 17.x line by the +launch-window convention while the prescription belongs at the major boundary +where `migrate meta` users look. + +**BREAKING** in the accept-set sense, and in TWO places rather than one — +landing in the launch window as `minor` on all four packages (the lockstep +convention: during the window the bump level is not the carrier, this banner and +the disposition above are). Nothing that was refused becomes admitted. + +1. **Bind time.** A `schedule` or `time_relative` flow that declares no + `organization` is no longer armed. +2. **Run time — the DATA PLANE.** A time-triggered run now carries a + `tenantId`, and a `time_relative` sweep now carries one on its own query. + Where a run previously read, updated and deleted across every organization, + it is now confined to the one it declares. + +⚠️ **Read (2) as a narrowing that can stop something that was working**, because +it is one. Two shapes to plan for, and neither is hypothetical: + +- **A deployment running ONE time-triggered flow to cover ALL organizations must + now declare one flow per organization.** That is the ruling + (「不允许跨组织的定时任务」) and it is the whole point, but it is migration + work: there is no fan-out, and a sweep wanted in N organizations is N + declarations. Nothing detects the shape for you — the flow simply starts + seeing one organization's rows. + + ⚠️ **And the split has three effects the sentence above does not carry.** Each + is deployment work, and none of them is detected for you either: + + 1. **A NULL-organization row fans out N-fold.** The driver's scope is + `org = :tenant OR org IS NULL` (`sql-driver.ts`), so a platform row with no + tenant column value stays visible to a *scoped* read — this PR's own + negative control fixture selects exactly that row under scope, on purpose. + After the split every `organization_id IS NULL` row in a swept object is + therefore matched **once per flow**: N runs, N notifications, each acting + as a different organization. Before the split it was matched once. ⇒ Either + backfill the tenant column on swept objects or declare the object + platform-global (`tenancy: { enabled: false }`, ADR-0066), which stops the + scope rather than multiplying under it. + 2. **The current window's dispatch claims are abandoned.** The dedup key + embeds the FLOW NAME — `schedule::` and + `time-relative:::` — so N differently-named + flows claim under N different keys. A window already delivered under the + old name can deliver again, once, under each new one. ⇒ Cut over at a + window boundary, or accept one duplicate window. + 3. **A run suspended before the upgrade is not retroactively confined.** + Resume rebuilds the run's context from `context_json` + (`suspended-run-store.ts`), and a row written before this change carries no + `tenantId` — so it resumes org-less, exactly as it ran. Nothing back-fills + it. Not a regression (that is how it already ran), but the banner would + otherwise imply "after upgrade, runs are confined". ⇒ Drain in-flight + suspended time-triggered runs, or accept that the tail of them is + unconfined. +- **On a SINGLE-organization install a time-triggered flow WAS delivering** — + the #8844 guard derives the only organization there — and after this change it + is unarmed at boot until someone adds one line. On `@objectstack/driver-sql` + that install loses nothing at run time once the line is added: the scope is + `org = :tenant OR org IS NULL` and its one organization is the only scope there + was. ⛔ **On `@objectstack/driver-memory` it does lose something, and the loss + has no legal configuration.** That driver refuses *any* call handed a tenant + scope (`assertCallNotTenantScoped`, `MEMORY_MULTI_TENANT_UNSUPPORTED`, #16589) + — `find` / `findOne` / `create` / `update` / `upsert` / `delete` / `count` / + `bulk*` / `aggregate`, one call at a time, regardless of how many + organizations the install holds. So a time-triggered flow that touches + per-organization data on that driver is refused per call if it declares an + organization and unarmed at boot if it does not. The declaration is not what + breaks it — the driver has no row-level tenant isolation to offer either way — + but this change is what moves such a flow from the "no organization context at + all → served" case into the refused one. Multi-organization deployments use + `@objectstack/driver-sql`; a `driver-memory` install whose swept objects are + genuinely platform-global can declare them so (`tenancy: { enabled: false }`, + ADR-0066) and is served unchanged, and ⛔ that is not a way to silence the + refusal on data that really is per-organization. + +A `type: 'schedule'` flow and a `time_relative` sweep now declare their acting organization on the start node, and the run executes as that organization. + +Maintainer ruling, 2026-09-08, verbatim: 「多组织定时任务本来只能在组织内运行，应该带组织ID，不允许跨组织的定时任务。」 + +A time-triggered flow launches its run from a job tick, and a job tick carries no identity, so `ScheduleTrigger` and `TimeRelativeTrigger` built an `AutomationContext` with no `tenantId`. Two consumers already read that key and both resolved NULL: `notify-node.ts` threads it onto the notification it emits (#11303), and `AutomationEngine.recordLog` copies it onto the `sys_automation_run` history row (#10101). On an install holding more than one `sys_organization` the #8844 guard then refused every tenant-scoped row beneath the run — `sys_inbox_message`, `sys_notification_delivery`, `sys_notification_receipt` and the history row — one layer BELOW anything that summarises a run. So the tick selected its rows, landed its `update_record` steps, reported `unmeasured=0`, and delivered nothing. + +- **`@objectstack/spec`** declares the start-node `config.organization` key (`schedule-organization.zod.ts`): `SCHEDULE_ORGANIZATION_KEY`, `ScheduleOrganizationSchema`, the `ScheduleOrganization` type, `resolveScheduleOrganization` and `describeMissingScheduleOrganization` — five names, so the engine's lift and both triggers cannot drift about what counts as declared. The near-miss scan is module-local and runs INSIDE the refusal sentence (`describeMissingScheduleOrganization(flowName, { kind, config })`): both callers only ever wanted the sentence, and a `minor` freezes what it publishes — removing an export later is breaking where adding one is not. +- **`@objectstack/lint`** teaches `validate-flow-trigger-readiness` the requirement, so an author learns at authoring time rather than from a production stderr line at boot. It re-implements no judgement: `resolveFlowTriggerKind` says which flows owe the key and `resolveScheduleOrganization` says whether one was declared, which are the same two answers the triggers refuse with. Severity `warning`, not `error` — see **The four flows this repo itself ships** below. +- **`@objectstack/service-automation`** lifts the declaration onto the `schedule` / `time_relative` binding, beside `schedule`. `record_change` and `api` bindings leave it `undefined` by construction: both are fired by a caller who already carries an organization, and lifting a declared one onto them would let a flow overrule the tenant of the write that triggered it. +- **`@objectstack/trigger-schedule`** refuses to bind a time-triggered flow that declares none — at `error`, naming the flow, and dropping any prior binding so a hot re-publish that REMOVES the key cannot leave the previous job armed — and threads the declared organization onto the run as `tenantId`, **and onto the `time_relative` sweep's own query**. The refusal is **thrown** from `start()`, not merely logged: `FlowTrigger.start` returns `void`, so a logged-and-returned refusal leaves the engine free to record the flow as bound. Thrown, it takes the engine's designed catch path — the flow is never marked bound, `getFlowRuntimeStates()` reports `bound: false`, and `getTriggerBindingAudit()` lists it, so the `kernel:bootstrapped` warning and the CLI startup summary both name it. + +**What an existing deployment feels.** A scheduled or time-relative flow with no `organization` stops being armed at boot; the log line names the flow, the key, where the key goes, and — when the author wrote a near-miss (`organizationId`, `tenantId`, `orgId`, …) — which spelling of theirs the open `config` record accepted and then ignored. On a SINGLE-organization install such a flow was working, because the #8844 guard derives the only organization there; it now needs one line to say so. That cost is the ruling's, not an implementation choice: "declared = enforced" is what makes the multi-organization case safe, and a posture-conditional refusal would leave a flow that is legal on a one-organization install and silently inert the day a second organization is created — which is the defect being closed, moved one step later. + +⛔ Nothing on this path ever CHOOSES an organization — not the install's only one, not the platform organization, not the first row of `sys_organization`, not the swept record's own `organization_id`. (The trigger does read the declared value from two places, the lifted binding field and the raw start-node `config`; that is one value read twice, so an engine predating the lift reports a correctly declared flow as declared instead of turning a version skew into an authoring error. It resolves nothing the author did not write.) A wrong `organization_id` is worse than a refusal: a refusal is visible at boot and names its flow, while a wrong value is silently authoritative to every report, export and cleanup that filters by organization. ⛔ There is no fan-out either: a sweep wanted in N organizations is declared N times, and a single flow never spans them. + +**Run-history volume is bounded by a contract that already exists.** Scheduled runs now persist to `sys_automation_run` where they previously could not, and that table's retention is two-sided and declared: a per-flow cap on terminal rows enforced at WRITE time (`runHistoryMaxPerFlow`, default 100) and declarative age retention (`retention: { maxAge: '30d', onlyWhen: { status: { $in: ['completed', 'failed'] } } }`, ADR-0057 / #2834, with `paused` rows retained regardless of age). A minute-cadence flow is bounded by the per-flow cap, not by the tick rate. Measured before landing this: nothing in the tree depends on scheduled runs NOT reaching `sys_automation_run` — no test asserts an absent or zero run-history row for a time-triggered flow, and no deployment config, migration or quota keys off that emptiness. + +No object's tenancy declaration changes, and `NotifyConfigSchema` is untouched — the two routes the ruling excluded. `system-write-organization.ts` stays exactly as it is: the producer it guards against now carries what it demands. + +**What the declaration now bounds, precisely.** The value goes onto the run's `AutomationContext.tenantId`, and — for a `time_relative` sweep — onto its `find` context as well. From there it is the platform's existing tenancy path and nothing new: `Engine.buildDriverOptions` turns `context.tenantId` into `DriverOptions.tenantId`, and the driver scopes reads, updates, deletes and aggregates to that organization. ⛔ No `organization_id` predicate is hand-built anywhere — that would be a second implementation of tenancy inside a trigger, hardcoding a column an object is free to rename, selecting nothing on a platform-global object and breaking a federated one. Two consequences follow from using the platform's mechanism rather than a private one, and both are stated rather than discovered: + +- **A store that cannot scope refuses the call instead of answering it.** `@objectstack/driver-memory` implements no row-level tenant isolation and refuses any call handed a tenant scope (`MEMORY_MULTI_TENANT_UNSUPPORTED`, #16589), so a time-triggered flow on that driver fails loudly rather than quietly crossing organizations. Multi-organization deployments use `@objectstack/driver-sql`; this is the same refusal that driver already gives every other org-scoped read. +- **On a platform-global (`tenancy: { enabled: false }`, ADR-0066) or federated (ADR-0015) object the declaration cannot narrow anything** — the engine drops the scope for those by design. Such a sweep still selects across every organization while its runs act as the declared one, and the trigger says so at bind, at `warn`, naming the object. ⛔ It does not pretend the flow is contained. + +**The four flows this repo itself ships stop firing, and cannot be repaired by authoring.** `showcase_scheduled_digest` and `showcase_task_due_reminder` (`examples/app-showcase`), `task_reminder` and `overdue_escalation` (`examples/app-todo`) are all time-triggered and none declares an organization. There is no value they COULD declare: organization ids are minted per install at runtime, so a package-shipped flow has nothing to write there, and ⛔ inventing a placeholder is strictly worse than the omission — a value matching no row is silently authoritative. Each of the four now carries a comment saying it does not fire as shipped and why. What a package-shipped time-triggered flow should do instead is an open maintainer decision, tracked on #17396; this changeset and those comments are the record until it is ruled. That corpus is also why the new lint id is a `warning`: at `error` it gates `objectstack build`, which was run and refuses `examples/app-showcase` outright — the repo would be unable to build its own examples for a defect they have no way to fix. diff --git a/content/docs/automation/flows.mdx b/content/docs/automation/flows.mdx index 9a21577172..49af08d9f7 100644 --- a/content/docs/automation/flows.mdx +++ b/content/docs/automation/flows.mdx @@ -1934,6 +1934,9 @@ export const contractExpirationCheck: Flow = { config: { triggerType: 'schedule', schedule: { type: 'cron', expression: '0 0 * * *', timezone: 'UTC' }, + // REQUIRED on every time-triggered flow — see "The acting + // organization" below. A flow that omits it is refused at bind. + organization: '', }, }, { id: 'find_expiring', type: 'get_record', label: 'Find Expiring Contracts' }, @@ -1975,6 +1978,9 @@ export const renewalReminder: Flow = { offsetDays: [60, 30, 7], // — or — withinDays: 30 (negative = overdue lookback) filter: { status: 'active' }, // optional, ANDed with the date window }, + // REQUIRED, and for a stronger reason than a plain schedule flow — + // see "The acting organization" below. + organization: '', // schedule: { type: 'cron', expression: '0 8 * * *' } // optional; defaults to daily 08:00 UTC }, }, @@ -1993,6 +1999,80 @@ required. Requires the `triggers` **and** `job` capabilities. The record is on the flow context (`record.*`), so the start `condition` and `{record.*}` interpolation work as in a record-change flow. +### The acting organization + +A `record_change` or `api` flow inherits its organization from whoever triggered +it: the caller's session rides into the run and every tenant-scoped write below +resolves the same organization a normal write would. A **time-triggered** flow +has no such caller — a job tick carries no identity at all. + +So a `schedule` or `timeRelative` flow **declares the organization it runs as**, +on the start node's `config`, beside the cadence it scopes: + +```typescript +config: { + schedule: { type: 'cron', expression: '0 8 * * *' }, + organization: 'org_msokm9oaz0cal87q', // a sys_organization.id +} +``` + +The run then executes as that organization: `tenantId` carries it, the +notifications a `notify` node emits land in that organization's inboxes, the +`sys_automation_run` history row is stamped with it, and every record the run +reads, updates or deletes is confined to it. + +**For a `timeRelative` sweep the declaration also bounds the QUERY.** A sweep +runs elevated on purpose — a background sweep must see every row rather than the +RLS-scoped subset an absent user would see — so nothing else keeps its selection +inside one organization. The declared id rides the sweep's own query, and the +driver scopes the read: a sweep declared for one organization matches that +organization's rows and launches a run per match. Elevation and tenancy are +independent axes, and a sweep carrying only the first is a cross-organization +scheduled task however its runs are stamped. + + +Two cases where a declaration cannot deliver containment, and both say so rather +than going quiet. A store with no row-level tenant isolation +(`@objectstack/driver-memory`) **refuses** a scoped sweep instead of answering it +unscoped, and the failure is logged at `error` naming the flow — multi-organization +deployments use `@objectstack/driver-sql`. And on a platform-global object +(`tenancy: { enabled: false }`) or a federated one, the engine applies no tenant +scope at all, so the sweep still selects across every organization while its runs +act as the declared one; the trigger warns at bind, naming the object. + + +`os lint`, `os validate` and `objectstack build` report a missing declaration as +a warning (`flow-schedule-organization-missing`) so it is visible at authoring +time rather than only in a server log at boot. + +**A time-triggered flow that declares none is a declaration error**, refused at +bind: + +- the trigger logs the reason at `error`, naming the flow; +- the flow is recorded as **not bound** — it is listed by + `getTriggerBindingAudit()`, warned at `kernel:bootstrapped`, printed in the + `os dev` / `os start` startup summary, and `getFlowRuntimeStates()` reports + `bound: false`; +- nothing fires it. + +There is deliberately **no fallback** — not the platform organization, not "the +install's only one". Without the declaration the run would reach every +tenant-scoped write with nothing to offer, and on an install holding more than +one `sys_organization` each of those writes is refused one layer below anything +that summarises the run: the tick reports itself healthy and delivers nothing. +A wrong `organization_id` is worse still, because it is silently authoritative +to every report, export and cleanup script that filters by organization. + +**No fan-out.** A single flow belongs to one organization. A sweep wanted in +several organizations is declared once per organization. + + +The start node's `config` is an open record, so a near-miss spelling — +`organizationId`, `organization_id`, `orgId`, `org_id`, `tenantId` — parses +happily and is then ignored. The bind-time refusal names the spelling you +actually wrote. + + ### Update-triggered flow Trigger on a record update and compare against the previous value: diff --git a/content/docs/automation/hooks.mdx b/content/docs/automation/hooks.mdx index 3b7ba885b4..9d3f980273 100644 --- a/content/docs/automation/hooks.mdx +++ b/content/docs/automation/hooks.mdx @@ -24,7 +24,7 @@ cannot express. | :--- | :--- | | Side effects after a save — create records, notify, call HTTP, request approval | **Flow** (`record_change`) | | Anything that pauses: approvals, screens, timers, signals | **Flow** — a hook runs inline and cannot pause | -| Scheduled or date-relative sweeps ("30 days before `end_date`") | **Flow** (`schedule` / `timeRelative`) | +| Scheduled or date-relative sweeps ("30 days before `end_date`") | **Flow** (`schedule` / `timeRelative`) — which [declares the organization it runs as](/docs/automation/flows#the-acting-organization) | | Mutating the pending record in the same write, before it is saved | **Before hook** | | An invariant enforced on every write path, across objects, no matter who writes | **Hook** — the backstop duty itself | | Read-side interception (`beforeFind` / `afterFind`) | **Hook** — flows have no read events | diff --git a/content/docs/getting-started/quick-reference.mdx b/content/docs/getting-started/quick-reference.mdx index f906dcf642..6fa60c86ec 100644 --- a/content/docs/getting-started/quick-reference.mdx +++ b/content/docs/getting-started/quick-reference.mdx @@ -150,7 +150,7 @@ REST endpoints, real-time subscriptions, and discovery. | **[Metadata](/docs/references/api/metadata)** | `metadata.zod.ts` | Metadata | API metadata endpoints | | **[Storage](/docs/references/api/storage)** | `storage.zod.ts` | Storage | API storage operations | -## Automation Protocol (4 of 13 schemas) +## Automation Protocol (4 of 14 schemas) Flows, state machines, approvals, and integrations. diff --git a/content/docs/permissions/capabilities.mdx b/content/docs/permissions/capabilities.mdx index 3612bbc92b..b093ce8940 100644 --- a/content/docs/permissions/capabilities.mdx +++ b/content/docs/permissions/capabilities.mdx @@ -25,7 +25,7 @@ Read the next section before you write either. | **Vocabulary** | Author-chosen names, `^[a-z][a-z0-9_.]*$` — `export_data`, `billing.refund` | A **closed** vocabulary: canonical kebab-case tokens from `PLATFORM_CAPABILITY_TOKENS` — `ai`, `automation`, `hierarchy-security` | | **Entry shape** | `defineCapability({ name, label, description, scope })` (`CapabilityDeclarationSchema`) | A plain `string` | | **Unknown value** | There is no "unknown" — you are minting the name | A `defineStack` **error** at authoring time (a typo, or a token no runtime provides) | -| **Needed but undeclared** | Nothing to detect — a name is minted here, then granted | A `defineStack` **error** too: a hierarchy scope (`unit` / `unit_and_below` / `own_and_reports`) needs `hierarchy-security`, and a `record_change` / `schedule` / `time_relative` / `api` flow needs `triggers` — without them the runtime fails closed (owner-only visibility) or, for flows, silently never fires | +| **Needed but undeclared** | Nothing to detect — a name is minted here, then granted | A `defineStack` **error** too: a hierarchy scope (`unit` / `unit_and_below` / `own_and_reports`) needs `hierarchy-security`, and a `record_change` / `schedule` / `time_relative` / `api` flow needs `triggers` — without them the runtime fails closed (owner-only visibility) or, for flows, silently never fires. ⚠️ `triggers` is a *capability*, not the whole declaration: a `schedule` / `time_relative` flow also [declares the organization it runs as](/docs/automation/flows#the-acting-organization), and one that does not is refused at bind rather than fired org-less | | **Consumed by** | `systemPermissions` (grant) and `requiredPermissions` (requirement), by name string | The runtime capability loader, which resolves each token to a service plugin | | **When it bites** | Never at boot — an ungranted capability is simply held by nobody | **Fail-fast at startup**: a declared-but-missing provider aborts boot instead of degrading silently | | **Spec** | ADR-0066 D1 | Platform service vocabulary — see the [CLI reference](/docs/deployment/cli) | diff --git a/content/docs/references/automation/index.mdx b/content/docs/references/automation/index.mdx index 09525b3036..85148c1ec9 100644 --- a/content/docs/references/automation/index.mdx +++ b/content/docs/references/automation/index.mdx @@ -17,6 +17,7 @@ This section contains all protocol schemas for the automation layer of ObjectSta + diff --git a/content/docs/references/automation/meta.json b/content/docs/references/automation/meta.json index c9990e2740..b32a7ec940 100644 --- a/content/docs/references/automation/meta.json +++ b/content/docs/references/automation/meta.json @@ -17,6 +17,7 @@ "builtin-node-config", "flow-function", "io-node-config", + "schedule-organization", "schemaless-node-config" ] } \ No newline at end of file diff --git a/content/docs/references/automation/schedule-organization.mdx b/content/docs/references/automation/schedule-organization.mdx new file mode 100644 index 0000000000..0932fd4caa --- /dev/null +++ b/content/docs/references/automation/schedule-organization.mdx @@ -0,0 +1,109 @@ +--- +title: Schedule Organization +description: Schedule Organization protocol schemas +--- + +{/* ⚠️ AUTO-GENERATED — DO NOT EDIT. Run build-docs.ts to regenerate. Hand-written docs live in the module folders under content/docs/. */} + +The ACTING ORGANIZATION of a time-triggered flow — the one start-node key +that says which organization a scheduled run executes as. + +## Why the key exists + +A record-change flow inherits its organization from the write that fired it: +the triggering session's `tenantId` rides the `AutomationContext` into +the run, so every tenant-scoped write below it — `sys_inbox_message`, +`sys_notification_delivery`, `sys_automation_run` — resolves an organization +the way a session write does. A TIME-triggered flow has no such session. The +schedule trigger and the time-relative sweep launch their runs from a job +tick, and a job tick carries no identity at all, so the run reached the +tenancy guard (`system-write-organization.ts`) with nothing to offer it. On +an install holding more than one `sys_organization` that guard refuses, +correctly and by design — and the refusal landed on rows the run never +reported: the notification wrote with `organization_id = NULL`, every +tenant-scoped row beneath it was refused, and the tick still summarised +itself as healthy. + +## The ruling this key implements + +Maintainer, 2026-09-08, verbatim: + +> 多组织定时任务本来只能在组织内运行，应该带组织ID，不允许跨组织的定时任务。 + +A time-triggered flow is **organization-scoped by construction**: it names +one organization and the run executes as that organization. There is +deliberately no fan-out — a tenant that wants the same sweep in N +organizations declares it N times — and there is deliberately no fallback: a +flow that names none is a DECLARATION ERROR, not a run that quietly picks +one. Guessing is the failure this key exists to prevent, and the platform +organization is not a safe guess: a wrong `organization_id` is worse than a +null, because a null is visibly missing while a wrong value is silently +authoritative to every report, export and cleanup script that filters by +organization. + +## Where it lives, and why there + +On the flow's START node `config`, beside the cadence it scopes: + +```ts +config: { + schedule: { type: 'cron', expression: '0 8 * * *' }, + organization: 'org_msokm9oaz0cal87q', +} +``` + +The start node is where every other trigger-binding fact already lives — +`FlowSchema` refuses a top-level `schedule` in as many words ("a schedule +flow declares its cron/interval as `config.schedule` on the START node, not +at the flow top level"), and `resolveTriggerBinding` hands the whole start +`config` to the trigger. Putting the organization at the flow top level would +split one binding across two layers; putting it inside the `schedule` +descriptor would make it invisible to the time-relative sweep, which carries +its cadence in the same slot but binds through a different descriptor. One +key, one layer, both time triggers. + +## Why `ScheduleOrganization…` and not `FlowActingOrganization…` + +The key governs both trigger kinds, and `FlowTriggerKind` lists +`time_relative` and `schedule` as two of its four members — so the name looks +inaccurate for half its subjects. It is not, and the deciding reading is the +AUTHORABLE surface rather than the derived kind: `FlowSchema.type` is +`z.enum(['autolaunched', 'record_change', 'schedule', 'screen', 'api'])` and +has no `time_relative` member at all. A time-relative sweep is authored as +`type: 'schedule'` with a `timeRelative` descriptor on its start node — the +docs say so in as many words ("a `schedule` flow whose `start` node declares +a `timeRelative` descriptor"), and `TimeRelativeTriggerSchema`'s own opening +line says the trigger "sweeps an object on a schedule". `FlowTriggerKind` +splits the two because the ENGINE routes them to different triggers; its +precedence note distinguishes a sweep from "a plain schedule flow", which is +a split inside the schedule family, not out of it. + +⇒ Every flow this key applies to declares `type: 'schedule'`. The name is +accurate for both subjects, and a `minor` freezes it, so this is recorded +rather than left to be re-litigated. + + +**Source:** `packages/spec/src/automation/schedule-organization.zod.ts` + + +## TypeScript Usage + +```typescript +import { ScheduleOrganizationSchema } from '@objectstack/spec/automation'; +import type { ScheduleOrganization } from '@objectstack/spec/automation'; + +// Validate data +const result = ScheduleOrganizationSchema.parse(data); +``` + +--- + +## ScheduleOrganization + +Organization id (sys_organization.id) this scheduled/time-relative flow runs as. Required: a time-triggered run has no session to inherit a tenant from. + +**Type:** `string` + + +--- + diff --git a/content/docs/references/index.mdx b/content/docs/references/index.mdx index 8d8694f35d..c1ceafa3ec 100644 --- a/content/docs/references/index.mdx +++ b/content/docs/references/index.mdx @@ -1,6 +1,6 @@ --- title: Protocol Reference -description: Every schema published by @objectstack/spec — 1520 schemas across 14 protocol modules +description: Every schema published by @objectstack/spec — 1521 schemas across 14 protocol modules --- {/* ⚠️ AUTO-GENERATED — DO NOT EDIT. Run build-docs.ts to regenerate. Hand-written docs live in the module folders under content/docs/. */} @@ -21,7 +21,7 @@ counts are sums of the rows they head. Regenerate with | :--- | ---: | ---: | :--- | | [AI Protocol](/docs/references/ai) | 11 | 66 | Agents, tools, skills, RAG and knowledge sources, model registry, conversations. | | [API Protocol](/docs/references/api) | 31 | 438 | REST contracts, endpoints, routing, realtime, batch, discovery. | -| [Automation Protocol](/docs/references/automation) | 13 | 73 | Flows and their nodes, approvals, ETL pipelines, webhooks, state machines, execution records. | +| [Automation Protocol](/docs/references/automation) | 14 | 74 | Flows and their nodes, approvals, ETL pipelines, webhooks, state machines, execution records. | | [Data Protocol](/docs/references/data) | 29 | 173 | Objects, fields, queries, filters, datasources and drivers — the ObjectQL layer. | | [Identity Protocol](/docs/references/identity) | 5 | 27 | Users and accounts, organizations, positions, SCIM provisioning. | | [Integration Protocol](/docs/references/integration) | 1 | 24 | The single connector protocol (ADR-0097) — catalog descriptors and provider-bound instances. | @@ -33,7 +33,7 @@ counts are sums of the rows they head. Regenerate with | [Studio Protocol](/docs/references/studio) | 3 | 35 | Studio designer metadata — the authoring surfaces for the protocols above. | | [System Protocol](/docs/references/system) | 33 | 272 | The runtime environment — logging, jobs, cache, metrics, notifications, i18n and compliance. | | [UI Protocol](/docs/references/ui) | 16 | 153 | Apps, pages, views, dashboards, reports, actions and themes — the ObjectUI layer. | -| **Total** | **191** | **1520** | 14 protocol modules | +| **Total** | **192** | **1521** | 14 protocol modules | --- @@ -103,7 +103,7 @@ REST contracts, endpoints, routing, realtime, batch, discovery. ## Automation Protocol -**Source:** `packages/spec/src/automation/` · **Import:** `@objectstack/spec/automation` · **13 pages, 73 schemas** +**Source:** `packages/spec/src/automation/` · **Import:** `@objectstack/spec/automation` · **14 pages, 74 schemas** Flows and their nodes, approvals, ETL pipelines, webhooks, state machines, execution records. @@ -118,6 +118,7 @@ Flows and their nodes, approvals, ETL pipelines, webhooks, state machines, execu | [`flow-function.zod.ts`](/docs/references/automation/flow-function) | `FlowFunctionEffect` | | [`io-node-config.zod.ts`](/docs/references/automation/io-node-config) | `HttpConfig`, `NotifyConfig` | | [`node-executor.zod.ts`](/docs/references/automation/node-executor) | `ActionCategory`, `ActionDescriptor`, `ActionParadigm`, `NodeExecutorDescriptor`, `WaitEventType`, `WaitExecutorConfig`, `WaitResumePayload`, `WaitTimeoutBehavior` | +| [`schedule-organization.zod.ts`](/docs/references/automation/schedule-organization) | `ScheduleOrganization` | | [`schemaless-node-config.zod.ts`](/docs/references/automation/schemaless-node-config) | `DecisionCondition`, `DecisionConfig`, `ScriptConfig`, `SubflowConfig` | | [`state-machine.zod.ts`](/docs/references/automation/state-machine) | `ActionRef`, `GuardRef`, `StateMachine`, `StateNode`, `Transition` | | [`time-relative-trigger.zod.ts`](/docs/references/automation/time-relative-trigger) | `TimeRelativeTrigger` | diff --git a/examples/app-showcase/src/automation/flows/index.ts b/examples/app-showcase/src/automation/flows/index.ts index 40dc48db0a..2b4708eeed 100644 --- a/examples/app-showcase/src/automation/flows/index.ts +++ b/examples/app-showcase/src/automation/flows/index.ts @@ -352,12 +352,34 @@ export const TaskCompletedSlackFlow = defineFlow({ * A `type: 'schedule'` flow whose start node carries an interval descriptor. * The automation engine parses that into a schedule binding; the schedule * trigger plugin (`@objectstack/trigger-schedule`, paired with the job - * service) registers a job that fires this flow every interval. Each tick runs - * the `notify` node, dropping a fresh `sys_inbox_message` row — so the - * scheduled fire is observable end-to-end with no manual `engine.execute()`. + * service) registers a job that fires this flow every interval, and each tick + * runs the `notify` node. * - * Install `requires: ['automation', 'triggers', 'job', 'messaging']` and this - * flow auto-launches on the interval. + * ⛔ AS SHIPPED, THIS FLOW DOES NOT FIRE. Since #16659 a time-triggered flow + * must declare the organization it runs as (`config.organization`, a + * `sys_organization.id`), and a flow that declares none is REFUSED at bind: + * the trigger logs the reason at `error` and throws, the engine records the + * flow as not bound, and it is listed in the startup summary's + * trigger-binding audit. A package-shipped flow has no legal value to write + * there — organization ids are minted at runtime, per install — so this + * example cannot declare one and ⛔ a placeholder id must NOT be invented: a + * value matching no row is silently authoritative, which is strictly worse + * than the refusal. + * + * ⇒ What a package-shipped time-triggered flow should do INSTEAD is an open + * maintainer decision. Its tracking card was destroyed along with a suspended + * account and is being re-filed; until that card carries a number, this + * paragraph is the record. Until it is settled this flow is a worked example of + * the SHAPE, and running it end-to-end means registering it at runtime with an + * `organization` your install actually holds. + * + * `os lint` / `os validate` / `objectstack build` say so too, as a `warning` + * (`flow-schedule-organization-missing`) — deliberately not an `error`, because + * an `error` would refuse this package's own build for a defect it has no + * authorable way to repair. + * + * Install `requires: ['automation', 'triggers', 'job', 'messaging']` for the + * binding machinery this example demonstrates. */ export const ScheduledDigestFlow = defineFlow({ name: 'showcase_scheduled_digest', @@ -1656,6 +1678,21 @@ export const CommitteeQuorumFlow = defineFlow({ * 3 and 1 days before its `due_date`, with the task on the flow context. Swap * `offsetDays` for `withinDays: 7` to nudge everything due within a week * (negative = overdue lookback). + * + * ⛔ AS SHIPPED, THIS SWEEP DOES NOT FIRE — same reason as + * {@link ScheduledDigestFlow}, and it is worth stating separately because a + * sweep is the case where the consequence is largest. Since #16659 a + * `time_relative` flow must declare `config.organization`, and a flow that + * declares none is REFUSED at bind. The declaration is not only the run's + * identity: it is the SWEEP QUERY's scope, so a sweep without one would select + * rows across every organization on the install. That is why there is no + * "fall back to something" path for it to take instead, and why a placeholder + * id ⛔ must not be invented here — a value matching no row is silently + * authoritative. + * + * ⇒ Package-shipped time-triggered flows are the open decision described on + * {@link ScheduledDigestFlow}. Register this sweep at runtime with an + * `organization` your install holds to see it work. */ export const TaskDueReminderFlow = defineFlow({ name: 'showcase_task_due_reminder', diff --git a/examples/app-todo/src/flows/task.flow.ts b/examples/app-todo/src/flows/task.flow.ts index 7f233f399e..23933b6fa2 100644 --- a/examples/app-todo/src/flows/task.flow.ts +++ b/examples/app-todo/src/flows/task.flow.ts @@ -1,8 +1,38 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. +// +// ⛔ NEITHER FLOW IN THIS FILE FIRES AS SHIPPED (#16659). +// +// Both are `type: 'schedule'`. Since #16659 a time-triggered flow must declare +// the organization it runs as — `config.organization`, a `sys_organization.id` +// — and a flow that declares none is REFUSED at bind: the trigger logs the +// reason at `error` and throws, the engine records the flow as NOT bound, and +// it appears in `getTriggerBindingAudit()` and the CLI's startup summary. +// +// A package-shipped flow has no legal value to write there: organization ids +// are minted at runtime, per install. ⛔ A placeholder id must NOT be invented +// — a value matching no row is silently authoritative to every report, export +// and cleanup that filters by organization, which is strictly worse than the +// refusal. +// +// ⇒ What a package-shipped time-triggered flow should do instead is an open +// maintainer decision. Its tracking card was destroyed along with a suspended +// account and is being re-filed; until that card carries a number, this note is +// the record. Until then these two are worked examples of the SHAPE: to run +// either end to end, register it at runtime with an `organization` the install +// actually holds. +// +// `os lint` / `os validate` / `objectstack build` report it as a `warning` +// (`flow-schedule-organization-missing`) — deliberately not an `error`, which +// would refuse this package's own build for a defect it cannot repair. import type { Flow } from '@objectstack/spec/automation'; -/** Task Reminder Flow — scheduled flow to send reminders for upcoming tasks */ +/** + * Task Reminder Flow — scheduled flow to send reminders for upcoming tasks. + * + * ⛔ Does not fire as shipped: it declares no `config.organization`. See the + * file header. + */ export const TaskReminderFlow: Flow = { name: 'task_reminder', label: 'Task Reminder Notification', @@ -54,7 +84,12 @@ export const TaskReminderFlow: Flow = { ], }; -/** Overdue Task Escalation Flow */ +/** + * Overdue Task Escalation Flow. + * + * ⛔ Does not fire as shipped: it declares no `config.organization`. See the + * file header. + */ export const OverdueEscalationFlow: Flow = { name: 'overdue_escalation', label: 'Overdue Task Escalation', diff --git a/packages/lint/src/authoring-rules.ts b/packages/lint/src/authoring-rules.ts index 9757f22c1c..719786c631 100644 --- a/packages/lint/src/authoring-rules.ts +++ b/packages/lint/src/authoring-rules.ts @@ -914,6 +914,14 @@ export const AUTHORING_RULES: readonly AuthoringRule[] = [ // (the object may come from another installed package — a hedge this rule // cannot decide), as did `flow-draft-status-ambiguous` (draft flows DO fire; // that one is ambiguity of intent, not a dead flow). + // + // #16659 added a sixth id, `flow-schedule-organization-missing`, at + // `warning`: a time-triggered flow declaring no `config.organization` is + // refused at bind, so on the criterion above it belongs with the four — and + // it is held at `warning` because an `error` gates `objectstack build`, and + // the repo's own shipped example apps carry such flows with no authorable + // repair (the only legal value is a `sys_organization.id` minted per install + // at runtime). Its own docblock in the rule file records that. { name: 'validateFlowTriggerReadiness', tier: 'gating', diff --git a/packages/lint/src/index.ts b/packages/lint/src/index.ts index 13589e4329..4a86872a97 100644 --- a/packages/lint/src/index.ts +++ b/packages/lint/src/index.ts @@ -120,6 +120,7 @@ export { FLOW_TIME_RELATIVE_DESCRIPTOR_INVALID, FLOW_TIME_RELATIVE_DESCRIPTOR_UNROUTABLE, FLOW_TRIGGER_UNROUTABLE, + FLOW_SCHEDULE_ORGANIZATION_MISSING, } from './validate-flow-trigger-readiness.js'; export type { FlowTriggerReadinessFinding, diff --git a/packages/lint/src/validate-flow-trigger-readiness.test.ts b/packages/lint/src/validate-flow-trigger-readiness.test.ts index 2e21b8f1e2..c5ca1ea145 100644 --- a/packages/lint/src/validate-flow-trigger-readiness.test.ts +++ b/packages/lint/src/validate-flow-trigger-readiness.test.ts @@ -10,6 +10,7 @@ import { FLOW_TIME_RELATIVE_DESCRIPTOR_INVALID, FLOW_TIME_RELATIVE_DESCRIPTOR_UNROUTABLE, FLOW_TRIGGER_UNROUTABLE, + FLOW_SCHEDULE_ORGANIZATION_MISSING, } from './validate-flow-trigger-readiness.js'; function recordFlow(overrides: Record = {}) { @@ -35,6 +36,20 @@ function recordFlow(overrides: Record = {}) { const candidateObject = { name: 'app_candidate', label: 'Candidate', fields: {} }; +/** + * [#16659] Every time-triggered fixture in this file that is ABOUT some other + * rule now declares an acting organization. + * + * `flow-schedule-organization-missing` fires on any `schedule` / + * `time_relative` flow without one, so a fixture that omits it would carry a + * second finding and stop isolating the rule it exists to pin — the tests below + * assert exact finding LISTS, which is what makes them worth having. ⛔ This is + * not a relaxation: the fixtures that are about the missing key omit it + * deliberately, in the `acting organization (#16659)` block and in the severity + * table. + */ +const FIXTURE_ORG = 'org_2mtx1w9d0k4bqf7v'; + describe('validateFlowTriggerReadiness', () => { it('passes a correctly wired, explicitly active record flow', () => { const findings = validateFlowTriggerReadiness({ @@ -118,7 +133,11 @@ describe('validateFlowTriggerReadiness', () => { name: 'digest', type: 'schedule', nodes: [ - { id: 'start', type: 'start', config: { schedule: { type: 'interval', intervalMs: 60000 } } }, + { + id: 'start', + type: 'start', + config: { schedule: { type: 'interval', intervalMs: 60000 }, organization: FIXTURE_ORG }, + }, { id: 'end', type: 'end' }, ], edges: [{ id: 'e1', source: 'start', target: 'end' }], @@ -141,6 +160,7 @@ describe('validateFlowTriggerReadiness', () => { type: 'start', config: { timeRelative: { object: 'contracts', dateField: 'end_date', offsetDays: [60, 30, 7] }, + organization: FIXTURE_ORG, }, }, { id: 'end', type: 'end' }, @@ -166,6 +186,7 @@ describe('validateFlowTriggerReadiness', () => { type: 'start', config: { timeRelative: { object: 'contract', dateField: 'end_date', withinDays: 60 }, + organization: FIXTURE_ORG, }, }, { id: 'end', type: 'end' }, @@ -207,7 +228,7 @@ describe('validateFlowTriggerReadiness', () => { status: 'active', runAs: 'system', nodes: [ - { id: 'start', type: 'start', config: { timeRelative } }, + { id: 'start', type: 'start', config: { timeRelative, organization: FIXTURE_ORG } }, { id: 'end', type: 'end' }, ], edges: [{ id: 'e1', source: 'start', target: 'end' }], @@ -991,11 +1012,17 @@ describe('validateFlowTriggerReadiness', () => { // ── #5762 — the family's severity map ──────────────────────────────────── // // The rules in this file were reviewed as ONE family and split on a single - // question: is this stack enough to know the flow is dead? Three rules answer - // yes and gate; two hedge and advise. The split is the contract, so it is - // pinned as a map rather than as five scattered `severity` lines — a later - // rule added to this file has to decide which side it is on, and a later edit - // that quietly demotes one of the three has to come past this test. + // question: is this stack enough to know the flow is dead? Four rules answer + // yes and gate; three hedge and advise. The split is the contract, so it is + // pinned as a map rather than as scattered `severity` lines — a later rule + // added to this file has to decide which side it is on, and a later edit that + // quietly demotes one of the gating ones has to come past this test. + // + // [#16659] `flow-schedule-organization-missing` is the one entry whose side is + // NOT decided by that question: it answers YES and still advises, because an + // `error` gates `objectstack build` and the repo's own shipped example apps + // carry time-triggered flows with no authorable repair. Its docblock records + // that, and this table is where a later promotion has to come past. // // Every entry is provoked through a real stack, so an id whose criterion stops // firing fails here instead of passing vacuously (the empty-verdict trap: an @@ -1113,6 +1140,25 @@ describe('validateFlowTriggerReadiness', () => { flows: [recordFlow()], }, ], + [ + // [#16659] `warning`, and its own docblock records why the corpus, not + // the verdict, is what holds it there. + FLOW_SCHEDULE_ORGANIZATION_MISSING, + 'warning', + { + objects: [{ name: 'task', label: 'Task', fields: {} }], + flows: [ + { + name: 'orgless_digest', + type: 'schedule', + status: 'active', + nodes: [ + { id: 'start', type: 'start', config: { schedule: { type: 'cron', expression: '0 1 * * *' } } }, + ], + }, + ], + }, + ], ]; for (const [rule, severity, stack] of provoke) { @@ -1159,7 +1205,15 @@ describe('validateFlowTriggerReadiness', () => { { id: 'start', type: 'start', - config: { timeRelative: { object: 'task', dateField: 'due_at', withinDays: 30 } }, + // [#16659] The clean fixture gained `organization`: after this + // card a CORRECT time-triggered flow declares the organization + // it runs as, so a fixture without one is no longer clean — + // the trigger would refuse to bind it. ⛔ Not a relaxation of + // the floor this case guards; the floor moved. + config: { + timeRelative: { object: 'task', dateField: 'due_at', withinDays: 30 }, + organization: 'org_2mtx1w9d0k4bqf7v', + }, }, ], }, @@ -1169,6 +1223,118 @@ describe('validateFlowTriggerReadiness', () => { }); }); + // ─── the acting organization (#16659) ─────────────────────────────────── + // + // `engine.ts`'s trigger-kind resolver states the invariant: it is shared with + // `defineStack`'s trigger-capability refusal and with this file, "so the + // runtime cannot drift from what authoring accepted". A key the two triggers + // REFUSE to bind without, and that authoring never mentions, is that drift — + // an author's first signal was a production stderr line at boot. + describe('acting organization (#16659)', () => { + const taskObject = { name: 'task', label: 'Task', fields: {} }; + + function timeTriggered(config: Record, overrides: Record = {}) { + return { + objects: [taskObject], + flows: [ + { + name: 'digest', + type: 'schedule', + status: 'active', + nodes: [{ id: 'start', type: 'start', config }], + ...overrides, + }, + ], + }; + } + + const orgFindings = (stack: Record) => + validateFlowTriggerReadiness(stack).filter((f) => f.rule === FLOW_SCHEDULE_ORGANIZATION_MISSING); + + it('fires on a `schedule` flow that declares none', () => { + const findings = orgFindings(timeTriggered({ schedule: { type: 'cron', expression: '0 1 * * *' } })); + expect(findings).toHaveLength(1); + expect(findings[0].path).toBe('flows[0].nodes[0].config.organization'); + expect(findings[0].message, 'the sentence is the trigger\'s own, so the two cannot drift').toContain( + 'declares no acting organization', + ); + expect(findings[0].message, 'the refusal names the flow — the ruling requires that').toContain('digest'); + }); + + it('fires on a `time_relative` sweep that declares none, and says WHICH kind', () => { + const findings = orgFindings( + timeTriggered({ timeRelative: { object: 'task', dateField: 'due_at', withinDays: 7 } }), + ); + expect(findings).toHaveLength(1); + expect( + findings[0].message, + 'a sweep must be named as one — the two kinds take the same refusal for different reasons', + ).toContain('time-relative flow'); + }); + + it('names the near-miss spelling the open `config` record accepted and ignored', () => { + const findings = orgFindings( + timeTriggered({ schedule: { type: 'cron', expression: '0 1 * * *' }, organizationId: 'org_x' }), + ); + expect(findings).toHaveLength(1); + expect( + findings[0].message, + 'an author who wrote `organizationId` is told about THEIR spelling, not about "nothing"', + ).toContain('organizationId'); + }); + + it('judges a present-but-unusable value exactly as the trigger does', () => { + // ⛔ Not a separate opinion: both read `resolveScheduleOrganization`, so a + // flow admitted by one and refused by the other is the silent hole again. + for (const bad of ['', 123, { id: 'org_x' }, null]) { + expect( + orgFindings(timeTriggered({ schedule: '0 1 * * *', organization: bad })), + `organization: ${JSON.stringify(bad)} must be judged undeclared`, + ).toHaveLength(1); + } + }); + + it('is silent once the flow declares one', () => { + expect( + orgFindings(timeTriggered({ schedule: { type: 'cron', expression: '0 1 * * *' }, organization: 'org_a' })), + ).toEqual([]); + }); + + it('never fires on a record_change, api, or manual flow', () => { + // Those bindings carry no organization BY CONSTRUCTION — they are fired by + // a caller who already holds one, and lifting a declared one onto them + // would let a flow overrule the tenant of the write that triggered it. A + // rule that asked them for the key would be asking for a defect. + expect(orgFindings({ objects: [candidateObject], flows: [recordFlow({ status: 'active' })] })).toEqual([]); + expect( + orgFindings({ + objects: [taskObject], + flows: [ + { + name: 'by_api', + type: 'api', + status: 'active', + nodes: [{ id: 'start', type: 'start', config: {} }], + }, + ], + }), + ).toEqual([]); + expect( + orgFindings({ + objects: [taskObject], + flows: [ + { + name: 'by_hand', + type: 'autolaunched', + status: 'active', + nodes: [{ id: 'start', type: 'start', config: {} }], + }, + ], + }), + ).toEqual([]); + }); + }); + it('handles map-keyed flows/objects and stacks with no flows', () => { expect(validateFlowTriggerReadiness({})).toEqual([]); const findings = validateFlowTriggerReadiness({ diff --git a/packages/lint/src/validate-flow-trigger-readiness.ts b/packages/lint/src/validate-flow-trigger-readiness.ts index 9a03303a74..7c34f359cd 100644 --- a/packages/lint/src/validate-flow-trigger-readiness.ts +++ b/packages/lint/src/validate-flow-trigger-readiness.ts @@ -51,6 +51,18 @@ // silence — every named runtime channel skips it because they all key off // the same resolution that already gave up. // +// 6. A `schedule` or `time_relative` flow that declares no acting +// organization (`config.organization`, #16659). Both triggers REFUSE to +// bind one — thrown, so the engine records the flow as not bound — and +// until this rule the refusal existed only at BOOT: `defineStack`, `os +// lint` and `verify_build` all passed a flow the trigger then refused, and +// an author's first signal was a production stderr line. That is exactly +// the authoring/runtime drift `engine.ts`'s trigger-kind resolver says +// must not exist, which is why the rule reads the SAME resolver and the +// SAME `resolveScheduleOrganization` helper the triggers refuse with. +// Severity is `warning` and the reason is the shipped corpus, not the +// strength of the verdict — see the id's own docblock. +// // The spec import is deliberate and is what makes rule 3 possible without a // second copy of the descriptor's shape living in this file. It stays inside the // package's stated dependency direction — lint → `@objectstack/spec`, never onto @@ -81,6 +93,15 @@ // authored-token → resolved-type map is a private chain of literal // `startsWith` / `typeof` tests with no registry lookup anywhere in it. No // package can teach the engine a new authored token. +// - `warning` — `flow-schedule-organization-missing` (#16659). On this +// paragraph's own criterion it belongs in the family above: the verdict is +// `resolveScheduleOrganization`'s and nothing installable changes it. It is +// held at `warning` by the CORPUS — an `error` gates `objectstack build`, +// and the repo's own shipped example apps carry time-triggered flows that +// cannot be repaired by authoring, because the only legal value is minted +// per install at runtime. Promoting it is a consequence of the open +// maintainer decision about package-shipped time-triggered flows, not a +// lint choice. // - `warning` — `flow-trigger-unknown-object`, both halves. An object name // this stack does not define may be defined by another installed package, // and this rule cannot see that package's objects. The hedge is real, so the @@ -103,7 +124,13 @@ // flows keep being served. What IS refused is the dead flow's own publish — and, // on the CLI surface, a package build whose stack contains one. -import { TimeRelativeTriggerSchema, resolveFlowTriggerKind } from '@objectstack/spec/automation'; +import { + TimeRelativeTriggerSchema, + resolveFlowTriggerKind, + SCHEDULE_ORGANIZATION_KEY, + resolveScheduleOrganization, + describeMissingScheduleOrganization, +} from '@objectstack/spec/automation'; import { recordsOf } from './object-graph.js'; export type FlowTriggerReadinessSeverity = 'error' | 'warning'; @@ -173,6 +200,42 @@ export const FLOW_TIME_RELATIVE_DESCRIPTOR_UNROUTABLE = 'flow-time-relative-desc * call sites that each skip it. */ export const FLOW_TRIGGER_UNROUTABLE = 'flow-trigger-unroutable'; +/** + * #16659 — a `schedule` or `time_relative` flow that declares no acting + * organization (`config.organization`). The trigger REFUSES to bind it, so the + * flow never fires; before this rule the author's first signal was a production + * stderr line at boot. + * + * It exists because `engine.ts`'s trigger-kind resolver states the invariant + * this rule keeps: the resolver is shared with `defineStack`'s + * trigger-capability refusal and this file, *"so the runtime cannot drift from + * what authoring accepted"*. A key required at bind and unknown to authoring is + * exactly that drift. + * + * ## Why `warning` and not `error`, when the never-fire family gates + * + * On the family's own criterion (#5762 — *is THIS STACK enough to know the flow + * is dead?*) this id belongs at `error`: the verdict is + * `resolveScheduleOrganization`'s, the same helper the two triggers refuse + * with, and no installed package changes it. + * + * What holds it at `warning` is the CORPUS, and it was measured rather than + * assumed. An `error` here is gating on the CLI surface too, so it refuses + * `objectstack build` — and the repo's own shipped example apps contain + * time-triggered flows that CANNOT be repaired by authoring: the only legal + * value is a `sys_organization.id`, minted per install at runtime, so a + * package-shipped flow has nothing to write there and ⛔ inventing a + * placeholder is worse than the omission (a value matching no row is silently + * authoritative). What a package-shipped time-triggered flow should do instead + * is an open maintainer decision, and promoting this id is that decision's + * consequence, not a lint choice: ⛔ do not raise it until the shipped corpus + * has an answer. + * + * The `warning` still discharges the invariant the rule exists for — the author + * learns at authoring time instead of at boot — and it is the same hedge + * `flow-trigger-unknown-object` carries, stated in the hint. + */ +export const FLOW_SCHEDULE_ORGANIZATION_MISSING = 'flow-schedule-organization-missing'; type AnyRec = Record; @@ -629,6 +692,54 @@ export function validateFlowTriggerReadiness(stack: AnyRec): FlowTriggerReadines }); } + // 1g. #16659 — a time-triggered flow that declares no acting organization. + // + // `ScheduleTrigger` and `TimeRelativeTrigger` refuse to bind one: the + // refusal is THROWN from `start()`, so the engine's catch records the + // flow as not bound, `getFlowRuntimeStates()` reports `bound: false` + // and `getTriggerBindingAudit()` lists it. That is a good runtime + // channel — and it is a BOOT-time one. Authoring said nothing at all: + // `defineStack`, `os lint` and `verify_build` all passed a flow the + // trigger then refused, which is precisely the drift `engine.ts`'s + // trigger-kind resolver says must not exist. + // + // ⛔ The judgement is NOT re-implemented here. `resolveFlowTriggerKind` + // answers WHICH flows owe the key (the engine's own precedence, the + // same resolver `isAutoTriggered` above already uses), + // `resolveScheduleOrganization` answers whether one was declared (the + // same helper both triggers refuse with, so a present-but-unusable + // value — `''`, a number — is judged identically here and there), and + // `describeMissingScheduleOrganization` writes the sentence, so this + // rule and the bind-time refusal cannot say different things about the + // same flow. + // + // `record_change` and `api` flows are outside it by construction: they + // are fired by a caller who already carries an organization, and the + // engine leaves `organization` undefined on their bindings. + const triggerKind = resolveFlowTriggerKind(flow); + if ( + start && + (triggerKind === 'schedule' || triggerKind === 'time_relative') && + resolveScheduleOrganization(flow) === undefined + ) { + findings.push({ + // `warning`, and the reason is the shipped corpus rather than the + // strength of the verdict — see FLOW_SCHEDULE_ORGANIZATION_MISSING's + // own docblock, which is where that decision is recorded. + severity: 'warning', + rule: FLOW_SCHEDULE_ORGANIZATION_MISSING, + where: `flow "${flowName}" › start node`, + path: `flows[${flowIndex}].nodes[${start.index}].config.${SCHEDULE_ORGANIZATION_KEY}`, + message: describeMissingScheduleOrganization(flowName, { kind: triggerKind, config }), + hint: + `Add config.${SCHEDULE_ORGANIZATION_KEY}: '' to the start node. The id is minted ` + + `by the running install, so a flow shipped INSIDE a package cannot carry one — register such a flow ` + + `at runtime with an organization that install actually holds, and ⛔ never write a placeholder id: a ` + + `value matching no row is silently authoritative to every report, export and cleanup that filters by ` + + `organization, which is strictly worse than the refusal.`, + }); + } + // 2. Auto-triggered flow whose status is 'draft' — authored or defaulted // (defineFlow parses at definition time, so the two are the same here). if (isAutoTriggered && (flow.status == null || flow.status === 'draft')) { diff --git a/packages/qa/dogfood/package.json b/packages/qa/dogfood/package.json index 74c77214cb..5e6059d64e 100644 --- a/packages/qa/dogfood/package.json +++ b/packages/qa/dogfood/package.json @@ -16,10 +16,10 @@ "@objectstack/example-crm": "workspace:*", "@objectstack/example-multi-package": "workspace:*", "@objectstack/example-showcase": "workspace:*", + "@objectstack/formula": "workspace:*", "@objectstack/mcp": "workspace:*", "@objectstack/metadata": "workspace:*", "@objectstack/metadata-core": "workspace:*", - "@objectstack/formula": "workspace:*", "@objectstack/objectql": "workspace:*", "@objectstack/platform-objects": "workspace:*", "@objectstack/plugin-approvals": "workspace:*", @@ -34,6 +34,7 @@ "@objectstack/service-storage": "workspace:*", "@objectstack/spec": "workspace:*", "@objectstack/trigger-record-change": "workspace:*", + "@objectstack/trigger-schedule": "workspace:*", "@objectstack/types": "workspace:*", "@objectstack/verify": "workspace:*" }, diff --git a/packages/qa/dogfood/test/fixtures/schedule-organization-fixture.ts b/packages/qa/dogfood/test/fixtures/schedule-organization-fixture.ts new file mode 100644 index 0000000000..078386662d --- /dev/null +++ b/packages/qa/dogfood/test/fixtures/schedule-organization-fixture.ts @@ -0,0 +1,178 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// Fixture for the #16659 acting-organization pins: two `schedule` flows that +// differ in EXACTLY ONE key — the `organization` declaration on the start node +// — so the pins' red/green is attributable to that key and to nothing else. +// +// Both flows are built at TEST time rather than declared in the stack config, +// because both of their load-bearing values are minted by the running stack: +// `sys_organization` ids and the recipient's `sys_user` id. A fixture that +// baked either one in would assert against a row that does not exist. + +/** + * Object the tick touches, so a run has a data write of its own to land. + * + * `due_date` is what the `time_relative` sweep selects on (#16659 F2). It is a + * `datetime` rather than a `date` deliberately: the window the trigger computes + * is a pair of ISO-8601 instants, and comparing them against a column the + * driver truncates to `YYYY-MM-DD` puts a per-driver truncation rule between + * the fixture and the property under test, which is WHICH ORGANIZATION's rows + * came back. + */ +const SweepTargetObject = { + name: 'sched_org_target', + label: 'Sweep Target', + fields: { + name: { type: 'text', label: 'Name', required: true }, + touched: { type: 'checkbox', label: 'Touched' }, + due_date: { type: 'datetime', label: 'Due' }, + }, +}; + +/** The stack both pins boot. Flows are registered after boot (see the header). */ +export const scheduleOrganizationStack = { + name: 'sched_org_fixture', + label: 'Schedule acting-organization fixture', + version: '1.0.0', + requires: ['automation', 'triggers', 'messaging'], + objects: [SweepTargetObject], +}; + +/** + * The organization-DECLARING flow: a `schedule` start node carrying a cadence + * and the `organization` key the ruling requires, then a `notify` node whose + * inbox rows are tenant-scoped. + * + * `runAs: 'system'` because a scheduled run has no trigger user (ADR-0049 / + * #1888) — the declaration every scheduled flow in this repo carries, and the + * one that makes the tenancy question live: an elevated write carries no + * session organization, so without the key below there is nothing to stamp and + * the #8844 guard refuses every tenant-scoped row beneath the run. + */ +export function declaringScheduleFlow(organizationId: string, recipientId: string): unknown { + return { + name: 'sched_org_declared', + label: 'Scheduled digest (organization declared)', + type: 'schedule', + status: 'active', + runAs: 'system', + nodes: [ + { + id: 'start', + type: 'start', + label: 'Every minute', + config: { + schedule: { type: 'cron', expression: '* * * * *' }, + organization: organizationId, + }, + }, + { + id: 'notify', + type: 'notify', + label: 'Digest', + config: { + topic: 'sched.digest', + recipients: [recipientId], + title: 'Nightly digest', + message: 'Your digest is ready.', + channels: ['inbox'], + }, + }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [ + { id: 'e1', source: 'start', target: 'notify' }, + { id: 'e2', source: 'notify', target: 'end' }, + ], + }; +} + +/** + * The organization-LESS twin: the flow above with the `organization` key + * deleted and its own name, derived from the same builder so the two can never + * drift into being two different flows that merely look alike. + */ +export function organizationLessScheduleFlow(recipientId: string): unknown { + const declared = declaringScheduleFlow('org_unused_placeholder', recipientId) as { + nodes: Array<{ id: string; config?: Record }>; + } & Record; + const nodes = declared.nodes.map((n) => { + if (n.id !== 'start') return n; + const config = { ...(n.config ?? {}) }; + delete config.organization; + return { ...n, config }; + }); + return { ...declared, name: 'sched_org_undeclared', nodes }; +} + +/** + * [#16659 F2] The `time_relative` twin: a sweep that declares its acting + * organization, selects `sched_org_target` rows whose `due_date` falls in the + * next week, and — once per matched record — notifies and writes. + * + * Both trailing nodes are load-bearing and they measure DIFFERENT halves: + * + * - `notify` produces one tenant-scoped inbox row per LAUNCHED run, so the + * count of those rows is the count of records the sweep SELECTED. That is + * the F2 property: an unscoped sweep selects the other organization's rows + * too and posts about them into the declared organization's inbox. + * - `update_record` is the data-plane half the branch previously left unpinned + * (the fixture flow was `start → notify → end`). The run is scoped to the + * declared organization, so a write aimed at another organization's row + * matches nothing — silently. Asserting WHICH rows got `touched` is what + * makes that narrowing observable instead of assumed. + */ +export function declaringTimeRelativeFlow(organizationId: string, recipientId: string): unknown { + return { + name: 'sched_org_sweep', + label: 'Time-relative sweep (organization declared)', + type: 'schedule', + status: 'active', + runAs: 'system', + nodes: [ + { + id: 'start', + type: 'start', + label: 'Daily sweep', + config: { + timeRelative: { + object: 'sched_org_target', + dateField: 'due_date', + withinDays: 7, + }, + organization: organizationId, + }, + }, + { + id: 'notify', + type: 'notify', + label: 'Due soon', + config: { + topic: 'sched.due', + recipients: [recipientId], + title: 'Due soon: {record.name}', + message: '{record.name} is due.', + channels: ['inbox'], + sourceObject: 'sched_org_target', + sourceId: '{record.id}', + }, + }, + { + id: 'touch', + type: 'update_record', + label: 'Mark touched', + config: { + objectName: 'sched_org_target', + filter: { id: '{record.id}' }, + fields: { touched: true }, + }, + }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [ + { id: 'e1', source: 'start', target: 'notify' }, + { id: 'e2', source: 'notify', target: 'touch' }, + { id: 'e3', source: 'touch', target: 'end' }, + ], + }; +} diff --git a/packages/qa/dogfood/test/schedule-acting-organization.dogfood.test.ts b/packages/qa/dogfood/test/schedule-acting-organization.dogfood.test.ts new file mode 100644 index 0000000000..5c059b728e --- /dev/null +++ b/packages/qa/dogfood/test/schedule-acting-organization.dogfood.test.ts @@ -0,0 +1,521 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// [#16659] A time-triggered flow declares its acting organization and the run +// executes as it — proven end to end through the real automation + messaging + +// ObjectQL stack, on BOTH drivers. +// +// @proof: schedule-acting-organization +// +// ## What was green and wrong +// +// `ScheduleTrigger` built its `AutomationContext` with no `tenantId`, because a +// job tick carries no identity. Two consumers read that key and both resolved +// NULL: `notify-node.ts` threads it onto the notification it emits (#11303), +// and `AutomationEngine.recordLog` copies it onto the `sys_automation_run` +// history row (#10101). On an install holding more than one `sys_organization` +// the #8844 guard then refused every tenant-scoped row beneath the run — the +// inbox rows and the history row — one layer BELOW anything that summarises the +// run, so the tick reported `unmeasured=0` and read healthy. +// +// ⚠️ Every assertion in this file passes vacuously if the run never happens at +// all, which is why each pin also asserts a POSITIVE fact about the run +// (the flow bound, the tick fired, the notification carries the declared id) and +// why the DIFFERENTIAL CONTROL below is in the same file: the same flow, on the +// same stack, through `POST /api/v1/automation/:name/trigger` under a session. +// That run reaches the identical `notify` node through the identical messaging +// chain, and its organization comes from the SESSION rather than from the +// declaration — so if the schedule pin ever goes green for a reason that has +// nothing to do with the fix, the control goes green the same way and the +// contrast that carries the proof is gone. +// +// ## The multi-organization condition +// +// Two `sys_organization` rows under the DEFAULT `single` posture — which is +// exactly the install the card measured, and exactly the state +// `system-write-organization.ts` calls `ambiguous-organization`: the posture is +// what the deployment asked for, the count is what the data is, and where they +// disagree the guard refuses rather than guessing. No enterprise organization +// plugin and no walled posture are needed to reach it, and using one would test +// a different topology than the report. + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { bootStack, type VerifyStack } from '@objectstack/verify'; +import { MessagingServicePlugin, INBOX_OBJECT, NOTIFICATION_EVENT_OBJECT } from '@objectstack/service-messaging'; +import { ScheduleTrigger, type JobServiceSurface, type TriggerLogger } from '@objectstack/trigger-schedule'; +import type { JobHandler, JobSchedule } from '@objectstack/spec/contracts'; +import { + scheduleOrganizationStack, + declaringScheduleFlow, + organizationLessScheduleFlow, +} from './fixtures/schedule-organization-fixture.js'; + +const RUN_HISTORY_OBJECT = 'sys_automation_run'; +const DECLARED_FLOW = 'sched_org_declared'; +const UNDECLARED_FLOW = 'sched_org_undeclared'; + +/** + * A job service the test fires by hand. The platform's own adapter owns cron + * timing; what these pins need is a DETERMINISTIC tick, and a real cron would + * make the suite wait on a wall clock to observe a property that has nothing to + * do with when the tick happens. + */ +function fakeJobService(): { + service: JobServiceSurface; + has(name: string): boolean; + names(): string[]; + fire(name: string, jobId?: string): Promise; +} { + const jobs = new Map(); + return { + service: { + async schedule(name: string, schedule: JobSchedule, handler: JobHandler) { + jobs.set(name, { schedule, handler }); + }, + async cancel(name: string) { + jobs.delete(name); + }, + }, + has: (name) => jobs.has(name), + names: () => [...jobs.keys()], + async fire(name, jobId = 'tick-1') { + const job = jobs.get(name); + if (!job) throw new Error(`no job registered under '${name}' — registered: ${[...jobs.keys()].join(', ') || '(none)'}`); + await job.handler({ jobId, data: {} } as never); + }, + }; +} + +/** Records every line the trigger logs, so the refusal pin can read it. */ +function recordingLogger(): { logger: TriggerLogger; errors: string[]; warns: string[] } { + const errors: string[] = []; + const warns: string[] = []; + return { + logger: { + info: () => {}, + debug: () => {}, + warn: (msg: string) => { warns.push(String(msg)); }, + error: (msg: string) => { errors.push(String(msg)); }, + }, + errors, + warns, + }; +} + +// eslint-disable-next-line @typescript-eslint/no-explicit-any +type Ql = any; +// eslint-disable-next-line @typescript-eslint/no-explicit-any +type Engine = any; + +const SYS = { context: { isSystem: true } }; + +/** + * Both drivers. The organization a run carries is resolved by the ObjectQL + * engine's system-insert path and stamped by the driver, and the two drivers + * reach that path differently — the SQL driver through + * `injectTenantOnInsert` + the partitioned unique index, the memory driver + * through its own tenant scope. A property about which organization a row lands + * with cannot be measured on one of them. + */ +for (const databaseDriver of ['sqlite-wasm', 'memory'] as const) { + describe(`dogfood [${databaseDriver}]: a scheduled run executes as its declared organization (#16659)`, () => { + let stack: VerifyStack; + let ql: Ql; + let automation: Engine; + let job: ReturnType; + let log: ReturnType; + let orgA: string; + let orgB: string; + let recipientId: string; + let memberToken: string; + + beforeAll(async () => { + stack = await bootStack(scheduleOrganizationStack as never, { + automation: true, + databaseDriver, + // `orgContext` binds the harness admin to a default organization, which + // is what lets the HTTP differential control carry an organization of + // its OWN (a caller bound to none fails to deliver for the same reason + // the schedule path used to, leaving the contrast certifying nothing). + // + // ⚠️ sqlite-wasm ONLY, and the asymmetry is measured rather than + // assumed: `driver-memory` declares NO row-level tenant isolation and + // REFUSES any call the engine hands a tenant scope + // (`MemoryMultiTenantUnsupportedError`, #16589 / #6915). An org-bound + // session makes the authorization resolver's own `sys_position` read + // tenant-scoped, so on that driver every HTTP request from such a + // session 503s before reaching any route. The HTTP control is therefore + // structurally unavailable there — see the driver-split control below, + // which pins that refusal so this exemption expires by itself the day + // the driver gains isolation. + orgContext: databaseDriver === 'sqlite-wasm', + // ⛔ Reliable delivery OFF, and not as a convenience: with the outbox + + // dispatcher on, `sys_inbox_message` is written by a background + // dispatcher on its own schedule, so an assertion made right after the + // tick reads an empty table whether or not the organization threaded. + // The property under test is WHICH ORGANIZATION the row carries, not + // when the dispatcher gets to it. + extraPlugins: [new MessagingServicePlugin({ reliableDelivery: false })], + }); + memberToken = await stack.signIn(); + ql = await stack.kernel.getServiceAsync('objectql'); + automation = stack.kernel.getService('automation'); + expect(automation?.registerFlow, 'automation engine must be wired').toBeTruthy(); + + // ── the multi-organization condition ────────────────────────────── + // TWO organizations, so "which organization owns this row" stops being + // derivable and the #8844 guard is live. One would make every pin below + // pass without the fix, because a single-organization install has a + // derivable answer and the guard supplies it. + // Two MORE organizations on top of whatever `orgContext` bootstrapped. + const a = await ql.insert('sys_organization', { name: 'Acme Employer' }, SYS); + const b = await ql.insert('sys_organization', { name: 'Beta Employer' }, SYS); + orgA = String(a.id); + orgB = String(b.id); + expect(orgA, 'organization A must have an id').toBeTruthy(); + expect(orgB, 'organization B must have an id').toBeTruthy(); + expect(orgA).not.toBe(orgB); + const orgs = await ql.find('sys_organization', { ...SYS }); + expect( + (orgs ?? []).length, + 'the guard only refuses when the install holds MORE THAN ONE organization — with one, every pin below passes unfixed', + ).toBeGreaterThanOrEqual(2); + + const admin = await ql.findOne('sys_user', { where: { email: 'admin@objectos.ai' }, ...SYS }); + recipientId = String(admin?.id ?? 'usr_system'); + + automation.registerFlow(DECLARED_FLOW, declaringScheduleFlow(orgA, recipientId)); + automation.registerFlow(UNDECLARED_FLOW, organizationLessScheduleFlow(recipientId)); + + job = fakeJobService(); + log = recordingLogger(); + automation.registerTrigger(new ScheduleTrigger(() => job.service, log.logger)); + await new Promise((r) => setTimeout(r, 0)); + }, 120_000); + + afterAll(async () => { + await stack?.stop(); + }); + + /** Rows of `object` this run wrote, read elevated so RLS never hides one. */ + async function rows(object: string, where: Record = {}): Promise>> { + return (await ql.find(object, { where, ...SYS })) ?? []; + } + + /** + * Wait for a row to appear, bounded. + * + * `recordTerminal` is a fire-and-forget write (`void this.store.recordTerminal(...)`), + * so the history row lands SHORTLY AFTER the tick's handler resolves. ⛔ This + * is a settle, never a retry that could paper over a refusal: a REFUSED + * insert never lands, so the bound expires and the assertion is red — which + * is exactly what it read on the unfixed tree. + */ + async function settleRows( + object: string, + where: Record = {}, + timeoutMs = 5_000, + ): Promise>> { + const deadline = Date.now() + timeoutMs; + let seen: Array> = []; + do { + seen = await rows(object, where); + if (seen.length > 0) return seen; + await new Promise((r) => setTimeout(r, 50)); + } while (Date.now() < deadline); + return seen; + } + + it('precondition: the declaring flow BOUND and the tick actually ran', async () => { + expect( + job.has(`flow-schedule:${DECLARED_FLOW}`), + `the declaring flow did not bind — registered jobs: ${job.names().join(', ') || '(none)'}`, + ).toBe(true); + await job.fire(`flow-schedule:${DECLARED_FLOW}`, 'tick-16659'); + const history = await settleRows(RUN_HISTORY_OBJECT, { flow_name: DECLARED_FLOW }); + expect( + history.length, + 'the tick produced no run at all — every pin below would then pass vacuously', + ).toBeGreaterThanOrEqual(1); + }); + + // ── CONSEQUENCE (1) — delivery ──────────────────────────────────────── + // + // PREDICTION, written before the run: on the unfixed tree the notification + // lands with `organization_id = NULL` and `sys_inbox_message` is EMPTY, + // because the inbox row is tenant-scoped and the guard refuses a + // system-context write that carries no organization on an install holding + // two. After the fix the notification carries `orgA` and the inbox row + // exists and carries `orgA`. + it('(1) the notification and its inbox row carry the DECLARED organization', async () => { + const notifications = await settleRows(NOTIFICATION_EVENT_OBJECT); + expect(notifications.length, 'the notify node emitted nothing').toBeGreaterThanOrEqual(1); + expect( + notifications.map((n) => n.organization_id ?? 'NULL'), + 'a scheduled run must stamp the organization it declared — NULL is the unfixed reading', + ).toContain(orgA); + + const inbox = await settleRows(INBOX_OBJECT); + expect( + inbox.length, + 'sys_inbox_message is EMPTY — the tenant-scoped write below the notification was refused, which is the defect', + ).toBeGreaterThanOrEqual(1); + expect(inbox.map((r) => r.organization_id ?? 'NULL')).toContain(orgA); + + // ⭐ Identity, not just presence: the declared organization is the one + // that landed, and the OTHER organization on this install never appears. + // A fix that stamped "some organization" would satisfy a presence check. + expect( + [...notifications, ...inbox].map((r) => r.organization_id).filter((v) => v === orgB), + 'a row landed in the organization the flow did NOT declare — cross-organization writes are exactly what the ruling forbids', + ).toHaveLength(0); + }); + + // ── CONSEQUENCE (2) — run history ───────────────────────────────────── + // + // Its OWN pin, deliberately not folded into (1): the history row is written + // by a different producer (`AutomationEngine.recordLog`) through a + // different consumer of the same key, and the card measured its refusal + // separately ("Insert on 'sys_automation_run' was REFUSED"). + // + // PREDICTION: unfixed, no `sys_automation_run` row exists for this flow at + // all. Fixed, exactly the scheduled run's row exists and carries `orgA`. + it('(2) the sys_automation_run history row persists, carrying the declared organization', async () => { + const history = await settleRows(RUN_HISTORY_OBJECT, { flow_name: DECLARED_FLOW }); + expect( + history.length, + "run history never persisted — the tick's sys_automation_run insert was refused", + ).toBeGreaterThanOrEqual(1); + expect( + history.map((r) => r.organization_id ?? 'NULL'), + 'the history row must carry the run\'s acting organization', + ).toContain(orgA); + expect( + history.map((r) => r.trigger_type), + 'the persisted row must still name WHAT fired the run (#7533)', + ).toContain('schedule'); + }); + + // ── CONSEQUENCE (3) — the declaration error ─────────────────────────── + // + // PREDICTION: unfixed, the organization-less flow binds exactly like the + // declaring one and its tick runs, delivering nothing. Fixed, it does NOT + // bind, the refusal is logged at `error`, and it names the flow. + // + // ⛔ The assertion is deliberately NOT "it logged something". It is: no job + // exists for it, so there is no path by which an organization-less + // time-triggered run reaches the data layer at all. + it('(3) an organization-less scheduled flow is REFUSED at bind, naming the flow', () => { + expect( + job.has(`flow-schedule:${UNDECLARED_FLOW}`), + 'the organization-less flow BOUND — it will tick, run, and deliver nothing, which is the defect', + ).toBe(false); + + const refusal = log.errors.find((l) => l.includes(UNDECLARED_FLOW)); + expect(refusal, `no refusal named '${UNDECLARED_FLOW}'; errors seen: ${JSON.stringify(log.errors)}`).toBeTruthy(); + expect(refusal, 'the refusal must be attributable to a flow, not to "a flow"').toContain(UNDECLARED_FLOW); + expect(refusal, 'the refusal must name the key the author has to write').toContain('organization'); + expect(refusal, 'a refused binding must say it is NOT BOUND').toContain('NOT BOUND'); + + // ⛔ And it must not have silently defaulted: neither organization on + // this install may appear in the refusal as a chosen value. + expect(refusal).not.toContain(orgA); + expect(refusal).not.toContain(orgB); + }); + + it('(3, control) refusing the organization-less flow did not disarm the declaring one', () => { + expect( + job.has(`flow-schedule:${DECLARED_FLOW}`), + 'the refusal took the sibling flow down with it — the refusal is per flow, not per trigger', + ).toBe(true); + }); + + // ⭐ (3) has a second half, and skipping it is how the first round of this + // card shipped a refusal the machine could not see. `job.has(...) === false` + // proves the JOB SERVICE was never asked; it says nothing about what the + // ENGINE recorded. `FlowTrigger.start()` returns `void`, so a trigger that + // logs and returns is indistinguishable from one that armed: the engine + // sets `boundFlowTriggers` and logs "bound" one line after the trigger said + // NOT BOUND, and every structured surface this repo built for "declared but + // not armed" then reports the opposite of the stderr line — Studio's badge + // via `getFlowRuntimeStates()`, and the silent-miss audit the automation + // plugin warns from at `kernel:bootstrapped` and the CLI prints in its + // startup summary via `getTriggerBindingAudit()`. + // + // PREDICTION, before the run: with a logged-and-returned refusal this pin + // is RED on both assertions (`bound: true`, audit empty of this flow); with + // the refusal thrown it is green, and the declaring flow stays out of the + // audit as the paired control. + it('(3, structured) the refused flow reads as NOT BOUND on every machine-readable surface', () => { + const states = automation.getFlowRuntimeStates() as Array<{ name: string; bound: boolean }>; + const refused = states.find((s) => s.name === UNDECLARED_FLOW); + expect(refused, `the refused flow is missing from getFlowRuntimeStates(): ${JSON.stringify(states.map((s) => s.name))}`).toBeTruthy(); + expect( + refused!.bound, + "Studio's status badge says this flow is armed while the trigger refused it — the loud channel and the structured channel disagree, which is the silent miss this card closes", + ).toBe(false); + expect( + states.find((s) => s.name === DECLARED_FLOW)?.bound, + 'control: the declaring flow must still read as bound, or this pin would pass with everything broken', + ).toBe(true); + + const audit = automation.getTriggerBindingAudit() as Array<{ + flowName: string; + triggerType: string; + reason: string; + }>; + const entry = audit.find((a) => a.flowName === UNDECLARED_FLOW); + expect( + entry, + `the silent-miss audit omits the refused flow, so the kernel:bootstrapped warning and the CLI startup summary both report every triggered flow as wired; audit: ${JSON.stringify(audit)}`, + ).toBeTruthy(); + expect(entry!.triggerType).toBe('schedule'); + expect( + entry!.reason, + 'the audit must say the binding FAILED (the trigger is registered), not that no trigger exists', + ).toContain('binding failed'); + expect( + audit.map((a) => a.flowName), + 'control: a flow that bound must not be listed as a silent miss', + ).not.toContain(DECLARED_FLOW); + }); + + // ── THE DIFFERENTIAL CONTROLS ───────────────────────────────────────── + // + // The pins above all assert that a row landed. Every one of them would also + // pass if delivery were simply broken in a way that happened to look like + // the fix working — so two controls run the SAME flow, the SAME nodes and + // the SAME messaging chain with the organization coming from somewhere + // OTHER than the start-node declaration. + + /** + * Control A — driver-portable, and the sharper of the two. + * + * The same flow, executed with an organization supplied by the CALLER's + * context (`tenantId`) instead of by the declaration: the record-change + * shape the card reports as unaffected ("the triggering session's + * organization is threaded, and delivery works on both drivers"). + * + * ⭐ It carries `orgB`, deliberately — the organization the flow does NOT + * declare. So it proves two things at once: the notify chain and the inbox + * write are live on this driver (the pins above are not vacuous), and the + * `orgA` those pins observed is attributable to the DECLARATION rather than + * to "whichever organization this install happens to have". + */ + it('control A: the same flow with a context-supplied organization delivers under THAT organization', async () => { + const before = new Set((await rows(INBOX_OBJECT)).map((r) => String(r.id))); + const result = await automation.execute(DECLARED_FLOW, { + event: 'api', + tenantId: orgB, + params: {}, + }); + expect(result?.success, `the control run failed: ${result?.error ?? '(no error)'}`).toBe(true); + + const deadline = Date.now() + 5_000; + let fresh: Array> = []; + do { + fresh = (await rows(INBOX_OBJECT)).filter((r) => !before.has(String(r.id))); + if (fresh.length > 0) break; + await new Promise((r) => setTimeout(r, 50)); + } while (Date.now() < deadline); + + expect( + fresh.length, + 'a context-supplied organization delivered nothing — delivery is broken for a reason unrelated to this card, and the pins above certify nothing', + ).toBeGreaterThanOrEqual(1); + expect( + fresh.map((r) => r.organization_id ?? 'NULL'), + 'the control row must carry the organization the CALLER supplied, not the one the flow declares', + ).toContain(orgB); + expect( + fresh.map((r) => r.organization_id), + "the caller's organization was overruled by the flow's declaration — a scheduled declaration must not reach a run it did not launch", + ).not.toContain(orgA); + }); + + /** + * Control B — the card's own control: the same flow through + * `POST /api/v1/automation/:name/trigger` under a session. + * + * Driver-split, because the drivers genuinely differ here and the split is + * pinned rather than papered over: + * + * - **sqlite-wasm** — the session is bound to the harness's default + * organization, so the run delivers under THAT organization: a third + * distinct id, and one more witness that `orgA` came from the + * declaration. + * - **memory** — the control is UNAVAILABLE, and this limb says so + * plainly rather than asserting something that cannot fail. + * + * All of control B's discriminating power comes from the session being + * bound to an organization OF ITS OWN: the run then delivers under that + * third id, and `orgA`'s absence is the witness. On `driver-memory` no + * session can be org-bound — the driver declares no row-level tenant + * isolation and refuses any tenant-scoped call + * (`MEMORY_MULTI_TENANT_UNSUPPORTED`, #16589 / #6915), so the + * authorization resolver's own `sys_position` read is refused and the + * door answers 503 before any route runs. This suite therefore boots + * memory with `orgContext: false`, which leaves the HTTP caller carrying + * no organization at all — the very state the unfixed schedule path was + * in. A run triggered that way discriminates nothing. + * + * ⛔ The first shape of this limb asserted `status < 300` under a message + * claiming it pinned a 503 refusal: opposite polarity, no delivery check, + * so it certified nothing in either direction. What is pinned instead is + * the REASON the control is unavailable, measured at the seam that makes + * it so — a tenant-scoped read on this driver produces NO ANSWER. The + * day the driver gains isolation that goes red and whoever fixes it + * enables the real control here. + */ + it('control B: the same flow via POST /automation/:name/trigger under a session', async () => { + if (databaseDriver === 'memory') { + // Not the control — the control cannot run here. This pins the reason, + // so the exemption expires by itself. + let thrown: unknown = null; + let answered: unknown = null; + try { + answered = await ql.find(INBOX_OBJECT, { where: {}, context: { userId: recipientId, tenantId: orgA } }); + } catch (err) { + thrown = err; + } + + expect( + thrown, + `driver-memory ANSWERED a tenant-scoped read (${JSON.stringify(answered)}) — it has gained row-level isolation, so an org-bound session is now possible here: boot this driver with orgContext and enable the real control B (#16589 / #6915)`, + ).toBeTruthy(); + // ⛔ Taken as a nullable value, not with `.toHaveLength`: a refused call + // must produce NO row count at all, and `.not.toHaveLength` passes over + // a null target for the wrong reason. + expect(Array.isArray(answered) ? answered.length : null).toBeNull(); + expect( + String((thrown as { code?: string }).code ?? (thrown as Error).message), + 'the refusal must be the driver\'s own tenancy refusal, not some unrelated failure that happens to throw', + ).toContain('MULTI_TENANT_UNSUPPORTED'); + return; + } + + const before = new Set((await rows(INBOX_OBJECT)).map((r) => String(r.id))); + const res = await stack.apiAs(memberToken, 'POST', `/automation/${DECLARED_FLOW}/trigger`, {}); + expect( + res.status, + `the session-triggered run did not start (${res.status}) — the control cannot certify the pins above`, + ).toBeLessThan(300); + + const deadline = Date.now() + 5_000; + let fresh: Array> = []; + do { + fresh = (await rows(INBOX_OBJECT)).filter((r) => !before.has(String(r.id))); + if (fresh.length > 0) break; + await new Promise((r) => setTimeout(r, 50)); + } while (Date.now() < deadline); + + expect( + fresh.length, + 'the session-triggered run delivered nothing — delivery is broken for a reason unrelated to this card', + ).toBeGreaterThanOrEqual(1); + expect( + fresh.map((r) => r.organization_id), + "the session-triggered row must carry the SESSION's organization, not the schedule declaration's", + ).not.toContain(orgA); + }); + }); +} diff --git a/packages/qa/dogfood/test/schedule-sweep-organization-scope.dogfood.test.ts b/packages/qa/dogfood/test/schedule-sweep-organization-scope.dogfood.test.ts new file mode 100644 index 0000000000..61ceb9f6ec --- /dev/null +++ b/packages/qa/dogfood/test/schedule-sweep-organization-scope.dogfood.test.ts @@ -0,0 +1,309 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// [#16659 F2] A `time_relative` sweep SELECTS inside its declared organization +// — proven on the real ObjectQL + driver stack, with matching rows in TWO +// organizations. +// +// @proof: schedule-acting-organization +// +// ## What this file measures that its sibling does not +// +// `schedule-acting-organization.dogfood.test.ts` proves the RUN carries the +// declared organization. That left the other half unmeasured, and it was wrong: +// the sweep's own query carried `context: { isSystem: true }` and nothing else, +// so a sweep declared for org A still MATCHED rows in org B and launched one run +// per match — each stamped A. Downstream that is worse than the defect the card +// opened on, not better: +// +// - the run is scoped to A, so its `update_record` on a B row matches nothing +// and reports success; +// - `notify` posts into A's inbox about B's record — a cross-tenant disclosure +// that was previously refused outright, because an org-less run could write +// nowhere; +// - the history row is stamped SUBJECT-first, so it lands under B while the +// inbox rows sit under A. +// +// ⚠️ THE DISCRIMINATING NUMBER IS THE COUNT OF LAUNCHED RUNS. A pin asserting +// only "A's row was touched" passes on the defect too — the defect touched it, +// alongside launching two runs about B's rows. So the fixture puts ONE matching +// row in A and TWO in B, and the pins read 1 vs 3. +// +// ## The multi-organization condition +// +// Two `sys_organization` rows under the DEFAULT `single` posture — the same +// install the card was measured on, and the state +// `system-write-organization.ts` calls `ambiguous-organization`. + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { bootStack, type VerifyStack } from '@objectstack/verify'; +import { MessagingServicePlugin, INBOX_OBJECT, NOTIFICATION_EVENT_OBJECT } from '@objectstack/service-messaging'; +import { TimeRelativeTrigger, type JobServiceSurface, type TriggerLogger } from '@objectstack/trigger-schedule'; +import type { JobHandler, JobSchedule } from '@objectstack/spec/contracts'; +import { + scheduleOrganizationStack, + declaringTimeRelativeFlow, +} from './fixtures/schedule-organization-fixture.js'; + +const TARGET_OBJECT = 'sched_org_target'; +const SWEEP_FLOW = 'sched_org_sweep'; +const SWEEP_JOB = `flow-time-relative:${SWEEP_FLOW}`; + +/** A job service the test fires by hand — the sweep's cadence is not the subject. */ +function fakeJobService(): { + service: JobServiceSurface; + has(name: string): boolean; + names(): string[]; + fire(name: string, jobId?: string): Promise; +} { + const jobs = new Map(); + return { + service: { + async schedule(name: string, schedule: JobSchedule, handler: JobHandler) { + jobs.set(name, { schedule, handler }); + }, + async cancel(name: string) { + jobs.delete(name); + }, + }, + has: (name) => jobs.has(name), + names: () => [...jobs.keys()], + async fire(name, jobId = 'tick-1') { + const job = jobs.get(name); + if (!job) throw new Error(`no job registered under '${name}' — registered: ${[...jobs.keys()].join(', ') || '(none)'}`); + await job.handler({ jobId, data: {} } as never); + }, + }; +} + +function recordingLogger(): { logger: TriggerLogger; errors: string[]; warns: string[] } { + const errors: string[] = []; + const warns: string[] = []; + return { + logger: { + info: () => {}, + debug: () => {}, + warn: (msg: string) => { warns.push(String(msg)); }, + error: (msg: string) => { errors.push(String(msg)); }, + }, + errors, + warns, + }; +} + +// eslint-disable-next-line @typescript-eslint/no-explicit-any +type Ql = any; +// eslint-disable-next-line @typescript-eslint/no-explicit-any +type Engine = any; + +const SYS = { context: { isSystem: true } }; + +/** + * Both drivers, and they answer DIFFERENT questions here — the split is the + * point, not an exemption: + * + * - **sqlite-wasm** enforces tenant isolation, so it can be asked the real + * differential question: with rows in A and B, which come back? + * - **memory** implements none and REFUSES any call handed a tenant scope + * (`MEMORY_MULTI_TENANT_UNSUPPORTED`, #16589). The question it answers is the + * one the card is really about: when a sweep required to stay inside one + * organization cannot be served, does it SAY SO or go quiet? + */ +for (const databaseDriver of ['sqlite-wasm', 'memory'] as const) { + describe(`dogfood [${databaseDriver}]: a time-relative sweep selects inside its declared organization (#16659)`, () => { + let stack: VerifyStack; + let ql: Ql; + let automation: Engine; + let job: ReturnType; + let log: ReturnType; + let orgA: string; + let orgB: string; + let rowA: string; + let rowsB: string[]; + + beforeAll(async () => { + stack = await bootStack(scheduleOrganizationStack as never, { + automation: true, + databaseDriver, + orgContext: databaseDriver === 'sqlite-wasm', + // ⛔ Reliable delivery OFF for the same reason the sibling suite turns + // it off: with the outbox + dispatcher on, `sys_inbox_message` is + // written by a background dispatcher on its own schedule, so a count + // taken right after the tick reads empty whatever the sweep selected. + extraPlugins: [new MessagingServicePlugin({ reliableDelivery: false })], + }); + await stack.signIn(); + ql = await stack.kernel.getServiceAsync('objectql'); + automation = stack.kernel.getService('automation'); + expect(automation?.registerFlow, 'automation engine must be wired').toBeTruthy(); + + const a = await ql.insert('sys_organization', { name: 'Acme Employer' }, SYS); + const b = await ql.insert('sys_organization', { name: 'Beta Employer' }, SYS); + orgA = String(a.id); + orgB = String(b.id); + expect(orgA).not.toBe(orgB); + + const admin = await ql.findOne('sys_user', { where: { email: 'admin@objectos.ai' }, ...SYS }); + const recipientId = String(admin?.id ?? 'usr_system'); + + // ── the differential fixture ────────────────────────────────────── + // One matching row in A, TWO in B. Every row is inside the window, so + // the ONLY thing that can keep B's out is the tenant scope. + const due = new Date(Date.now() + 2 * 86_400_000).toISOString(); + const inA = await ql.insert(TARGET_OBJECT, { name: 'A-1', due_date: due, organization_id: orgA }, SYS); + const inB1 = await ql.insert(TARGET_OBJECT, { name: 'B-1', due_date: due, organization_id: orgB }, SYS); + const inB2 = await ql.insert(TARGET_OBJECT, { name: 'B-2', due_date: due, organization_id: orgB }, SYS); + rowA = String(inA.id); + rowsB = [String(inB1.id), String(inB2.id)]; + + const seeded = (await ql.find(TARGET_OBJECT, { ...SYS })) ?? []; + expect( + seeded.length, + 'precondition: three rows must exist, or "B was not selected" proves nothing', + ).toBe(3); + expect( + seeded.map((r: Record) => String(r.organization_id ?? 'NULL')).sort(), + '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', + ).toEqual([orgA, orgB, orgB].sort()); + + automation.registerFlow(SWEEP_FLOW, declaringTimeRelativeFlow(orgA, recipientId)); + + job = fakeJobService(); + log = recordingLogger(); + automation.registerTrigger(new TimeRelativeTrigger(() => job.service, () => ql, log.logger)); + await new Promise((r) => setTimeout(r, 0)); + }, 120_000); + + afterAll(async () => { + await stack?.stop(); + }); + + async function rows(object: string, where: Record = {}): Promise>> { + return (await ql.find(object, { where, ...SYS })) ?? []; + } + + it('precondition: the sweep BOUND', () => { + expect( + job.has(SWEEP_JOB), + `the sweep did not bind — registered jobs: ${job.names().join(', ') || '(none)'}`, + ).toBe(true); + }); + + if (databaseDriver === 'memory') { + /** + * The store cannot honour the scope, so the sweep must be LOUD. + * + * PREDICTION, written before the run: `driver-memory` refuses the scoped + * `find` (#16589), the sweep's own error isolation catches it, and the + * failure is logged at `error` naming the flow. ⛔ What must NOT happen is + * the sweep quietly answering with every organization's rows — that is the + * silent non-isolation the driver's refusal exists to remove, and this + * card's whole subject is a tick that looks healthy while being wrong. + */ + it('a store with no tenant isolation REFUSES the sweep, loudly, and launches nothing', async () => { + await job.fire(SWEEP_JOB, 'tick-16659-f2'); + + const failure = log.errors.find((l) => l.includes('sweep failed')); + expect( + failure, + `the sweep did not report a failure; errors: ${JSON.stringify(log.errors)} · warns: ${JSON.stringify(log.warns)}`, + ).toBeTruthy(); + expect(failure, 'the failure must be attributable to a flow').toContain(SWEEP_FLOW); + expect( + failure, + "and carry the driver's own refusal, not some unrelated error that happens to throw", + ).toContain('NO row-level tenant isolation'); + + const touched = (await rows(TARGET_OBJECT)).filter((r) => r.touched === true || r.touched === 1); + expect( + touched.map((r) => String(r.id)), + 'a refused sweep must launch no runs at all — a partially-served sweep is the cross-organization task the ruling forbids', + ).toEqual([]); + }); + + it('control: the refusal is about the SCOPE, not about the object or the window', async () => { + // The same query without a tenant scope is served. So "nothing came + // back" above is attributable to the scope the sweep asked for, and not + // to a fixture whose rows never matched. + const unscoped = (await ql.find(TARGET_OBJECT, { where: {}, context: { isSystem: true } })) ?? []; + expect( + unscoped.length, + 'the unscoped read must still see all three rows, or the fixture — not the scope — is what this suite measured', + ).toBe(3); + }); + return; + } + + /** + * PREDICTION, written before the run: on the unfixed tree the sweep selects + * all THREE rows (the query carried no scope), launches three runs, and + * three inbox rows land under org A — two of them naming org B's records. + * With the fix it selects one, launches one, and one inbox row lands. + */ + it('DIFFERENTIAL: exactly ONE run is launched — the declared organization\'s row', async () => { + const before = new Set((await rows(INBOX_OBJECT)).map((r) => String(r.id))); + await job.fire(SWEEP_JOB, 'tick-16659-f2'); + + const deadline = Date.now() + 5_000; + let fresh: Array> = []; + do { + fresh = (await rows(INBOX_OBJECT)).filter((r) => !before.has(String(r.id))); + if (fresh.length > 0) break; + await new Promise((r) => setTimeout(r, 50)); + } while (Date.now() < deadline); + + expect( + fresh.length, + `the sweep launched ${fresh.length} run(s); 3 is the unfixed reading (org B's two rows swept in), 0 means the sweep delivered nothing and the pin below would be vacuous`, + ).toBe(1); + }); + + it('the run acts on the DECLARED organization\'s record, and on no other', async () => { + const all = await rows(TARGET_OBJECT); + const dump = JSON.stringify(all.map((r) => ({ id: r.id, name: r.name, touched: r.touched, org: r.organization_id }))); + const touched = all.filter((r) => Boolean(r.touched)).map((r) => String(r.id)); + expect( + touched, + `the sweep's \`update_record\` must land on org A's row; rows: ${dump}`, + ).toEqual([rowA]); + for (const id of rowsB) { + expect( + touched, + 'a row in an organization this flow never declared was acted on — the cross-organization scheduled task the ruling forbids', + ).not.toContain(id); + } + }); + + it('no notification describes a record from an organization the flow never declared', async () => { + // ⭐ The DISCLOSURE half, and the one the write-side pin above cannot + // reach. An unscoped sweep launches a run per matched row whatever the + // run is then scoped to: the `update_record` on org B's row matches + // nothing — silently — but the `notify` node has already emitted, so B's + // record is named in a notification stamped with A's organization. A + // fix that only narrowed the WRITES would leave this leak open. + // + // Read off `sys_notification` rather than `sys_inbox_message`: the + // `sourceObject`/`sourceId` click-through pair writes + // `source_object`/`source_id` THERE (io-node-config.zod.ts), and the + // inbox row carries the rendered `action_url` instead. + const notifications = await rows(NOTIFICATION_EVENT_OBJECT); + const dump = JSON.stringify(notifications.map((r) => ({ org: r.organization_id, src: r.source_id, title: r.title }))); + const sourceIds = notifications + .map((r) => String((r as Record).source_id ?? '')) + .filter((v) => v !== ''); + for (const id of rowsB) { + expect( + sourceIds, + `a notification under the declared organization names another organization's record — a cross-tenant disclosure; notifications: ${dump}`, + ).not.toContain(id); + } + expect( + sourceIds, + `control: the declared organization's own record IS named, so the assertion above is not passing on an empty set; notifications: ${dump}`, + ).toContain(rowA); + expect( + notifications.map((r) => String(r.organization_id ?? 'NULL')), + 'and every one of them is stamped with the declared organization', + ).toEqual([orgA]); + }); + }); +} diff --git a/packages/qa/dogfood/tsconfig.json b/packages/qa/dogfood/tsconfig.json index 55825a9035..a0da68ba8d 100644 --- a/packages/qa/dogfood/tsconfig.json +++ b/packages/qa/dogfood/tsconfig.json @@ -60,7 +60,8 @@ "@objectstack/organizations": ["../../plugins/organizations/src/index.ts"], "@objectstack/formula": ["../../formula/src/index.ts"], "@objectstack/plugin-approvals": ["../../plugins/plugin-approvals/src/index.ts"], - "@objectstack/trigger-record-change": ["../../triggers/trigger-record-change/src/index.ts"] + "@objectstack/trigger-record-change": ["../../triggers/trigger-record-change/src/index.ts"], + "@objectstack/trigger-schedule": ["../../triggers/trigger-schedule/src/index.ts"] } }, "include": ["test/**/*"], diff --git a/packages/qa/dogfood/vitest.config.ts b/packages/qa/dogfood/vitest.config.ts index bfbcf3f1ee..ef6b2f3aba 100644 --- a/packages/qa/dogfood/vitest.config.ts +++ b/packages/qa/dogfood/vitest.config.ts @@ -151,6 +151,22 @@ export default defineConfig({ find: /^@objectstack\/trigger-record-change$/, replacement: path.resolve(__dirname, '../../triggers/trigger-record-change/src/index.ts'), }, + // [#16659] `schedule-acting-organization.dogfood.test.ts` and + // `schedule-sweep-organization-scope.dogfood.test.ts` drive + // `ScheduleTrigger` / `TimeRelativeTrigger` themselves: the pins' + // whole subject is which + // organization the trigger puts on the run it launches, which + // organization its SWEEP QUERY is scoped to, and that a flow + // declaring none is refused at bind. A dist merely behind + // would run the pin green against the trigger's OLD context + // construction — the exact shape this card is about, since the + // defect was a run that reported itself healthy while carrying + // nothing. Aliased to source so the verdict is about THIS + // checkout. + { + find: /^@objectstack\/trigger-schedule$/, + replacement: path.resolve(__dirname, '../../triggers/trigger-schedule/src/index.ts'), + }, ], }, test: { diff --git a/packages/services/service-automation/src/engine.ts b/packages/services/service-automation/src/engine.ts index d67cfbd384..d0add9423b 100644 --- a/packages/services/service-automation/src/engine.ts +++ b/packages/services/service-automation/src/engine.ts @@ -24,7 +24,7 @@ import { FlowSchema, FLOW_STRUCTURAL_NODE_TYPES, validateControlFlow, collectFlo // shared with `defineStack`'s trigger-capability refusal and `@objectstack/lint`'s // `validate-flow-trigger-readiness`, so the runtime cannot drift from what // authoring accepted. See `resolveTriggerBinding`. -import { resolveFlowTriggerKind } from '@objectstack/spec/automation'; +import { resolveFlowTriggerKind, resolveScheduleOrganization } from '@objectstack/spec/automation'; import { predicateSlotRefusal, resolveFlowNodeExpressions, structuralConditionRefusal } from '@objectstack/spec/automation'; // [#15137] The `value`-role half of the ledger. Both halves of "is this envelope // well-formed?" are IMPORTED, never re-spelled here: the shape rule is @@ -427,6 +427,25 @@ export interface FlowTriggerBinding { readonly condition?: string | { dialect?: string; source?: string; ast?: unknown }; /** schedule: cron/interval descriptor (parsed but not yet acted on here). */ readonly schedule?: unknown; + /** + * [#16659] schedule / time_relative: the ACTING ORGANIZATION the flow + * declares on its start node (`config.organization`), resolved through + * `@objectstack/spec`'s {@link resolveScheduleOrganization} so authoring, + * this lift and the triggers cannot disagree about what counts as declared. + * + * Populated only for the two time-triggered kinds. `record_change` and + * `api` bindings leave it `undefined` BY CONSTRUCTION rather than by + * omission: both are fired by a caller who already carries an organization, + * and lifting a declared one onto them would let a flow overrule the tenant + * of the very write that triggered it. + * + * `undefined` here is a REFUSAL condition for the trigger that receives it, + * never a default to be filled in downstream: the time triggers THROW from + * `start()` on it, so {@link activateFlowTrigger}'s catch records the flow + * as unbound and {@link getTriggerBindingAudit} lists it — see the schedule + * trigger's `refuseMissingOrganization`. + */ + readonly organization?: string; /** The raw start-node `config`, for trigger-specific fields not modeled above. */ readonly config?: Record; } @@ -3046,6 +3065,13 @@ export class AutomationEngine implements IAutomationService { ? config.objectName : undefined, schedule: config.schedule, + // [#16659] Lifted beside `schedule`, for the same + // reason `schedule` is lifted: it is a BINDING fact the + // trigger acts on, not a config value it interprets. + // `config` still carries it verbatim below, so a + // trigger built against the older binding shape reads + // the same declaration from the same place. + organization: resolveScheduleOrganization(flow), condition, config, }, @@ -3055,7 +3081,16 @@ export class AutomationEngine implements IAutomationService { case 'schedule': return { triggerType: kind, - binding: { flowName, schedule: config.schedule, condition, config }, + // [#16659] `organization` rides beside `schedule`: the two + // together ARE a scheduled flow's binding — when it fires, + // and which organization it fires as. + binding: { + flowName, + schedule: config.schedule, + organization: resolveScheduleOrganization(flow), + condition, + config, + }, }; // Inbound HTTP (ADR-0041 Tier 1): an `api` flow waits for an external diff --git a/packages/services/service-automation/src/suspended-run-store.ts b/packages/services/service-automation/src/suspended-run-store.ts index 1796a17c1a..8175538d79 100644 --- a/packages/services/service-automation/src/suspended-run-store.ts +++ b/packages/services/service-automation/src/suspended-run-store.ts @@ -641,10 +641,17 @@ export class ObjectStoreSuspendedRunStore implements SuspendedRunStore { // `sys_api_key`'s divergent `active_organization_id` included), falling // back to the acting context's tenant (`record.organizationId`) when the // trigger carries no record or the object has no organization of its - // own. A plain scheduled sweep has neither and keeps NULL — fabricating - // an acting organization stays vetoed (Option C). Same inputs and same - // precedence as `serialize()` below, so a run's paused row and its - // terminal row agree by construction. + // own. Fabricating an acting organization stays vetoed (Option C). Same + // inputs and same precedence as `serialize()` below, so a run's paused + // row and its terminal row agree by construction. + // + // [#16659] This used to end "a plain scheduled sweep has neither and + // keeps NULL", and that stopped being true when a time-triggered flow + // began declaring the organization it runs as: such a sweep now arrives + // with `record.organizationId` set, so the second limb answers and the + // row is stamped. What has NOT changed is the precedence — the SUBJECT's + // organization still wins over the acting one where both exist, which is + // deliberate (a history row belongs with the record it is about). organization_id: this.recordOrgResolver.organizationOf(record.triggerObject ?? '', record.triggerRecord) ?? record.organizationId ?? diff --git a/packages/spec/api-surface/automation.json b/packages/spec/api-surface/automation.json index 099a823985..2af06dd3bb 100644 --- a/packages/spec/api-surface/automation.json +++ b/packages/spec/api-surface/automation.json @@ -199,8 +199,11 @@ "RetryPolicy (type)", "RetryPolicyParsed (type)", "RetryPolicySchema (const)", + "SCHEDULE_ORGANIZATION_KEY (const)", "SCHEMALESS_NODE_CONFIG_SCHEMAS (const)", "STRUCTURAL_CONDITION_SHAPE_REFUSAL (const)", + "ScheduleOrganization (type)", + "ScheduleOrganizationSchema (const)", "ScheduleState (type)", "ScheduleStateParsed (type)", "ScheduleStateSchema (const)", @@ -259,6 +262,7 @@ "defineActionDescriptor (function)", "defineFlow (function)", "defineWebhook (function)", + "describeMissingScheduleOrganization (function)", "exportConstructsToBpmn (function)", "findRegionEntry (function)", "flowForm (const)", @@ -273,6 +277,7 @@ "predicateSlotRefusal (function)", "resolveFlowNodeExpressions (function)", "resolveFlowTriggerKind (function)", + "resolveScheduleOrganization (function)", "structuralConditionRefusal (function)", "validateControlFlow (function)" ] diff --git a/packages/spec/declaration-map/automation.json b/packages/spec/declaration-map/automation.json index d943434410..7436be618c 100644 --- a/packages/spec/declaration-map/automation.json +++ b/packages/spec/declaration-map/automation.json @@ -104,6 +104,8 @@ "ParallelBranchSchema": "automation/ParallelBranch", "ParallelConfig": "automation/ParallelConfig", "ParallelConfigSchema": "automation/ParallelConfig", + "ScheduleOrganization": "automation/ScheduleOrganization", + "ScheduleOrganizationSchema": "automation/ScheduleOrganization", "ScheduleState": "automation/ScheduleState", "ScheduleStateSchema": "automation/ScheduleState", "ScreenConfig": "automation/ScreenConfig", diff --git a/packages/spec/export-origins/automation.json b/packages/spec/export-origins/automation.json index aadc165a7b..7d6a2788b8 100644 --- a/packages/spec/export-origins/automation.json +++ b/packages/spec/export-origins/automation.json @@ -194,8 +194,11 @@ "RetryPolicy": "src/shared/retry-policy.zod.ts#RetryPolicy (type)", "RetryPolicyParsed": "src/shared/retry-policy.zod.ts#RetryPolicyParsed (type)", "RetryPolicySchema": "src/shared/retry-policy.zod.ts#RetryPolicySchema (const)", + "SCHEDULE_ORGANIZATION_KEY": "src/automation/schedule-organization.zod.ts#SCHEDULE_ORGANIZATION_KEY (const)", "SCHEMALESS_NODE_CONFIG_SCHEMAS": "src/automation/schemaless-node-config.zod.ts#SCHEMALESS_NODE_CONFIG_SCHEMAS (const)", "STRUCTURAL_CONDITION_SHAPE_REFUSAL": "src/automation/flow-node-expression-paths.ts#STRUCTURAL_CONDITION_SHAPE_REFUSAL (const)", + "ScheduleOrganization": "src/automation/schedule-organization.zod.ts#ScheduleOrganization (type)", + "ScheduleOrganizationSchema": "src/automation/schedule-organization.zod.ts#ScheduleOrganizationSchema (const)", "ScheduleState": "src/automation/execution.zod.ts#ScheduleState (type)", "ScheduleStateParsed": "src/automation/execution.zod.ts#ScheduleStateParsed (type)", "ScheduleStateSchema": "src/automation/execution.zod.ts#ScheduleStateSchema (const)", @@ -253,6 +256,7 @@ "defineActionDescriptor": "src/automation/node-executor.zod.ts#defineActionDescriptor (function)", "defineFlow": "src/automation/flow.zod.ts#defineFlow (function)", "defineWebhook": "src/automation/webhook.zod.ts#defineWebhook (function)", + "describeMissingScheduleOrganization": "src/automation/schedule-organization.zod.ts#describeMissingScheduleOrganization (function)", "exportConstructsToBpmn": "src/automation/bpmn-mapping.ts#exportConstructsToBpmn (function)", "findRegionEntry": "src/automation/control-flow.zod.ts#findRegionEntry (function)", "flowForm": "src/automation/flow.form.ts#flowForm (const)", @@ -267,6 +271,7 @@ "predicateSlotRefusal": "src/automation/flow-node-expression-paths.ts#predicateSlotRefusal (function)", "resolveFlowNodeExpressions": "src/automation/flow-node-expression-paths.ts#resolveFlowNodeExpressions (function)", "resolveFlowTriggerKind": "src/automation/flow-trigger-kind.ts#resolveFlowTriggerKind (function)", + "resolveScheduleOrganization": "src/automation/schedule-organization.zod.ts#resolveScheduleOrganization (function)", "structuralConditionRefusal": "src/automation/flow-node-expression-paths.ts#structuralConditionRefusal (function)", "validateControlFlow": "src/automation/control-flow.zod.ts#validateControlFlow (function)" } diff --git a/packages/spec/json-schema.manifest/automation.json b/packages/spec/json-schema.manifest/automation.json index 7a28689cb9..0ebb133885 100644 --- a/packages/spec/json-schema.manifest/automation.json +++ b/packages/spec/json-schema.manifest/automation.json @@ -57,6 +57,7 @@ "automation/ParallelBranch", "automation/ParallelConfig", "automation/RetryPolicy", + "automation/ScheduleOrganization", "automation/ScheduleState", "automation/ScreenConfig", "automation/ScreenFieldConfig", diff --git a/packages/spec/llms.txt b/packages/spec/llms.txt index fc3bf3a893..45e78d70ae 100644 --- a/packages/spec/llms.txt +++ b/packages/spec/llms.txt @@ -77,7 +77,7 @@ const query = { --- -## 3. Schema Inventory by Domain (198 schemas) +## 3. Schema Inventory by Domain (199 schemas) Counted as `*.zod.ts` modules under `packages/spec/src//` — the sources that ship in this tarball (`files` includes `src/**/*.zod.ts`), so every number @@ -90,7 +90,7 @@ here is verifiable from the installed package. | data | 30 | Object, Field, Query, Filter, Driver (SQL/NoSQL/Memory/Mongo/Postgres), Cube | | api | 30 | Endpoint, REST Server, Discovery, OData, Batch, WebSocket, Response Envelope, Package Lifecycle | | ui | 18 | View, App, Action, Dashboard, Page, Chart, Component, Animation | -| automation | 13 | Flow, Approval, BPMN Interop, Control Flow, State Machine, Webhook | +| automation | 14 | Flow, Approval, BPMN Interop, Control Flow, State Machine, Webhook, Schedule Organization | | shared | 14 | Enums, HTTP, Identifiers, Mapping, Metadata Types, Connector Auth, Retry Policy, Value Domain, Epoch Instant (EpochMs) | | ai | 11 | Agent, Conversation, Knowledge Source/Document, Model Registry, MCP, Skill, Tool | | identity | 5 | Identity, Organization, Position, SCIM, Eval User | diff --git a/packages/spec/scripts/liveness/proof-registry.mts b/packages/spec/scripts/liveness/proof-registry.mts index 9275be6657..bb651c9686 100644 --- a/packages/spec/scripts/liveness/proof-registry.mts +++ b/packages/spec/scripts/liveness/proof-registry.mts @@ -324,6 +324,32 @@ export const HIGH_RISK_CLASSES: HighRiskClass[] = [ // ── Registered, honestly unbound ──────────────────────────────────────── + { + id: 'schedule-acting-organization', + label: 'Schedule / time-relative acting organization', + summary: + 'a time-triggered flow launches from a job tick, which carries no identity, so the run ' + + 'reached the #8844 tenancy guard with no organization to offer it. On an install holding ' + + 'more than one `sys_organization` every tenant-scoped row beneath the run was refused — the ' + + 'inbox rows a `notify` node emits and the `sys_automation_run` history row — one layer below ' + + 'anything that summarises a run, so the tick reported `unmeasured=0` and delivered nothing. ' + + 'The flow now DECLARES its acting organization on the start node and the run executes as it; ' + + 'a flow declaring none is refused at bind, naming the flow (#16659).', + proofId: 'schedule-acting-organization', + proofRef: + 'packages/qa/dogfood/test/schedule-acting-organization.dogfood.test.ts#schedule-acting-organization', + bound: false, + ledgerBindings: [], + blockedReason: + 'the property it guards — the start-node `config.organization` key — lives inside the flow ' + + "node `config` slot, which is an OPEN record by design (ADR-0018) and which the ledger " + + 'classifies as a CONTAINER rather than per key, so there is no `type.path` entry whose ' + + '`live` status this proof could gate. What it actually guards is a RUNTIME invariant: which ' + + 'organization a time-triggered run executes as, and that a flow declaring none is not armed. ' + + 'It runs unconditionally in the dogfood suite, on both drivers. ⛔ Not bound to `flow.runAs`, ' + + 'which it merely uses: binding a proof to a property it does not author is the false comfort ' + + 'the ledger exists to end.', + }, { id: 'flow-runas-userless', label: 'Flow runAs — the user-less run', diff --git a/packages/spec/src/automation/index.ts b/packages/spec/src/automation/index.ts index 92ebb2c1ad..5b57917337 100644 --- a/packages/spec/src/automation/index.ts +++ b/packages/spec/src/automation/index.ts @@ -41,6 +41,15 @@ export * from './approval.zod'; // (Prime Directive #12); the #4480 template cluster fell the same way. export * from './time-relative-trigger.zod'; export * from './flow-trigger-kind'; +// The acting-organization declaration a time-triggered flow carries, and the +// one refusal sentence the schedule trigger and the time-relative sweep both +// say it with (#16659). ⛔ `FlowSchema` does NOT emit that sentence: the key is +// enforced at BIND, not at parse, because the start node's `config` is an open +// record and a parse-time requirement would make every package-shipped +// scheduled flow unparseable. Named beside `flow-trigger-kind` because the two +// kinds that owe an organization are exactly the two that flow-trigger-kind +// resolves to a clock rather than to a session. +export * from './schedule-organization.zod'; // `sync.zod.ts` (L1 "Simple Sync": DataSyncConfig, its ConflictResolution enum // and the Sync factory) was removed here (#4738, ledger #4535 C13+C15). The L1 // layer was narrative-only — zero importers across objectstack / cloud / diff --git a/packages/spec/src/automation/schedule-organization.test.ts b/packages/spec/src/automation/schedule-organization.test.ts new file mode 100644 index 0000000000..7e0891b84a --- /dev/null +++ b/packages/spec/src/automation/schedule-organization.test.ts @@ -0,0 +1,169 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { describe, expect, it } from 'vitest'; +import { + SCHEDULE_ORGANIZATION_KEY, + ScheduleOrganizationSchema, + describeMissingScheduleOrganization, + resolveScheduleOrganization, +} from './schedule-organization.zod'; + +// [#16659] The declaration side of the acting-organization ruling. Two +// consumers read this module and they must not be able to disagree about what +// counts as DECLARED: the automation engine lifts the value onto the trigger +// binding (`resolveTriggerBinding`), and both time triggers refuse a binding +// that resolves to nothing. A value one layer calls usable and the other calls +// missing reopens the silent hole the card closed. + +function flow(config?: Record, extra: Record = {}) { + return { + name: 'nightly_sweep', + label: 'Nightly sweep', + type: 'schedule', + nodes: [ + { id: 'start', type: 'start', label: 'Start', ...(config ? { config } : {}) }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [{ id: 'e1', source: 'start', target: 'end' }], + ...extra, + }; +} + +describe('SCHEDULE_ORGANIZATION_KEY', () => { + it('is the bare `organization` spelling the refusal tells authors to write', () => { + // The constant, the refusal sentence and the docs all have to name ONE + // spelling; every other spelling in this area is a near-miss by definition. + expect(SCHEDULE_ORGANIZATION_KEY).toBe('organization'); + expect(describeMissingScheduleOrganization('f')).toContain('`organization`'); + }); +}); + +describe('ScheduleOrganizationSchema', () => { + it('accepts any non-empty string, including a non-`org_` id', () => { + // Deliberately not a pattern: organization ids are minted at runtime and a + // deployment that migrated ids from elsewhere must not be refused by a + // shape this layer invented. What is checked is that a value was DECLARED. + expect(ScheduleOrganizationSchema.safeParse('org_msokm9oaz0cal87q').success).toBe(true); + expect(ScheduleOrganizationSchema.safeParse('7f3c1e00-0000-4000-8000-000000000001').success).toBe(true); + }); + + it.each([ + ['an empty string', ''], + ['undefined', undefined], + ['null', null], + ['a number', 42], + ['an object', { id: 'org_x' }], + ])('refuses %s', (_label, value) => { + expect(ScheduleOrganizationSchema.safeParse(value).success).toBe(false); + }); +}); + +describe('resolveScheduleOrganization', () => { + it('reads the start node config', () => { + expect(resolveScheduleOrganization(flow({ organization: 'org_a' }))).toBe('org_a'); + }); + + it('answers undefined for a flow that declares none', () => { + expect(resolveScheduleOrganization(flow({ schedule: { type: 'cron', expression: '0 1 * * *' } }))).toBeUndefined(); + }); + + it('answers undefined for a present-but-unusable value', () => { + // ⭐ The engine lifts this onto the binding and the trigger refuses on + // `undefined`. Reporting an empty string as "declared" would arm a flow + // with nothing to stamp — the exact green-and-wrong shape of the card. + expect(resolveScheduleOrganization(flow({ organization: '' }))).toBeUndefined(); + expect(resolveScheduleOrganization(flow({ organization: 7 }))).toBeUndefined(); + expect(resolveScheduleOrganization(flow({ organization: { id: 'org_a' } }))).toBeUndefined(); + }); + + it('is structural: anything that is not a flow answers undefined rather than throwing', () => { + for (const input of [undefined, null, 42, 'flow', {}, { nodes: 'not-an-array' }, { nodes: [] }]) { + expect(() => resolveScheduleOrganization(input)).not.toThrow(); + expect(resolveScheduleOrganization(input)).toBeUndefined(); + } + }); + + it('ignores an `organization` that is not on the START node', () => { + const f = flow(undefined) as { nodes: Array> }; + f.nodes[1].config = { organization: 'org_on_the_end_node' }; + expect(resolveScheduleOrganization(f)).toBeUndefined(); + }); +}); + +// The near-miss scan is NOT published — it folded into the sentence below, so +// every case that used to call it directly now goes through the one exported +// door. The vocabulary is still pinned member by member; what is no longer +// pinned is a NAME a consumer could import, which is the point of the fold. +describe('the near-miss scan, through the sentence that owns it', () => { + it('takes the START NODE CONFIG — the record a trigger actually holds', () => { + // ⛔ Not a flow. The engine hands a trigger the start node's `config`, + // never the flow, so a flow-shaped `config` must find nothing. + expect(describeMissingScheduleOrganization('f', { config: { organizationId: 'org_a' } })).toContain( + '`organizationId`', + ); + expect(describeMissingScheduleOrganization('f', { config: flow({ organizationId: 'org_a' }) })).not.toContain( + '`organizationId`', + ); + }); + + it.each(['organizationId', 'organization_id', 'organizationID', 'orgId', 'org_id', 'org', 'tenantId', 'tenant_id', 'tenant'])( + 'recognises `%s`', + (key) => { + expect(describeMissingScheduleOrganization('f', { config: { [key]: 'org_a' } })).toContain(`\`${key}\``); + }, + ); + + it('ignores a near-miss key present but empty or null', () => { + // A key the author left blank is not evidence of the mistake the message + // describes ("you wrote X, which is not this key"). + for (const value of ['', null]) { + const msg = describeMissingScheduleOrganization('f', { config: { organizationId: value } }); + expect(msg).not.toContain('`organizationId`'); + // Non-vacuity: the sentence itself was produced, so the absence above is + // "no near-miss clause", not "no message". + expect(msg).toContain('`organization`'); + } + }); + + it('answers with no near-miss clause for anything that is not a record', () => { + for (const input of [undefined, null, 42, 'org_a', []]) { + expect(() => describeMissingScheduleOrganization('f', { config: input })).not.toThrow(); + expect(describeMissingScheduleOrganization('f', { config: input })).not.toContain('which is not this key'); + } + }); +}); + +describe('describeMissingScheduleOrganization', () => { + it('names the flow, the key, and the consequence an operator already saw', () => { + const msg = describeMissingScheduleOrganization('nightly_sweep'); + expect(msg).toContain("'nightly_sweep'"); + expect(msg).toContain('`organization`'); + expect(msg).toContain('sys_automation_run'); + expect(msg).toContain('reports itself healthy'); + }); + + it('names the near-miss spelling and never a value', () => { + const msg = describeMissingScheduleOrganization('nightly_sweep', { config: { organizationId: 'org_a' } }); + expect(msg).toContain('`organizationId`'); + expect(msg).toContain('open'); + // The KEY, never what was written under it — a diagnostic that echoes the + // value puts an id into every log line that carries the refusal. + expect(msg).not.toContain('org_a'); + }); + + it('says `time-relative` for the sweep and `scheduled` for the plain cadence', () => { + expect(describeMissingScheduleOrganization('f', { kind: 'time_relative' })).toContain('time-relative flow'); + expect(describeMissingScheduleOrganization('f', { kind: 'schedule' })).toContain('scheduled flow'); + expect(describeMissingScheduleOrganization('f')).toContain('scheduled flow'); + }); + + it('⛔ never offers a fallback: no organization is ever chosen for the author', () => { + // The ruling forbids a silent default and forbids the platform + // organization. The sentence must ASK for a value, not supply one. + const msg = describeMissingScheduleOrganization('nightly_sweep', { config: { orgId: 'org_a' } }); + expect(msg).toContain(''); + expect(msg).not.toMatch(/defaults? to/i); + expect(msg).not.toMatch(/platform organization/i); + expect(msg).toContain('no organization is ever chosen for it'); + }); +}); diff --git a/packages/spec/src/automation/schedule-organization.zod.ts b/packages/spec/src/automation/schedule-organization.zod.ts new file mode 100644 index 0000000000..be66393b39 --- /dev/null +++ b/packages/spec/src/automation/schedule-organization.zod.ts @@ -0,0 +1,247 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { z } from 'zod'; + +/** + * The ACTING ORGANIZATION of a time-triggered flow — the one start-node key + * that says which organization a scheduled run executes as. + * + * ## Why the key exists + * + * A record-change flow inherits its organization from the write that fired it: + * the triggering session's `tenantId` rides the {@link AutomationContext} into + * the run, so every tenant-scoped write below it — `sys_inbox_message`, + * `sys_notification_delivery`, `sys_automation_run` — resolves an organization + * the way a session write does. A TIME-triggered flow has no such session. The + * schedule trigger and the time-relative sweep launch their runs from a job + * tick, and a job tick carries no identity at all, so the run reached the + * tenancy guard (`system-write-organization.ts`) with nothing to offer it. On + * an install holding more than one `sys_organization` that guard refuses, + * correctly and by design — and the refusal landed on rows the run never + * reported: the notification wrote with `organization_id = NULL`, every + * tenant-scoped row beneath it was refused, and the tick still summarised + * itself as healthy. + * + * ## The ruling this key implements + * + * Maintainer, 2026-09-08, verbatim: + * + * > 多组织定时任务本来只能在组织内运行，应该带组织ID，不允许跨组织的定时任务。 + * + * A time-triggered flow is **organization-scoped by construction**: it names + * one organization and the run executes as that organization. There is + * deliberately no fan-out — a tenant that wants the same sweep in N + * organizations declares it N times — and there is deliberately no fallback: a + * flow that names none is a DECLARATION ERROR, not a run that quietly picks + * one. Guessing is the failure this key exists to prevent, and the platform + * organization is not a safe guess: a wrong `organization_id` is worse than a + * null, because a null is visibly missing while a wrong value is silently + * authoritative to every report, export and cleanup script that filters by + * organization. + * + * ## Where it lives, and why there + * + * On the flow's START node `config`, beside the cadence it scopes: + * + * ```ts + * config: { + * schedule: { type: 'cron', expression: '0 8 * * *' }, + * organization: 'org_msokm9oaz0cal87q', + * } + * ``` + * + * The start node is where every other trigger-binding fact already lives — + * `FlowSchema` refuses a top-level `schedule` in as many words ("a schedule + * flow declares its cron/interval as `config.schedule` on the START node, not + * at the flow top level"), and `resolveTriggerBinding` hands the whole start + * `config` to the trigger. Putting the organization at the flow top level would + * split one binding across two layers; putting it inside the `schedule` + * descriptor would make it invisible to the time-relative sweep, which carries + * its cadence in the same slot but binds through a different descriptor. One + * key, one layer, both time triggers. + * + * ## Why `ScheduleOrganization…` and not `FlowActingOrganization…` + * + * The key governs both trigger kinds, and `FlowTriggerKind` lists + * `time_relative` and `schedule` as two of its four members — so the name looks + * inaccurate for half its subjects. It is not, and the deciding reading is the + * AUTHORABLE surface rather than the derived kind: `FlowSchema.type` is + * `z.enum(['autolaunched', 'record_change', 'schedule', 'screen', 'api'])` and + * has no `time_relative` member at all. A time-relative sweep is authored as + * `type: 'schedule'` with a `timeRelative` descriptor on its start node — the + * docs say so in as many words ("a `schedule` flow whose `start` node declares + * a `timeRelative` descriptor"), and `TimeRelativeTriggerSchema`'s own opening + * line says the trigger "sweeps an object on a schedule". `FlowTriggerKind` + * splits the two because the ENGINE routes them to different triggers; its + * precedence note distinguishes a sweep from "a plain schedule flow", which is + * a split inside the schedule family, not out of it. + * + * ⇒ Every flow this key applies to declares `type: 'schedule'`. The name is + * accurate for both subjects, and a `minor` freezes it, so this is recorded + * rather than left to be re-litigated. + */ + +/** The start-node `config` key naming a time-triggered flow's acting organization. */ +export const SCHEDULE_ORGANIZATION_KEY = 'organization'; + +/** + * The value shape: an organization id — `sys_organization.id`, the same string + * a session write carries as `ExecutionContext.tenantId` and the same string + * the tenancy guard stamps onto `organization_id`. + * + * A bare non-empty string rather than a pattern: organization ids are minted at + * runtime (`org_…` today) and a deployment that has migrated ids from elsewhere + * must not be refused by a shape this layer invented. What is checked is that a + * value was DECLARED — which is the whole of what the ruling asks for. + */ +export const ScheduleOrganizationSchema = z + .string() + .min(1) + .describe( + 'Organization id (sys_organization.id) this scheduled/time-relative flow runs as. Required: a time-triggered run has no session to inherit a tenant from.', + ); + +/** + * The declared value's type — the alias the machine-readable surface needs + * beside the schema, and the name a reference page's import example carries. + * + * `z.input` per ADR-0122: the bare name is reserved for the AUTHOR state. No + * `ScheduleOrganizationParsed` sibling exists because the two states are the + * same type here — the schema neither transforms nor defaults, it only refuses + * what cannot be acted on. + */ +export type ScheduleOrganization = z.input; + +/** + * Spellings an author reaches for that are NOT this key, in the order a + * diagnostic should try them. The start node's `config` is an OPEN record by + * design (ADR-0018), so none of these is refused by any schema — a flow + * carrying `organizationId` parses, binds, and runs with no organization at + * all. Naming them in the refusal is the only place the mistake becomes + * visible, so this list is load-bearing rather than decorative. + * + * Module-local on purpose, and so is the scan that reads it: every caller that + * needs the vocabulary needs the SENTENCE, and + * {@link describeMissingScheduleOrganization} is the one that writes it. Both + * callers of the scan did `find` then `describe` back to back, so publishing + * the finder froze an orphan diagnostic on the surface — a `minor` freezes what + * it publishes, and removing an export later is breaking where adding one is + * not. + */ +const SCHEDULE_ORGANIZATION_NEAR_MISSES: readonly string[] = Object.freeze([ + 'organizationId', + 'organization_id', + 'organizationID', + 'orgId', + 'org_id', + 'org', + 'tenantId', + 'tenant_id', + 'tenant', +]); + +/** The start node's `config`, or `{}` for a flow shaped like anything else. */ +function startConfigOf(flow: unknown): Record { + if (!flow || typeof flow !== 'object') return {}; + const nodes = (flow as { nodes?: unknown }).nodes; + if (!Array.isArray(nodes)) return {}; + const start = nodes.find( + (n): n is { config?: unknown } => + !!n && typeof n === 'object' && (n as { type?: unknown }).type === 'start', + ); + return start?.config && typeof start.config === 'object' + ? (start.config as Record) + : {}; +} + +/** + * The acting organization a flow declares, or `undefined`. + * + * Structural, like {@link resolveFlowTriggerKind}: it reads a raw authored + * object, a `defineFlow` result and a parsed stack's flow alike, and answers + * `undefined` for anything else rather than throwing. A present-but-unusable + * value (empty string, a number, an object) answers `undefined` too — the + * caller's next step is the refusal either way, and reporting "declared" for a + * value nothing can act on is the silent-acceptance this key exists to end. + */ +export function resolveScheduleOrganization(flow: unknown): string | undefined { + const raw = startConfigOf(flow)[SCHEDULE_ORGANIZATION_KEY]; + const parsed = ScheduleOrganizationSchema.safeParse(raw); + return parsed.success ? parsed.data : undefined; +} + +/** + * The near-miss key an organization-less START NODE `config` actually wrote, if + * any — so the refusal can say "you wrote `organizationId`" instead of "you + * wrote nothing". + * + * ⛔ Takes the start node's `config` record, NOT a flow — hence the name. The + * caller that needs this is a TRIGGER, and a trigger never holds the flow: the + * engine parses the start node and hands it a binding whose `config` is that + * record. A flow-shaped input would answer `undefined` for the very shape the + * only caller has, which is the silent-acceptance this module exists to end. + * + * Anything that is not a record answers `undefined` rather than throwing, + * matching {@link resolveScheduleOrganization}'s structural posture. + * + * ⛔ NOT exported. It was, briefly, and had two consumers that each called it + * only to hand the answer straight back to + * {@link describeMissingScheduleOrganization} on the next line. A published + * name is answerable forever after a `minor`, so the one that ships is the one + * a caller actually wants: the sentence. + */ +function findScheduleOrganizationNearMissInConfig( + startConfig: unknown, +): string | undefined { + if (!startConfig || typeof startConfig !== 'object') return undefined; + const config = startConfig as Record; + return SCHEDULE_ORGANIZATION_NEAR_MISSES.find( + (k) => Object.prototype.hasOwnProperty.call(config, k) && config[k] != null && config[k] !== '', + ); +} + +/** + * The one refusal sentence, so the schedule trigger and the time-relative + * trigger say the same thing about the same defect. + * + * ⛔ `FlowSchema` does NOT emit it, and deliberately does not: the start node's + * `config` is an open record (ADR-0018) and every flow this repo's own packages + * ship would become unparseable if the key were required at parse time. + * Enforcement is at BIND — the two triggers below — which is where the + * consequence lives: there is no path by which an organization-less + * time-triggered run reaches the data layer once bind refuses. + * + * It names the flow (the ruling requires that), the key, where the key goes, + * and — when the author wrote a near-miss — which spelling of theirs was + * dropped. `options.config` is the START NODE's `config` record (what a trigger + * holds on its binding, and what the lint rule reads off the parsed start + * node); anything else, or nothing, simply yields no near-miss clause. It states the consequence rather than only the rule, because the + * consequence is the part an operator has already seen: this is the flow whose + * tick delivered nothing. + */ +export function describeMissingScheduleOrganization( + flowName: string, + options?: { readonly kind?: string; readonly config?: unknown }, +): string { + const kind = options?.kind === 'time_relative' ? 'time-relative' : 'scheduled'; + // The scan lives HERE rather than at the two call sites, which both ran it + // and passed the answer straight in. One published name, one place the + // near-miss vocabulary is consulted, and no way for a caller to describe a + // near-miss the scan would not have found. + const nearMiss = findScheduleOrganizationNearMissInConfig(options?.config); + return ( + `${kind} flow '${flowName}' declares no acting organization: its start node's \`config\` is ` + + `missing the \`${SCHEDULE_ORGANIZATION_KEY}\` key` + + (nearMiss + ? ` (it carries \`${nearMiss}\`, which is not this key — the start node's \`config\` is an open ` + + `record, so that spelling was accepted and then ignored)` + : '') + + `. A time-triggered run has no session to inherit a tenant from, so without this key the run ` + + `executes with no organization: on an install holding more than one \`sys_organization\` every ` + + `tenant-scoped write beneath it is refused — the inbox rows a \`notify\` node emits and the ` + + `\`sys_automation_run\` history row — while the tick still reports itself healthy. ` + + `Declare the organization the sweep runs in: \`config: { ${SCHEDULE_ORGANIZATION_KEY}: '' }\`. ` + + `A sweep wanted in several organizations is declared once per organization — ` + + `a single flow is never fanned out across them, and no organization is ever chosen for it.` + ); +} diff --git a/packages/spec/src/migrations/entries/semantic/18.schedule-flow-acting-organization-required.ts b/packages/spec/src/migrations/entries/semantic/18.schedule-flow-acting-organization-required.ts new file mode 100644 index 0000000000..5ae53cd68f --- /dev/null +++ b/packages/spec/src/migrations/entries/semantic/18.schedule-flow-acting-organization-required.ts @@ -0,0 +1,66 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import type { SemanticMigration } from '../../types.js'; + +export const entry: SemanticMigration = { + id: 'schedule-flow-acting-organization-required', + surface: + 'The START NODE `config.organization` key of every time-triggered flow — a `type: ' + + "'schedule'` flow carrying a `config.schedule` cadence, and the `timeRelative` sweep " + + 'that carries its cadence in the same slot (`FlowTriggerKind` `schedule` / ' + + '`time_relative`). Nothing is renamed, retired or re-typed: the start node\'s `config` ' + + 'is an OPEN record (ADR-0018), so the key is an ADDITION to a slot that already ' + + 'accepted it, and every flow that parses today parses byte-identically after the ' + + 'change. What narrows is the BIND-time accept set and the RUN-time data plane.', + replacement: + 'Declare the organization the flow runs as, on the start node beside the cadence: ' + + "`config: { schedule: { … }, organization: '' }`. There is " + + 'deliberately NO fan-out — a sweep wanted in N organizations is N flows, one per ' + + 'organization — and deliberately no fallback: nothing on this path ever chooses an ' + + 'organization, because a wrong `organization_id` is silently authoritative to every ' + + 'report, export and cleanup that filters by organization, while a refusal is visible ' + + 'at boot and names its flow. ⚠️ Three consequences of the split that the declaration ' + + 'itself does not carry, and each is deployment work: (1) rows whose tenant column is ' + + 'NULL stay visible to a scoped read (`org = :tenant OR org IS NULL`), so after the ' + + 'split each such row is matched ONCE PER FLOW — N runs and N notifications for one ' + + 'row, each acting as a different organization; (2) the dispatch-claim key embeds the ' + + 'flow name (`schedule::`, ' + + '`time-relative:::`), so renaming one flow into N abandons ' + + "the current window's claims and a window already delivered under the old name can " + + 'deliver once more under the new ones; (3) a run SUSPENDED before the upgrade ' + + 'rehydrates its context from `context_json`, which carries no `tenantId`, so it ' + + 'resumes org-less — drain or accept in-flight suspended runs rather than assuming the ' + + 'upgrade confines them retroactively.', + reason: + 'Maintainer ruling, 2026-09-08, verbatim, untranslated: ' + + '「多组织定时任务本来只能在组织内运行，应该带组织ID，不允许跨组织的定时任务。」 A time-triggered ' + + 'run is launched from a job tick and a job tick carries no identity, so the run reached ' + + 'the tenancy guard with nothing to offer it: the notification wrote ' + + '`organization_id = NULL`, every tenant-scoped row beneath it was refused, and the tick ' + + 'still summarised itself as healthy. ⛔ NOT losslessly convertible, and the reason is ' + + 'that the remedy is a value only the deployment holds: an organization id is minted per ' + + 'install at runtime, so there is no authored artifact and no stored representation a ' + + 'transform could rewrite — `objectstack migrate meta` cannot know which organization a ' + + 'given sweep belongs to, and inventing one is precisely what the ruling forbids. ' + + 'Registered under ADR-0087 D3 rather than left silent because the change DOES carry a ' + + 'prescription — "declare one flow per organization, no fan-out" is deployment work a ' + + 'human must do, which is what D3 says a structured TODO is for. The direct precedent is ' + + '`rest-requireauth-default-flip` (protocol 12): behaviour-only, no shape moved, a ' + + 'deployment judgement no transform can make, registered anyway.', + acceptanceCriteria: + 'Every `schedule` / `time_relative` flow in the stack declares a non-empty ' + + '`config.organization` on its start node. `os lint` reports ' + + '`flow-schedule-organization-missing` for none of them (severity `warning`, so it does ' + + 'NOT gate a build — an unfixed flow is silently unarmed, which is why the lint run is ' + + 'part of the criteria rather than the build), and boot logs no ' + + '`[schedule] NOT BOUND` / `[time-relative] NOT BOUND` line: ' + + '`getFlowRuntimeStates()` reports `bound: true` and `getTriggerBindingAudit()` lists ' + + 'no time-triggered flow. A deployment that ran ONE flow across all organizations has ' + + 'split it into one flow per organization and has re-checked the three consequences ' + + 'above — NULL-tenant rows, abandoned dispatch claims, suspended runs. ⚠️ ' + + '`@objectstack/driver-memory` has NO legal configuration for a time-triggered flow ' + + 'that touches per-organization data: it refuses any call handed a tenant scope ' + + '(`MEMORY_MULTI_TENANT_UNSUPPORTED`), so a declared flow is refused per call while an ' + + 'undeclared one is not armed at all. Multi-organization deployments use ' + + '`@objectstack/driver-sql`.', +}; diff --git a/packages/spec/src/migrations/registry.ts b/packages/spec/src/migrations/registry.ts index fe22322bf6..9cb707f8a9 100644 --- a/packages/spec/src/migrations/registry.ts +++ b/packages/spec/src/migrations/registry.ts @@ -9389,6 +9389,68 @@ const step18: MigrationStep = { + 'of the ten keys ever reached it. No code imports `CrudEndpointPattern(Schema)` from ' + '`@objectstack/spec/api` (TS2305 after upgrade).', }, + { + id: 'schedule-flow-acting-organization-required', + surface: + 'The START NODE `config.organization` key of every time-triggered flow — a `type: ' + + "'schedule'` flow carrying a `config.schedule` cadence, and the `timeRelative` sweep " + + 'that carries its cadence in the same slot (`FlowTriggerKind` `schedule` / ' + + '`time_relative`). Nothing is renamed, retired or re-typed: the start node\'s `config` ' + + 'is an OPEN record (ADR-0018), so the key is an ADDITION to a slot that already ' + + 'accepted it, and every flow that parses today parses byte-identically after the ' + + 'change. What narrows is the BIND-time accept set and the RUN-time data plane.', + replacement: + 'Declare the organization the flow runs as, on the start node beside the cadence: ' + + "`config: { schedule: { … }, organization: '' }`. There is " + + 'deliberately NO fan-out — a sweep wanted in N organizations is N flows, one per ' + + 'organization — and deliberately no fallback: nothing on this path ever chooses an ' + + 'organization, because a wrong `organization_id` is silently authoritative to every ' + + 'report, export and cleanup that filters by organization, while a refusal is visible ' + + 'at boot and names its flow. ⚠️ Three consequences of the split that the declaration ' + + 'itself does not carry, and each is deployment work: (1) rows whose tenant column is ' + + 'NULL stay visible to a scoped read (`org = :tenant OR org IS NULL`), so after the ' + + 'split each such row is matched ONCE PER FLOW — N runs and N notifications for one ' + + 'row, each acting as a different organization; (2) the dispatch-claim key embeds the ' + + 'flow name (`schedule::`, ' + + '`time-relative:::`), so renaming one flow into N abandons ' + + "the current window's claims and a window already delivered under the old name can " + + 'deliver once more under the new ones; (3) a run SUSPENDED before the upgrade ' + + 'rehydrates its context from `context_json`, which carries no `tenantId`, so it ' + + 'resumes org-less — drain or accept in-flight suspended runs rather than assuming the ' + + 'upgrade confines them retroactively.', + reason: + 'Maintainer ruling, 2026-09-08, verbatim, untranslated: ' + + '「多组织定时任务本来只能在组织内运行，应该带组织ID，不允许跨组织的定时任务。」 A time-triggered ' + + 'run is launched from a job tick and a job tick carries no identity, so the run reached ' + + 'the tenancy guard with nothing to offer it: the notification wrote ' + + '`organization_id = NULL`, every tenant-scoped row beneath it was refused, and the tick ' + + 'still summarised itself as healthy. ⛔ NOT losslessly convertible, and the reason is ' + + 'that the remedy is a value only the deployment holds: an organization id is minted per ' + + 'install at runtime, so there is no authored artifact and no stored representation a ' + + 'transform could rewrite — `objectstack migrate meta` cannot know which organization a ' + + 'given sweep belongs to, and inventing one is precisely what the ruling forbids. ' + + 'Registered under ADR-0087 D3 rather than left silent because the change DOES carry a ' + + 'prescription — "declare one flow per organization, no fan-out" is deployment work a ' + + 'human must do, which is what D3 says a structured TODO is for. The direct precedent is ' + + '`rest-requireauth-default-flip` (protocol 12): behaviour-only, no shape moved, a ' + + 'deployment judgement no transform can make, registered anyway.', + acceptanceCriteria: + 'Every `schedule` / `time_relative` flow in the stack declares a non-empty ' + + '`config.organization` on its start node. `os lint` reports ' + + '`flow-schedule-organization-missing` for none of them (severity `warning`, so it does ' + + 'NOT gate a build — an unfixed flow is silently unarmed, which is why the lint run is ' + + 'part of the criteria rather than the build), and boot logs no ' + + '`[schedule] NOT BOUND` / `[time-relative] NOT BOUND` line: ' + + '`getFlowRuntimeStates()` reports `bound: true` and `getTriggerBindingAudit()` lists ' + + 'no time-triggered flow. A deployment that ran ONE flow across all organizations has ' + + 'split it into one flow per organization and has re-checked the three consequences ' + + 'above — NULL-tenant rows, abandoned dispatch claims, suspended runs. ⚠️ ' + + '`@objectstack/driver-memory` has NO legal configuration for a time-triggered flow ' + + 'that touches per-organization data: it refuses any call handed a tenant scope ' + + '(`MEMORY_MULTI_TENANT_UNSUPPORTED`), so a declared flow is refused per call while an ' + + 'undeclared one is not armed at all. Multi-organization deployments use ' + + '`@objectstack/driver-sql`.', + }, { id: 'scim-provider-object-retired', surface: diff --git a/packages/spec/src/type-alias-convention.pin.test.ts b/packages/spec/src/type-alias-convention.pin.test.ts index 98b0651556..45c2386654 100644 --- a/packages/spec/src/type-alias-convention.pin.test.ts +++ b/packages/spec/src/type-alias-convention.pin.test.ts @@ -199,6 +199,7 @@ import type * as M177 from './data/date-macros.zod.js'; import type * as M178 from './data/field-value.zod.js'; import type * as M179 from './data/mapping.zod.js'; import type * as M180 from './security/sharing.zod.js'; +import type * as M186 from './automation/schedule-organization.zod.js'; import type * as M114 from './shared/metadata-types.zod.js'; import type * as M115 from './shared/protection.zod.js'; import type * as M116 from './stack.zod.js'; @@ -269,7 +270,7 @@ import type * as M184 from './shared/value-domain.zod.js'; import type * as M185 from './shared/epoch.zod.js'; // --------------------------------------------------------------------------- -// 782 isomorphic aliases: `z.input` === `z.infer`, so no `XParsed` is declared. +// 783 isomorphic aliases: `z.input` === `z.infer`, so no `XParsed` is declared. // // That number is machine-checked, not hand-kept. The runtime companion at the // bottom of this file recomputes the pin count from the source and asserts that @@ -628,6 +629,13 @@ export type Iso246 = Assert, z.infe // automation/time-relative-trigger.zod.ts export type Iso247 = Assert, z.infer< typeof M43.TimeRelativeTriggerSchema > >>; +// automation/schedule-organization.zod.ts +// [#16659] A bare non-empty string: no transform, no default, no coercion — an +// organization id is written exactly as it is stored. So input === infer, and an +// `XParsed` here would be a permanent synonym. The day this schema learns to +// normalize an id, this line goes red and the ADR's remedy applies. +export type Iso872 = Assert, z.infer< typeof M186.ScheduleOrganizationSchema > >>; + // automation/webhook.zod.ts export type Iso248 = Assert, z.infer< typeof M44.WebhookTriggerType > >>; @@ -1646,7 +1654,7 @@ describe('ADR-0122 type-alias convention', () => { // this title and the section header above the pin list — are now asserted // against the recomputed count below, so neither can go stale without a red // test naming it. - it('still declares all 782 isomorphic pins', () => { + it('still declares all 783 isomorphic pins', () => { // The truth of each pin is proved by tsc, not here — an `Assert>` // that stops holding is a compile error with the alias named. What tsc // cannot notice is a pin that was DELETED: removing the assertion removes @@ -2152,7 +2160,31 @@ describe('ADR-0122 type-alias convention', () => { // reader to `api/discovery.zod.ts` (M17), where it is pinned again as // `Iso871`. The four package-format modules (M50–M53) moved to // `marketplace/` and kept their pins. -30 + 1. - expect(pins).toHaveLength(782); + // + // 782 -> 783 is #16659's `ScheduleOrganizationSchema` + // (automation/schedule-organization.zod.ts, new module slot M186): the + // acting organization a time-triggered flow declares, a bare + // `z.string().min(1)` — no coercion, no default, no transform, because an + // organization id is written exactly as it is stored. The (RISE) case, one + // new pin. +1 added. + // + // ⚠️ Worth one line on how it ARRIVED, because the module is not new — only + // its NAME is. It shipped in the same card as `schedule-organization.ts`, + // and every gate in this family reads `*.zod.ts` only, so neither this pin + // file nor `check:spec-parsed-alias` could see it. Renaming the file to + // `.zod.ts` is what asked the question, and the answer was a real ADR-0122 + // violation (`z.infer` on the bare alias) sitting green behind an extension. + // + // ⚠️ Its id took a SECOND number on the merge, and that is the interesting + // half: this branch authored the pin as `Iso871` while #16325 landed its + // own `Iso871` (`EnvironmentTypeSchema`) on `main`. The two additions are + // disjoint pins, so the text merge took BOTH lines with no conflict marker + // and left two declarations sharing one id — a duplicate-identifier error + // tsc catches, but only after a merge that read clean. Renumbered here to + // `Iso872`, the next free id after the merged file's maximum; ids are + // claims about pins, not positions, so the collision costs nothing but a + // number. The merged count is 811 - 30 + 1 + 1. + expect(pins).toHaveLength(783); // The count is stated in PROSE twice as well — this case's title and the // section header above the pin list — and until #6605 nothing read either diff --git a/packages/triggers/trigger-schedule/README.md b/packages/triggers/trigger-schedule/README.md index 1de14fe7db..d9e4859850 100644 --- a/packages/triggers/trigger-schedule/README.md +++ b/packages/triggers/trigger-schedule/README.md @@ -14,13 +14,15 @@ engine baseline, a different event source. ## What it does -A flow whose `start` node declares a schedule: +A flow whose `start` node declares a schedule **and the organization it runs +as**: ```ts { type: 'start', config: { schedule: { type: 'cron', expression: '0 1 * * *', timezone: 'UTC' }, + organization: '', // REQUIRED — see below condition: "...", // optional start-condition gate }, } @@ -28,8 +30,29 @@ A flow whose `start` node declares a schedule: ``` auto-launches on that schedule — no manual `engine.execute()`. When it fires, -the flow runs with `event: 'schedule'` and `params: { jobId, flowName, schedule }` -in its context. +the flow runs with `event: 'schedule'`, `tenantId` set to the declared +organization, and `params: { jobId, flowName, schedule }` in its context. + +### The acting organization is required + +A scheduled run has no session to inherit a tenant from, so it carries no +organization unless the flow declares one. Without it every tenant-scoped write +beneath the run — the inbox rows a `notify` node emits, the +`sys_automation_run` history row — is refused on any install holding more than +one `sys_organization`, while the tick still reports itself healthy. + +So `config.organization` is **required on every `schedule` and `timeRelative` +flow**, and a flow that omits it is **refused at bind**: the trigger logs the +reason at `error` naming the flow, and throws, so the engine records the flow as +NOT bound — it appears in `getTriggerBindingAudit()` and in the CLI's startup +summary, and `getFlowRuntimeStates()` reports `bound: false`. There is +deliberately no fallback: no platform organization, no "the install's only one". +A sweep wanted in several organizations is declared once per organization; a +single flow is never fanned out across them. + +⚠️ The start node's `config` is an open record, so a near-miss spelling +(`organizationId`, `org_id`, `tenantId`, …) parses and is then ignored. The +refusal names the spelling you wrote. ### Schedule shapes @@ -89,12 +112,46 @@ schedule and launched **once per matching record**: filter: { status: 'active' }, // optional, ANDed with the date window maxRecords: 1000, // optional per-sweep cap (default 1000) }, + organization: '', // REQUIRED — same refusal as above schedule: { type: 'cron', expression: '0 8 * * *' }, // optional; defaults to daily 08:00 UTC condition: '...', // optional per-record start-condition gate }, } ``` +The sweep owes the acting organization for a **stronger** reason than a plain +schedule flow does, and it is the value that BOUNDS THE QUERY. The sweep runs +elevated on purpose (`context: { isSystem: true }` — a background sweep must see +every row, not the RLS-scoped subset an absent user would see), so nothing else +keeps its selection inside one organization: the declared id is passed as +`context.tenantId` on the same query, the engine turns that into +`DriverOptions.tenantId`, and the driver scopes the read. Selection and +identity are then the same organization. A `timeRelative` flow that declares +none takes the same bind-time refusal. + +⚠️ Elevation and tenancy are **independent axes** — `isSystem` decides what the +sweep is allowed to see, `tenantId` decides whose rows they are. A sweep that +passed only the first was a cross-organization scheduled task even with a +declaration on the flow: it matched rows in every tenant and launched runs +stamped with one, so `update_record` matched nothing, `notify` posted into the +declared organization's inbox about another organization's record, and the +history row landed under the record's organization rather than the run's. + +Two consequences worth knowing before you declare one: + +- **A store that cannot scope refuses the sweep rather than answering it.** + `@objectstack/driver-memory` implements no row-level tenant isolation and + refuses any call handed a tenant scope (`MEMORY_MULTI_TENANT_UNSUPPORTED`), so + a scoped sweep there fails loudly every tick instead of quietly selecting + every organization's rows. Multi-organization deployments use + `@objectstack/driver-sql`. +- **On a platform-global or federated object the declaration cannot narrow + anything.** The engine drops the tenant scope for an object declaring + `tenancy: { enabled: false }` (ADR-0066) or carrying `external` (ADR-0015), so + such a sweep still selects across every organization while its runs act as the + declared one. The trigger says so at bind time, at `warn`, naming the object — + ⛔ it does not pretend the flow is contained. + The matched record rides on the automation context (`event: 'time_relative'`, `record`, `params`), so the start-node `condition` gate and `{record.}` interpolation work exactly as for a record-change flow. Because the window is diff --git a/packages/triggers/trigger-schedule/src/kernel-rebuild-rebind.test.ts b/packages/triggers/trigger-schedule/src/kernel-rebuild-rebind.test.ts index 8ce6bc7d9c..9a952f0052 100644 --- a/packages/triggers/trigger-schedule/src/kernel-rebuild-rebind.test.ts +++ b/packages/triggers/trigger-schedule/src/kernel-rebuild-rebind.test.ts @@ -103,7 +103,7 @@ describe('#8362 — a rebuilt kernel re-binds scheduled flows (both triggers)', const FLOW = 'nightly_contract_rollup'; const JOB = `flow-schedule:${FLOW}`; const fired: string[] = []; - const binding: FlowTriggerBinding = { flowName: FLOW, schedule: DAILY }; + const binding: FlowTriggerBinding = { flowName: FLOW, schedule: DAILY, organization: 'org_2mtx1w9d0k4bqf7v' }; // ── kernel 1 ────────────────────────────────────────────────────── const k1 = cronBackedJobService(); @@ -144,6 +144,7 @@ describe('#8362 — a rebuilt kernel re-binds scheduled flows (both triggers)', const binding: FlowTriggerBinding = { flowName: FLOW, schedule: DAILY, + organization: 'org_2mtx1w9d0k4bqf7v', config: { timeRelative: { object: 'xqao_contract', dateField: 'expiry_date', offsetDays: [3] }, }, @@ -186,7 +187,7 @@ describe('#8362 — a failed bind is reported where an operator sees it', () => it('ScheduleTrigger reports at ERROR, naming the consequence and the remedy', async () => { const logger = recordingLogger(); const trigger = new ScheduleTrigger(() => rejectingService(), logger); - trigger.start({ flowName: 'nightly_rollup', schedule: DAILY }, async () => {}); + trigger.start({ flowName: 'nightly_rollup', schedule: DAILY, organization: 'org_2mtx1w9d0k4bqf7v' }, async () => {}); await flush(); expect(logger.errors).toHaveLength(1); @@ -209,6 +210,7 @@ describe('#8362 — a failed bind is reported where an operator sees it', () => { flowName: 'xqao_contract_expiry_reminder_flow', schedule: DAILY, + organization: 'org_2mtx1w9d0k4bqf7v', config: { timeRelative: { object: 'xqao_contract', dateField: 'expiry_date', offsetDays: [3] }, }, diff --git a/packages/triggers/trigger-schedule/src/schedule-dispatch-claim.test.ts b/packages/triggers/trigger-schedule/src/schedule-dispatch-claim.test.ts index c0bc68c329..d96380f18b 100644 --- a/packages/triggers/trigger-schedule/src/schedule-dispatch-claim.test.ts +++ b/packages/triggers/trigger-schedule/src/schedule-dispatch-claim.test.ts @@ -42,6 +42,8 @@ const JOB = `flow-schedule:${FLOW}`; const CRON: FlowTriggerBinding = { flowName: FLOW, schedule: { type: 'cron', expression: '0 1 * * *', timezone: 'UTC' }, + // [#16659] the acting organization every tick of this flow runs as. + organization: 'org_2mtx1w9d0k4bqf7v', }; /** Inside the 2026-09-07T01:00Z window of `0 1 * * *`. */ diff --git a/packages/triggers/trigger-schedule/src/schedule-runas-e2e.test.ts b/packages/triggers/trigger-schedule/src/schedule-runas-e2e.test.ts index dfc8805744..4709dd5659 100644 --- a/packages/triggers/trigger-schedule/src/schedule-runas-e2e.test.ts +++ b/packages/triggers/trigger-schedule/src/schedule-runas-e2e.test.ts @@ -58,7 +58,10 @@ function scheduledDataFlow(name: string, runAs?: 'system' | 'user') { type: 'schedule', ...(runAs ? { runAs } : {}), nodes: [ - { id: 'start', type: 'start', label: 'Start', config: { schedule: { type: 'interval', intervalMs: 1000 } } }, + // [#16659] The acting organization a time-triggered flow declares. The + // engine lifts it onto the binding and the trigger threads it onto the + // run as `tenantId`; a flow without it is refused at bind. + { id: 'start', type: 'start', label: 'Start', config: { schedule: { type: 'interval', intervalMs: 1000 }, organization: 'org_2mtx1w9d0k4bqf7v' } }, { id: 'mk', type: 'create_record', label: 'Create', config: { objectName: 'thing', fields: { a: 1 } } }, { id: 'end', type: 'end', label: 'End' }, ], diff --git a/packages/triggers/trigger-schedule/src/schedule-trigger.test.ts b/packages/triggers/trigger-schedule/src/schedule-trigger.test.ts index 502245cd7f..57a010645a 100644 --- a/packages/triggers/trigger-schedule/src/schedule-trigger.test.ts +++ b/packages/triggers/trigger-schedule/src/schedule-trigger.test.ts @@ -5,6 +5,7 @@ import type { AutomationContext, JobSchedule, JobHandler } from '@objectstack/sp import { ScheduleTrigger, normalizeSchedule, + resolveBindingOrganization, type FlowTriggerBinding, type JobServiceSurface, type TriggerLogger, @@ -43,10 +44,30 @@ function silentLogger(): TriggerLogger { return { info: () => {}, warn: () => {}, debug: () => {} }; } +/** Keeps every line, so the refusal suite can read the `error` channel. */ +function recordingLogger(): { logger: TriggerLogger; errors: string[]; warns: string[] } { + const errors: string[] = []; + const warns: string[] = []; + return { + logger: { + info: () => {}, + debug: () => {}, + warn: (msg: string) => void warns.push(String(msg)), + error: (msg: string) => void errors.push(String(msg)), + }, + errors, + warns, + }; +} + function binding(overrides: Partial = {}): FlowTriggerBinding { return { flowName: 'nightly_health_sweep', schedule: { type: 'cron', expression: '0 1 * * *', timezone: 'UTC' }, + // [#16659] A time-triggered binding carries its acting organization; a + // binding without one is refused — see + // `ScheduleTrigger — the acting-organization refusal (#16659)` below. + organization: 'org_2mtx1w9d0k4bqf7v', ...overrides, }; } @@ -293,3 +314,174 @@ describe('ScheduleTriggerPlugin', () => { expect(job.jobs.size).toBe(1); }); }); + +// ─── The acting-organization refusal (#16659) ─────────────────────── +// +// The unit half of the card's consequence (3): a time-triggered flow that +// declares no acting organization is REFUSED at bind, and the refusal reaches +// the engine rather than only stderr. +// +// ⚠️ Every assertion here would pass vacuously against a trigger that refused +// EVERYTHING, so each limb that expects a refusal is paired with the declaring +// binding from `binding()` above, which must still arm. + +describe('ScheduleTrigger — the acting-organization refusal (#16659)', () => { + const orgLess = () => binding({ organization: undefined, config: {} }); + + it('THROWS from start(), so the engine cannot record the flow as bound', () => { + const job = fakeJobService(); + const log = recordingLogger(); + const trigger = new ScheduleTrigger(() => job.service, log.logger); + + // ⭐ The whole of F1: `FlowTrigger.start` is `void`, so a logged-and- + // returned refusal is indistinguishable from a successful arm and the + // engine sets `boundFlowTriggers` anyway. The throw is the engine's + // designed catch path. + expect(() => trigger.start(orgLess(), async () => {})).toThrow(/declares no acting organization/); + expect(job.jobs.size, 'a refused flow must have no job at all').toBe(0); + }); + + it('logs the same sentence at `error`, naming the flow, the key and NOT BOUND', () => { + const job = fakeJobService(); + const log = recordingLogger(); + const trigger = new ScheduleTrigger(() => job.service, log.logger); + + let thrown = ''; + try { + trigger.start(orgLess(), async () => {}); + } catch (err) { + thrown = String((err as Error).message); + } + + expect(log.errors, 'the refusal is an `error`, not a `warn`').toHaveLength(1); + const line = log.errors[0]; + expect(line).toContain('NOT BOUND'); + expect(line, 'the refusal must be attributable to a flow, not to "a flow"').toContain( + 'nightly_health_sweep', + ); + expect(line, 'it must name the key the author has to write').toContain('organization'); + // The loud channel and the thrown text the engine's audit points at + // must not be able to drift apart. + expect(line).toContain(thrown); + }); + + it('names the near-miss spelling the author actually wrote', () => { + const job = fakeJobService(); + const log = recordingLogger(); + const trigger = new ScheduleTrigger(() => job.service, log.logger); + + expect(() => + trigger.start( + binding({ organization: undefined, config: { organizationId: 'org_written_wrong' } }), + async () => {}, + ), + ).toThrow(); + + // The start node's `config` is an open record, so `organizationId` was + // accepted and then ignored — the refusal is the only place that + // becomes visible. + expect(log.errors[0]).toContain('organizationId'); + }); + + it('⛔ never picks an organization for the author', () => { + const job = fakeJobService(); + const log = recordingLogger(); + const trigger = new ScheduleTrigger(() => job.service, log.logger); + + expect(() => + trigger.start( + binding({ organization: undefined, config: { organizationId: 'org_written_wrong' } }), + async () => {}, + ), + ).toThrow(); + + // The refusal names the KEY the author misspelt and never their VALUE: + // echoing an id back is one edit away from acting on it, and the one + // value in scope here is precisely the one nothing may adopt. + expect(log.errors[0]).not.toContain('org_written_wrong'); + expect(job.jobs.size).toBe(0); + }); + + it('a hot re-publish that REMOVES the key drops the prior job', async () => { + const job = fakeJobService(); + const log = recordingLogger(); + const trigger = new ScheduleTrigger(() => job.service, log.logger); + + trigger.start(binding(), async () => {}); + await flush(); + expect(job.jobs.size, 'precondition: the declaring binding armed').toBe(1); + + // ⭐ Without the `stop()` that precedes the throw, the previous, still + // armed job keeps firing org-less ticks behind an error saying the flow + // was refused — the exact "armed, listed and inert" shape this card closes. + expect(() => trigger.start(orgLess(), async () => {})).toThrow(); + await flush(); + expect(job.jobs.size).toBe(0); + }); + + it('refusing one flow does not disarm a sibling', async () => { + const job = fakeJobService(); + const log = recordingLogger(); + const trigger = new ScheduleTrigger(() => job.service, log.logger); + + trigger.start(binding({ flowName: 'declares_one' }), async () => {}); + await flush(); + expect(() => + trigger.start(binding({ flowName: 'declares_none', organization: undefined, config: {} }), async () => {}), + ).toThrow(); + await flush(); + + expect(job.jobs.has('flow-schedule:declares_one')).toBe(true); + expect(job.jobs.has('flow-schedule:declares_none')).toBe(false); + }); + + it('falls back to `warn` when the logger has no `error` channel', () => { + const job = fakeJobService(); + const warn = vi.fn(); + const trigger = new ScheduleTrigger(() => job.service, { info: () => {}, warn, debug: () => {} }); + + expect(() => trigger.start(orgLess(), async () => {})).toThrow(); + expect(warn, 'the refusal must still be said, not swallowed').toHaveBeenCalled(); + }); +}); + +describe('resolveBindingOrganization (#16659)', () => { + it('reads the lifted binding field first', () => { + expect(resolveBindingOrganization(binding())).toBe('org_2mtx1w9d0k4bqf7v'); + }); + + it('falls back to the raw start-node config, for an engine that predates the lift', () => { + // ⭐ Not redundancy: the binding is a STRUCTURAL mirror, so a host on an + // older engine hands this trigger no `organization` field and a `config` + // that still carries the author's declaration. Refusing there would + // report an engine-version skew as an authoring error. + expect( + resolveBindingOrganization( + binding({ organization: undefined, config: { organization: 'org_from_config' } }), + ), + ).toBe('org_from_config'); + }); + + it('the lifted field wins over the raw config', () => { + expect( + resolveBindingOrganization(binding({ config: { organization: 'org_stale' } })), + ).toBe('org_2mtx1w9d0k4bqf7v'); + }); + + it.each([ + ['absent', undefined], + ['an empty string', ''], + ['a number', 42], + ['an object', { id: 'org_x' }], + ['null', null], + ])('answers null for a value that is %s', (_label, value) => { + // A present-but-unusable value takes the refusal path: "declared" must + // mean "usable", or a flow admitted by one layer and refused by the next + // is the silent hole again. + expect( + resolveBindingOrganization( + binding({ organization: undefined, config: { organization: value } as Record }), + ), + ).toBeNull(); + }); +}); diff --git a/packages/triggers/trigger-schedule/src/schedule-trigger.ts b/packages/triggers/trigger-schedule/src/schedule-trigger.ts index e91602a727..7d381a9cd7 100644 --- a/packages/triggers/trigger-schedule/src/schedule-trigger.ts +++ b/packages/triggers/trigger-schedule/src/schedule-trigger.ts @@ -3,6 +3,11 @@ import { Cron } from 'croner'; import type { AutomationContext } from '@objectstack/spec/contracts'; import type { JobSchedule, JobHandler } from '@objectstack/spec/contracts'; +import { + SCHEDULE_ORGANIZATION_KEY, + ScheduleOrganizationSchema, + describeMissingScheduleOrganization, +} from '@objectstack/spec/automation'; /** * Structural mirror of the automation engine's `FlowTriggerBinding` @@ -18,6 +23,21 @@ export interface FlowTriggerBinding { readonly event?: string; readonly condition?: string | { dialect?: string; source?: string; ast?: unknown }; readonly schedule?: unknown; + /** + * [#16659] The ACTING ORGANIZATION a time-triggered flow declares on its + * start node (`config.organization`), lifted onto the binding by the + * engine's `resolveTriggerBinding` the same way `schedule` is. + * + * Optional on this interface and REQUIRED by the two time triggers — the + * split is deliberate. The interface is the structural mirror of the + * engine's binding, which is shared with `record_change` and `api` flows + * that legitimately carry none (their trigger threads the firing session's + * own tenant). "Absent" is therefore a real state the type must be able to + * express; what must not exist is a time-triggered run that PROCEEDS + * without it, and that verdict is {@link resolveBindingOrganization}'s, + * one layer down. + */ + readonly organization?: string; readonly config?: Record; } @@ -222,6 +242,106 @@ export interface TriggerLogger { const JOB_PREFIX = 'flow-schedule'; +/** + * Resolve the acting organization of a time-triggered binding (#16659), or + * `null` when the flow declared none. + * + * Reads the binding's lifted `organization` first and the raw start-node + * `config` second. The second read is not redundancy for its own sake: the + * binding is a STRUCTURAL mirror of the engine's type, so a host running an + * engine that predates the lift hands this trigger a binding with no + * `organization` field and a `config` that still carries the author's + * declaration. Reading only the lifted field there would report a correctly + * declared flow as organization-less and refuse it — turning an engine-version + * skew into an authoring error, which is the wrong diagnosis pointed at the + * wrong person. + * + * A present-but-unusable value (empty string, a number) resolves to `null` and + * takes the refusal path, exactly as {@link resolveScheduleOrganization} does + * at validation: this trigger and the validator must agree about what counts + * as declared, or a flow refused by one and admitted by the other is the + * silent hole again. + */ +export function resolveBindingOrganization(binding: FlowTriggerBinding): string | null { + const lifted = ScheduleOrganizationSchema.safeParse(binding.organization); + if (lifted.success) return lifted.data; + const raw = binding.config?.[SCHEDULE_ORGANIZATION_KEY]; + const declared = ScheduleOrganizationSchema.safeParse(raw); + return declared.success ? declared.data : null; +} + +/** + * Refuse to bind a time-triggered flow that declares no acting organization + * (#16659): say why at `error`, then THROW so the engine records the refusal. + * + * ## Why it throws, and does not merely log and return + * + * `FlowTrigger.start` returns `void`, so a trigger that logs and returns is + * indistinguishable — to the engine — from one that armed successfully. The + * engine's `activateFlowTrigger` then runs `boundFlowTriggers.set(flowName, …)` + * and logs `Flow '' bound to trigger 'schedule'` one line after this + * function said NOT BOUND, and every structured surface built for exactly this + * state reports the opposite of it: `getFlowRuntimeStates()` (Studio's status + * badge) answers `bound: true`, and `getTriggerBindingAudit()` — the silent-miss + * audit the automation plugin warns from at `kernel:bootstrapped` and the CLI + * prints in its startup summary — skips the flow because it is in + * `boundFlowTriggers`. A refusal only an operator reading stderr can see, in a + * repo that built three machine-readable channels to say "declared but not + * armed", is the same silent-miss shape this card exists to close. + * + * Throwing is the engine's DESIGNED path for this: `activateFlowTrigger` wraps + * `trigger.start(...)` in a `try/catch` whose `catch` logs the plugin-supplied + * thrown text and — because the `set` is inside the `try`, after the call — never + * marks the flow bound. The audit then lists it with `binding failed — see + * earlier warnings`, which points at the `error` line this function already + * emitted. The message is the same sentence both times, so the loud channel and + * the structured channel cannot drift. + * + * ## Why this REFUSES rather than binding and degrading + * + * The whole defect this closes is a run that looked healthy while delivering + * nothing: the tick selected its rows, landed its `update_record` steps, + * reported `unmeasured=0`, and every tenant-scoped write beneath it — the + * inbox rows and the `sys_automation_run` history row — was refused one layer + * down where nothing summarised it. Binding such a flow and warning once at + * boot would reproduce exactly that shape: a flow that is armed, listed, and + * inert. So the flow is NOT bound, and the reason names it. + * + * ## Why `error` and not `warn` + * + * The repo's degradation-log-level rule asks one question: after the + * degradation, does the system still look normal from the outside while + * something it claims is in place has not landed? It does, completely — the + * flow stays published and active in `sys_metadata`, Studio lists it, the + * metadata API serves it and `verify_build` passes — which is the same + * reasoning {@link reportBindFailure} records for its own branch, and the same + * `error` class. + * + * ⛔ There is deliberately no limb here that picks an organization. Not the + * install's only one, not the platform organization, not the first row of + * `sys_organization`. A wrong `organization_id` is worse than a refusal: a + * refusal is visible at boot and names its flow, while a wrong value is + * silently authoritative to every report, export and cleanup script that + * filters by organization. + */ +export function refuseMissingOrganization( + logger: TriggerLogger, + tag: 'schedule' | 'time-relative', + flowName: string, + binding: FlowTriggerBinding, +): never { + const sentence = describeMissingScheduleOrganization(flowName, { + kind: tag === 'time-relative' ? 'time_relative' : 'schedule', + // The start node's `config` as the engine handed it over; the near-miss + // scan is `@objectstack/spec`'s and runs inside the sentence, so a + // trigger cannot report a spelling the scan would not have found. + config: binding.config, + }); + const report = logger.error?.bind(logger) ?? logger.warn.bind(logger); + report(`[${tag}] NOT BOUND — ${sentence}`); + throw new Error(sentence); +} + /** * Report a scheduled flow that failed to bind to the job service. * @@ -403,6 +523,21 @@ export class ScheduleTrigger implements FlowTrigger { return; } + // [#16659] The acting organization is part of the BINDING, so it is + // checked before the job service is even resolved: a flow that cannot + // legally run must not be reported as "not scheduled because the job + // service is missing", which is a different defect with a different + // remedy. + const organization = resolveBindingOrganization(binding); + if (organization === null) { + // Drop any prior binding for this flow FIRST. A hot re-publish that + // REMOVES the organization must not leave the previous, still-armed + // job firing org-less ticks behind an error that says it was + // refused — and the throw below leaves this method immediately. + this.stop(binding.flowName); + refuseMissingOrganization(this.logger, 'schedule', binding.flowName, binding); + } + const jobService = this.getJobService(); if (!jobService || typeof jobService.schedule !== 'function') { this.logger.warn( @@ -442,6 +577,21 @@ export class ScheduleTrigger implements FlowTrigger { try { const ctx: AutomationContext = { event: 'schedule', + // [#16659] The run executes AS this organization. This is + // the one line the whole card is about: `tenantId` is the + // acting run's organization, and every consumer already + // reads it — `notify-node.ts` threads it onto the + // notification it emits (#11303), and the engine copies it + // onto the `sys_automation_run` history row (#10101). The + // producer was simply never supplying a value, so both + // consumers resolved NULL and the tenancy guard refused the + // rows beneath them. + // + // ⛔ Never conditional. `organization` is non-null here by + // construction — the bind above refused the flow otherwise + // — and spelling this `...(organization ? {…} : {})` would + // re-open the org-less run as a silent state. + tenantId: organization, params: { jobId, flowName: binding.flowName, diff --git a/packages/triggers/trigger-schedule/src/time-relative-trigger.test.ts b/packages/triggers/trigger-schedule/src/time-relative-trigger.test.ts index 3a02867ac8..16b513da4e 100644 --- a/packages/triggers/trigger-schedule/src/time-relative-trigger.test.ts +++ b/packages/triggers/trigger-schedule/src/time-relative-trigger.test.ts @@ -49,7 +49,7 @@ interface FindCall { objectName: string; where: Record; limit?: number; - context?: { isSystem?: boolean }; + context?: { isSystem?: boolean; tenantId?: string }; } /** @@ -74,6 +74,39 @@ function fakeDataEngine(rows: Row[], knownObjects: string[] = ['contracts']) { return { engine, calls }; } +/** + * [#16659] A fake ObjectQL surface that HONOURS `context.tenantId`, so the + * differential control can put matching rows in two organizations and observe + * which ones come back. + * + * The scope it implements is the SQL driver's documented one — `tenantId` + * present ⇒ `(organization_id = :tenant OR organization_id IS NULL)`; `tenantId` + * absent ⇒ no predicate at all, which is exactly how the unfixed sweep read. + * ⛔ Not a convenience double that filters whatever it is handed: the "absent" + * arm has to reproduce the DEFECT, or ablating the fix would still look scoped. + */ +function tenantScopedDataEngine(rows: Row[], knownObjects: string[] = ['contracts']) { + const calls: FindCall[] = []; + const engine: TimeRelativeDataEngine = { + async find(objectName, query) { + const where = (query?.where ?? {}) as Record; + calls.push({ objectName, where, limit: query?.limit, context: query?.context }); + const tenant = query?.context?.tenantId; + const scoped = rows.filter((row) => { + if (tenant === undefined) return true; + const org = row.organization_id; + return org === tenant || org == null; + }); + const out = scoped.filter((row) => matches(row, where)); + return typeof query?.limit === 'number' ? out.slice(0, query.limit) : out; + }, + getObject(name) { + return knownObjects.includes(name) ? { name } : undefined; + }, + }; + return { engine, calls }; +} + /** Minimal where matcher: temporal range on the date field + scalar equality. */ function matches(row: Row, where: Record): boolean { for (const [key, cond] of Object.entries(where)) { @@ -98,11 +131,21 @@ function silentLogger(): TriggerLogger { /** Fixed reference clock: 2026-07-18 (noon UTC). */ const NOW = () => new Date('2026-07-18T12:00:00.000Z'); +/** + * [#16659] The organization every fixture binding declares. Named rather than + * inlined because it is now asserted from two directions — the sweep's query + * scope and the launched run's identity — and a literal repeated at both ends + * of that pair can drift into agreeing with itself. + */ +const TEST_ORG = 'org_2mtx1w9d0k4bqf7v'; + function binding(timeRelative: unknown, overrides: Partial = {}): FlowTriggerBinding { return { flowName: 'renewal_alert', object: 'contracts', config: { timeRelative }, + // [#16659] see the schedule trigger's fixture note. + organization: TEST_ORG, ...overrides, }; } @@ -225,8 +268,14 @@ describe('TimeRelativeTrigger', () => { // Context is record-shaped (so `{record.x}` + start conditions work). expect(seen[0]).toMatchObject({ object: 'contracts', event: 'time_relative' }); expect(seen[0].record).toBe(seen[0].params); - // The sweep queries as a system op (sees all rows, RLS-bypassing). - expect(calls[0].context).toEqual({ isSystem: true }); + // The sweep queries as a system op (sees all rows, RLS-bypassing) AND + // inside its declared organization. [#16659] This assertion used to + // read `{ isSystem: true }` and it was pinning the defect: `isSystem` + // is AUTHORIZATION and `tenantId` is TENANCY, and a sweep carrying only + // the first selects across every tenant while its runs act as one. + // ⛔ Do not relax it back to a subset match — the exact-equality is + // what makes "the sweep asks for no scope" red. + expect(calls[0].context).toEqual({ isSystem: true, tenantId: TEST_ORG }); expect(calls[0].where).toEqual({ status: 'active', end_date: { $gte: '2026-07-18T00:00:00.000Z', $lte: '2026-09-16T23:59:59.999Z' }, @@ -727,3 +776,261 @@ describe('TimeRelativeTriggerPlugin', () => { await expect(fake.readyHandlers[0]()).resolves.toBeUndefined(); }); }); + +// ─── The acting-organization refusal (#16659) ─────────────────────── +// +// The time-relative sweep is NOT the weaker case for carrying an organization, +// it is the stronger one: it runs ELEVATED on purpose (`isSystem` — a +// background sweep must see every row, not the RLS-scoped subset), so the +// declaration is the only thing keeping its SELECTION inside one organization. +// An org-less sweep selects across every tenant and then launches a run that +// can write into none of them; a sweep whose declaration reached only the run +// selects across every tenant and launches runs that write into ONE, which is +// worse. Both halves are pinned below. + +describe('TimeRelativeTrigger — the acting-organization refusal (#16659)', () => { + const DESC = { object: 'contracts', dateField: 'end_date', withinDays: 60 }; + + function recordingLogger(): { logger: TriggerLogger; errors: string[]; warns: string[] } { + const errors: string[] = []; + const warns: string[] = []; + return { + logger: { + info: () => {}, + debug: () => {}, + warn: (msg: string) => void warns.push(String(msg)), + error: (msg: string) => void errors.push(String(msg)), + }, + errors, + warns, + }; + } + + it('THROWS from start() and arms no sweep, naming the flow and the key', () => { + const job = fakeJobService(); + const { engine } = fakeDataEngine([]); + const log = recordingLogger(); + const trigger = new TimeRelativeTrigger(() => job.service, () => engine, log.logger, NOW); + + expect(() => + trigger.start(binding(DESC, { organization: undefined, config: { timeRelative: DESC } }), async () => {}), + ).toThrow(/declares no acting organization/); + + expect(job.jobs.size, 'a refused sweep must have no job at all').toBe(0); + expect(log.errors).toHaveLength(1); + expect(log.errors[0]).toContain('[time-relative] NOT BOUND'); + expect(log.errors[0]).toContain('renewal_alert'); + // The sentence is the time-relative one, not the plain-schedule one. + expect(log.errors[0]).toContain('time-relative flow'); + }); + + it('a hot re-publish that REMOVES the key drops the prior sweep', async () => { + const job = fakeJobService(); + const { engine } = fakeDataEngine([]); + const log = recordingLogger(); + const trigger = new TimeRelativeTrigger(() => job.service, () => engine, log.logger, NOW); + + trigger.start(binding(DESC), async () => {}); + await flush(); + expect(job.jobs.size, 'precondition: the declaring binding armed').toBe(1); + + expect(() => + trigger.start(binding(DESC, { organization: undefined, config: { timeRelative: DESC } }), async () => {}), + ).toThrow(); + await flush(); + expect(job.jobs.size).toBe(0); + }); + + // ── the SELECTION half (#16659, F2) ─────────────────────────────────── + // + // Declaring an organization bounded the RUN and left the QUERY unbounded, + // so a sweep declared for A matched rows in every tenant and launched runs + // stamped A about other organizations' records. These pins are about the + // query. + + it('EVERY window query carries the declared organization, not just the first', async () => { + // Offset mode issues one query per offset — a scope threaded onto only + // the first would leave the rest crossing organizations, and a pin that + // read `calls[0]` alone would not notice. + const job = fakeJobService(); + const { engine, calls } = fakeDataEngine([]); + const trigger = new TimeRelativeTrigger(() => job.service, () => engine, silentLogger(), NOW); + + trigger.start( + binding({ object: 'contracts', dateField: 'end_date', offsetDays: [60, 30, 7] }), + async () => {}, + ); + await flush(); + await job.fire('flow-time-relative:renewal_alert'); + + expect(calls.length, 'offset mode must issue one query per offset').toBe(3); + expect( + calls.map((c) => c.context?.tenantId ?? 'NO-SCOPE'), + 'an unscoped window query selects every organization\'s rows', + ).toEqual([TEST_ORG, TEST_ORG, TEST_ORG]); + // The scope is the ONLY thing tenancy contributes: the author's filter + // and the date window are untouched, so no organization predicate was + // hand-built onto `where` (which would hardcode a column name the + // object is free to rename, and select nothing where there is none). + for (const call of calls) { + expect(Object.keys(call.where)).toEqual(['end_date']); + } + }); + + it('DIFFERENTIAL: with matching rows in two organizations only the declared one is swept', async () => { + // The discriminating shape. A pin that only proved "A's rows are found" + // passes on the defect too — the defect FOUND them, alongside B's. + // + // The double implements the documented driver contract rather than a + // convenient one: `DriverOptions.tenantId` scopes to + // `(organization_id = :tenant OR organization_id IS NULL)` + // (sql-driver's own `tenantFieldByTable` note), and an ABSENT scope + // applies no predicate at all — which is precisely how the unfixed + // sweep read. + const ORG_B = 'org_beta_0000000000000'; + const rows: Row[] = [ + { id: 'a1', end_date: '2026-07-25T00:00:00.000Z', organization_id: TEST_ORG }, + { id: 'b1', end_date: '2026-07-25T00:00:00.000Z', organization_id: ORG_B }, + { id: 'b2', end_date: '2026-07-26T00:00:00.000Z', organization_id: ORG_B }, + ]; + const job = fakeJobService(); + const { engine, calls } = tenantScopedDataEngine(rows); + const trigger = new TimeRelativeTrigger(() => job.service, () => engine, silentLogger(), NOW); + const seen: AutomationContext[] = []; + + trigger.start( + binding({ object: 'contracts', dateField: 'end_date', withinDays: 60 }), + async (ctx) => void seen.push(ctx), + ); + await flush(); + await job.fire('flow-time-relative:renewal_alert'); + + expect( + seen.map((c) => (c.record as Row).id), + 'the sweep launched a run for a record in an organization the flow never declared', + ).toEqual(['a1']); + expect( + seen.map((c) => c.tenantId), + 'and the run still acts as the declared organization', + ).toEqual([TEST_ORG]); + expect(calls[0].context?.tenantId, 'the scope must reach the engine, not be applied afterwards').toBe(TEST_ORG); + }); + + it('NEGATIVE CONTROL: a single-organization install selects exactly what it selected before', async () => { + // The narrowing must remove NOTHING where there is nothing to remove. + // Two legs over ONE fixture: + // A — the sweep as it ships, scoped to the declared organization; + // B — the SAME query with `context: { isSystem: true }` and no scope, + // which is the pre-fix sweep byte for byte. + // Equal answers is the claim; ⛔ the pair is what makes it a measurement + // rather than an argument, and the DIFFERENTIAL above is its opposite + // limb — add a second organization and these two legs must diverge. + // + // The NULL-organization row is in the fixture on purpose: the driver's + // scope is `org = :tenant OR org IS NULL`, so a platform row stays + // visible to a scoped read. That is part of "identical", not an + // exception to it. + const DESC = { object: 'contracts', dateField: 'end_date', withinDays: 60 }; + const rows: Row[] = [ + { id: 'c1', end_date: '2026-07-25T00:00:00.000Z', organization_id: TEST_ORG }, + { id: 'c2', end_date: '2026-08-01T00:00:00.000Z', organization_id: TEST_ORG }, + { id: 'c3', end_date: '2026-07-20T00:00:00.000Z', organization_id: null }, + ]; + const job = fakeJobService(); + const { engine, calls } = tenantScopedDataEngine(rows); + const trigger = new TimeRelativeTrigger(() => job.service, () => engine, silentLogger(), NOW); + const seen: AutomationContext[] = []; + + trigger.start(binding(DESC), async (ctx) => void seen.push(ctx)); + await flush(); + await job.fire('flow-time-relative:renewal_alert'); + const scoped = seen.map((c) => (c.record as Row).id); + + // Leg B, through the same double: the query the sweep used to send. + const window = computeDateWindows(DESC, NOW())[0]; + const before = + (await engine.find('contracts', { + where: buildWindowWhere(DESC, window), + limit: 1000, + context: { isSystem: true }, + })) ?? []; + const unscoped = before.map((r) => r.id); + + expect( + scoped, + 'a single-organization install must see the same rows it saw before — the scope removes nothing there', + ).toEqual(unscoped); + // Non-vacuity, both directions: the set is not empty, and the scope was + // genuinely applied rather than quietly absent. + expect(scoped, 'an empty answer would make the equality above pass with everything broken').toEqual(['c1', 'c2', 'c3']); + expect(calls[0].context?.tenantId, 'leg A must really have asked for a scope').toBe(TEST_ORG); + }); + + it('a store that CANNOT honour the scope is reported at `error`, never answered unscoped', async () => { + // `driver-memory` refuses any call handed a tenant scope (#16589). A + // sweep required to stay inside one organization, talking to a store + // that cannot keep it there, must be LOUD — "selected nothing this + // tick" and "cannot select at all" are different facts. + const job = fakeJobService(); + const engine: TimeRelativeDataEngine = { + async find(_objectName, query) { + if (query?.context?.tenantId !== undefined) { + throw Object.assign(new Error('[driver-memory] Refusing to answer: this driver has NO row-level tenant isolation.'), { + code: 'MEMORY_MULTI_TENANT_UNSUPPORTED', + }); + } + return []; + }, + getObject: () => ({ name: 'contracts' }), + }; + const log = recordingLogger(); + const trigger = new TimeRelativeTrigger(() => job.service, () => engine, log.logger, NOW); + + trigger.start(binding({ object: 'contracts', dateField: 'end_date', withinDays: 60 }), async () => {}); + await flush(); + await job.fire('flow-time-relative:renewal_alert'); + + const failure = log.errors.find((l) => l.includes('sweep failed')); + expect(failure, `the sweep failed silently; errors seen: ${JSON.stringify(log.errors)}`).toBeTruthy(); + expect(failure, 'the failure must name the flow it belongs to').toContain('renewal_alert'); + expect(failure, "and carry the store's own reason").toContain('NO row-level tenant isolation'); + }); + + it('says so at bind when the swept object is one the engine will NOT scope', async () => { + // ⚠️ The quiet direction. A `tenancy.enabled: false` object (ADR-0066) + // is exempt from the engine's tenant scope, so the declaration cannot + // narrow this sweep at all — it still selects across every + // organization, while the flow's `organization` line makes it LOOK + // contained. Nothing here changes which rows come back; the pin is that + // the operator is TOLD. + const job = fakeJobService(); + const { engine } = fakeDataEngine([]); + engine.getObject = () => ({ name: 'contracts', tenancy: { enabled: false } }); + const log = recordingLogger(); + const trigger = new TimeRelativeTrigger(() => job.service, () => engine, log.logger, NOW); + + trigger.start(binding({ object: 'contracts', dateField: 'end_date', withinDays: 60 }), async () => {}); + await flush(); + + const said = log.warns.find((l) => l.includes('tenancy')); + expect(said, `nothing was said; warns seen: ${JSON.stringify(log.warns)}`).toBeTruthy(); + expect(said, 'the warning must name the object whose declaration makes the scope inert').toContain('contracts'); + expect(said, 'and state the consequence, not just the fact').toContain('does NOT narrow this sweep'); + expect(job.jobs.size, 'the sweep still binds — this is a disclosure, not a refusal').toBe(1); + }); + + it('control: a tenant-scoped object gets NO such warning', async () => { + const job = fakeJobService(); + const { engine } = fakeDataEngine([]); + const log = recordingLogger(); + const trigger = new TimeRelativeTrigger(() => job.service, () => engine, log.logger, NOW); + + trigger.start(binding({ object: 'contracts', dateField: 'end_date', withinDays: 60 }), async () => {}); + await flush(); + + expect( + log.warns.filter((l) => l.includes('does NOT narrow this sweep')), + 'an ordinary object must not be warned about — that would train operators to ignore the line', + ).toHaveLength(0); + }); +}); diff --git a/packages/triggers/trigger-schedule/src/time-relative-trigger.ts b/packages/triggers/trigger-schedule/src/time-relative-trigger.ts index 27138da89d..385bb8ec55 100644 --- a/packages/triggers/trigger-schedule/src/time-relative-trigger.ts +++ b/packages/triggers/trigger-schedule/src/time-relative-trigger.ts @@ -7,7 +7,12 @@ import { TIME_RELATIVE_DEFAULT_MAX_RECORDS, } from '@objectstack/spec/automation'; import type { TimeRelativeTrigger as TimeRelativeDescriptor } from '@objectstack/spec/automation'; -import { normalizeSchedule, reportBindFailure } from './schedule-trigger.js'; +import { + normalizeSchedule, + reportBindFailure, + refuseMissingOrganization, + resolveBindingOrganization, +} from './schedule-trigger.js'; import type { FlowTrigger, FlowTriggerBinding, JobServiceSurface, TriggerLogger } from './schedule-trigger.js'; /** @@ -24,8 +29,28 @@ export interface TimeRelativeDataEngine { where?: Record; fields?: string[]; limit?: number; - /** Elevated context — a background sweep must see all rows, not RLS-scoped ones. */ - context?: { isSystem?: boolean }; + /** + * The sweep's execution context. Two INDEPENDENT axes, and this + * sweep sets both (#16659): + * + * - `isSystem` is AUTHORIZATION — a background sweep must see + * every row the organization holds, not the RLS-scoped subset + * some absent user would see. + * - `tenantId` is TENANCY — which organization those rows belong + * to. The engine turns it into `DriverOptions.tenantId` and the + * driver scopes the read. + * + * The engine's own contract keeps them apart in as many words + * (`Engine.buildDriverOptions`: *"System / isSystem callers may + * still cross tenants by clearing `tenantId` themselves"*), so + * elevating a sweep has never implied unscoping it — the previous + * shape simply never passed the second one. Both members are + * `ExecutionContext` keys the engine's `find` already accepts + * (`EngineQueryOptions.context` is `ExecutionContextSchema.partial()`), + * so naming `tenantId` here widens no contract; it declares the + * slice of one this trigger uses. + */ + context?: { isSystem?: boolean; tenantId?: string }; }, ): Promise> | undefined>; /** @@ -164,6 +189,37 @@ export function buildWindowWhere(desc: TimeRelativeDescriptor, window: DateWindo }; } +/** + * [#16659] Why the engine will DROP this sweep's tenant scope for `schema`, or + * `null` when it will apply it. + * + * `Engine.buildDriverOptions` scopes a read by `context.tenantId` unless the + * object opts out, and it documents exactly two opt-outs: `tenancy.enabled: + * false` (ADR-0066 — a platform-global catalog, whose NULL-organization rows + * would vanish under a scope) and `external != null` (ADR-0015 — a federated + * object whose table belongs to a remote database, where the platform has no + * ground to guess a tenant column onto someone else's schema). + * + * ⛔ This is NOT a copy of that predicate for the sweep to act on — the sweep + * passes `tenantId` either way and lets the engine decide. It exists so the + * bind line can SAY that a declared organization is inert for this object, + * which is the one case where the ruling's containment is not achievable and + * the flow's declaration would otherwise imply it is. Read from the schema the + * engine's own `getObject` hands back; an unrecognised shape answers `null` + * (say nothing) rather than guessing. + * + * Module-private on purpose: its only consumer is the bind line below, and this + * package's barrel already states the rule that an export with no consumer + * outside its own package does not belong in it. + */ +function organizationScopeIsInertFor(schema: unknown): string | null { + if (!schema || typeof schema !== 'object') return null; + const s = schema as { tenancy?: { enabled?: unknown } | null; external?: unknown }; + if (s.tenancy?.enabled === false) return 'declares `tenancy: { enabled: false }` (ADR-0066, platform-global)'; + if (s.external != null) return 'is a federated object (ADR-0015 `external`), whose table the remote database owns'; + return null; +} + function errMessage(err: unknown): string { return (err as Error)?.message ?? String(err); } @@ -239,6 +295,28 @@ export class TimeRelativeTrigger implements FlowTrigger { } const desc = parsed.data; + // [#16659] A time-relative sweep launches from a clock, exactly as a + // plain schedule flow does, so it owes the same declaration and takes + // the same refusal. It is NOT the weaker case for carrying an + // organization, it is the stronger one: the sweep runs ELEVATED + // (`isSystem`, deliberately — a background sweep must see all rows + // rather than RLS-scoped ones), so the declaration is the only thing + // that keeps its SELECTION inside one organization. Without it the + // sweep would match rows in every tenant and then launch runs able to + // write into none of them; with it the same value bounds the query and + // the run (see `sweep`'s `organization` parameter). + const organization = resolveBindingOrganization(binding); + if (organization === null) { + // Drop any prior sweep FIRST: a hot re-publish that removes the key + // must not leave the previous, still-armed job sweeping org-less + // behind an error saying it was refused. The call below throws, so + // the engine's catch records the refusal instead of marking this + // flow bound — see `refuseMissingOrganization`'s header for why a + // logged-and-returned refusal is invisible to every audit surface. + this.stop(binding.flowName); + refuseMissingOrganization(this.logger, 'time-relative', binding.flowName, binding); + } + // Cadence: the flow's start-node schedule descriptor, or a daily default. // A daily sweep is the whole point (evaluate the window every day so a // threshold day is never missed), so an omitted schedule means "daily", @@ -270,6 +348,30 @@ export class TimeRelativeTrigger implements FlowTrigger { `[time-relative] flow '${binding.flowName}' targets unknown object '${desc.object}' — the sweep is bound but will match nothing until that object is registered. ` + `Object names match exactly; check config.timeRelative.object.`, ); + } else { + const inertBecause = organizationScopeIsInertFor(known); + if (inertBecause) { + // [#16659] ⛔ A DISCLOSURE, never a narrowing. The sweep + // passes `context.tenantId` unconditionally and the ENGINE + // decides whether it applies; this branch re-reads the two + // declarations the engine documents as its exemptions + // (`tenancy.enabled: false`, ADR-0066; `external`, + // ADR-0015) purely so the operator is told when their + // declaration cannot narrow anything. Nothing here changes + // which rows come back, so if this predicate ever drifts + // from the engine's, the cost is a wrong WARNING — never a + // wrong row. That is the only reason a second reading of + // tenancy is tolerable in a trigger at all. + // + // Saying it matters because the quiet direction here is the + // dangerous one: the sweep keeps selecting across every + // organization, exactly as it did before this card, while + // the flow's `organization` line makes it LOOK contained. + this.logger.warn( + `[time-relative] flow '${binding.flowName}' sweeps '${desc.object}', which ${inertBecause} — the engine applies no tenant scope to such an object, so the declared organization does NOT narrow this sweep: it still selects rows in every organization, while each run it launches acts as the declared one. ` + + `If '${desc.object}' really is per-organization data, that declaration on the OBJECT is what to fix.`, + ); + } } } @@ -282,11 +384,22 @@ export class TimeRelativeTrigger implements FlowTrigger { const handler: JobHandler = async () => { try { - await this.sweep(binding.flowName, desc, maxRecords, callback); + await this.sweep(binding.flowName, desc, maxRecords, organization, callback); } catch (err) { // Error isolation: a sweep failure must not crash the job // runner / ticker. Log and swallow. - this.logger.warn( + // + // [#16659] At `error` when the logger has one, for the reason + // {@link TriggerLogger.error} already states: the CLI's + // boot-quiet window swallows stdout, so a `warn` here can be + // the whole of what a broken sweep says and still be invisible. + // Since the query became organization-scoped, "this sweep can + // no longer see anything" is a REACHABLE state — a store that + // cannot honour the scope refuses the call rather than + // answering it unscoped — and a sweep that selects nothing for + // a structural reason must be as loud as one that crashed. + const log = this.logger.error?.bind(this.logger) ?? this.logger.warn.bind(this.logger); + log( `[time-relative] flow '${binding.flowName}' sweep failed: ${errMessage(err)}`, ); } @@ -300,10 +413,15 @@ export class TimeRelativeTrigger implements FlowTrigger { const mode = desc.offsetDays ? `offsets [${desc.offsetDays.join(', ')}]d` : `within ${desc.withinDays}d`; + // [#16659] The organization is on the BIND line, not only in + // the refusal: it is now the sweep's selection scope as well as + // the run's identity, so "which rows can this flow ever see" is + // answerable from the boot log instead of from the metadata. this.logger.info( `[time-relative] bound flow '${binding.flowName}' → sweep '${desc.object}.${desc.dateField}' ${mode} on ${schedule.type}` + (schedule.expression ? ` '${schedule.expression}'` : '') + - (schedule.intervalMs ? ` every ${schedule.intervalMs}ms` : ''), + (schedule.intervalMs ? ` every ${schedule.intervalMs}ms` : '') + + ` as organization '${organization}'`, ); }) .catch((err) => { @@ -321,6 +439,20 @@ export class TimeRelativeTrigger implements FlowTrigger { flowName: string, desc: TimeRelativeDescriptor, maxRecords: number, + /** + * [#16659] The declared organization. It bounds this sweep TWICE, and + * both halves are load-bearing: + * + * 1. SELECTION — it goes onto the `find` context as `tenantId`, so the + * rows this sweep can match are the declared organization's. Without + * it the sweep is a cross-organization scheduled task whatever the + * run is stamped with. + * 2. IDENTITY — every run launched from a matched row executes as it. + * + * Required, not optional: `start()` refuses the binding without one, so + * a sweep can never be reached with nothing to pass. + */ + organization: string, callback: (ctx: AutomationContext) => Promise, ): Promise { const engine = this.getDataEngine(); @@ -342,7 +474,41 @@ export class TimeRelativeTrigger implements FlowTrigger { (await engine.find(desc.object, { where, limit: maxRecords, - context: { isSystem: true }, + // [#16659] SELECTION is scoped to the declared organization, + // not just the run that follows it. + // + // `isSystem` alone was the whole context here, and it made + // this sweep a cross-organization scheduled task — the thing + // the ruling forbids — with the declaration papering over + // it: a sweep declared for A still MATCHED rows in B, then + // launched a run stamped A about B's record. Downstream that + // is worse than the original defect, not better: the run's + // `update_record` matches nothing (silently, because the run + // is scoped to A), `notify` posts into A's inbox about B's + // record, and the history row is stamped from the SUBJECT, + // so it lands under B. One run, three organizations' + // opinions about who it belonged to. + // + // ⛔ Not a hand-built `organization_id` predicate on + // `where`. That would be a SECOND implementation of tenancy + // living in a trigger: it would hardcode a column name the + // object is free to rename (`tenancy.tenantField`), select + // NOTHING on a platform-global object that carries no such + // column, break a federated object outright, and — worst — + // read as a scoped query to a driver that never learned the + // caller wanted scoping, so a driver with no isolation would + // answer it silently instead of refusing it. The platform + // already owns this: `Engine.buildDriverOptions` turns + // `context.tenantId` into `DriverOptions.tenantId`, drops it + // for the two postures where it must not apply + // (`tenancy.enabled: false`, ADR-0066; federated, ADR-0015), + // and every driver that CAN isolate then scopes, while + // `driver-memory` — which cannot — refuses the call by name + // (#16589). Refusal is the correct answer for a sweep that + // is required to stay inside one organization and is talking + // to a store that cannot keep it there, and it arrives as a + // logged sweep failure rather than as silence. + context: { isSystem: true, tenantId: organization }, })) ?? []; for (const row of rows) { const id = (row as { id?: unknown }).id; @@ -385,6 +551,15 @@ export class TimeRelativeTrigger implements FlowTrigger { record, object: desc.object, event: 'time_relative', + // [#16659] The declared acting organization — the same key + // a record-change run inherits from its triggering session, + // and the one `notify-node.ts` and the run-history writer + // already read. ⛔ Never derived from the swept RECORD's + // own `organization_id`: the sweep runs elevated and can + // match rows in any tenant, so keying on the row would let + // one flow write into organizations it never declared — + // the cross-organization scheduled task the ruling forbids. + tenantId: organization, // Expose the record as params too, so flows with named `isInput` // variables matching record fields get them seeded (parity with // the record-change trigger). @@ -405,7 +580,7 @@ export class TimeRelativeTrigger implements FlowTrigger { } this.logger.debug?.( - `[time-relative] flow '${flowName}' swept '${desc.object}': ${matched.length} matched, ${launched} launched, ${deduped} already dispatched, ${failed} failed`, + `[time-relative] flow '${flowName}' swept '${desc.object}' as organization '${organization}': ${matched.length} matched, ${launched} launched, ${deduped} already dispatched, ${failed} failed`, ); } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index b98543fd58..bb035dd835 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -2077,6 +2077,9 @@ importers: '@objectstack/trigger-record-change': specifier: workspace:* version: link:../../triggers/trigger-record-change + '@objectstack/trigger-schedule': + specifier: workspace:* + version: link:../../triggers/trigger-schedule '@objectstack/types': specifier: workspace:* version: link:../../types