diff --git a/.changeset/schedule-flow-dispatch-claim-ledger.md b/.changeset/schedule-flow-dispatch-claim-ledger.md new file mode 100644 index 0000000000..9e30c5c01f --- /dev/null +++ b/.changeset/schedule-flow-dispatch-claim-ledger.md @@ -0,0 +1,31 @@ +--- +"@objectstack/trigger-schedule": minor +"@objectstack/service-automation": minor +"@objectstack/service-job": minor +--- + +A scheduled (cron) flow is now delivered once per tick window, and replaying a window that was already delivered is refused instead of silently sent again. + +A `time_relative` flow has taken a persisted dispatch claim per `(flow, window, record)` since #10220, so per-record once-only delivery is free for it. A `schedule` flow runs once per tick with no record and had no claim surface at all, so "this batch already went out" fell back to whatever each app remembered for itself. A scheduled digest that was replayed by an operator, or whose process restarted inside its window, delivered twice. + +Scheduled flows now claim `(flow, tick-window)` in the same `sys_flow_dispatch` ledger, and settle that claim with what the run turned into: + +- **A second fire inside one window does nothing.** The window key is a pure function of the schedule descriptor and the clock — the previous occurrence of the very same cron expression in the very same timezone, computed with the same library the job adapter schedules with — so a restart inside the window computes the same key and hits the same claim. +- **`IJobService.replay()` refuses a delivered window**, with the ADR-0112 envelope its contract declares: `code: 'RESOURCE_CONFLICT'`, `status: 409`, and a message naming the window and the claim that refused it. The promise rejects — an operator who presses replay and sees nothing happen is exactly the outcome this replaces. +- **`replay(name, data, { force: true })` sends anyway.** The duplicate is the operator's, taken knowingly. +- **A window whose claim is absent, failed or unsettled re-runs** on a plain `replay()`, with no force needed. A job that takes no claim at all — every job that is not a scheduled flow — is the absent row and behaves exactly as before. +- **`succeeded` is absorbing.** A replay that repairs a failed window records `succeeded`, so the next unforced replay is refused. A *forced* replay that throws leaves the window recorded delivered rather than rewriting it to `failed` — otherwise a failed re-send would silently reopen the unforced re-delivery door. An operator whose forced replay failed forces again. +- **A `once` schedule now has a tick window too** — the single instant it is due, which is one window for the job's whole life. The visible consequence is on replay: an operator who replays a one-shot job *before* its due instant claims that single window, so the real fire then finds the claim and does nothing. Previously both ran. + +The error-isolation `catch` that keeps a throwing flow from crashing the ticker is unchanged and still swallows. What it no longer does is leave the run indistinguishable from a delivered one: the throw settles the window's claim as `failed`, so a replay repairs it. + +`sys_flow_dispatch` gains two optional columns, `outcome` and `settled_at`. Rows written before this release read as unsettled, which reads as not delivered — the safe direction, since a replay of one re-runs rather than being refused. Only `schedule:` claims are ever settled; a `time_relative` sweep's rows stay `null` by design. + +⚠️ **If you manage this table's DDL out of band** — anything other than letting the platform sync `sys_flow_dispatch` from its object definition — add `outcome` (text) and `settled_at` (datetime) yourself before upgrading. Without them every `settle()` throws against the driver. Dispatch dedup still works and no flow fails (the settle is best-effort and logged), but no claim ever records an outcome, so the replay refusal never fires and this release's headline change is silently absent. + +Interface changes for hosts that implement the ledger themselves: + +- `FlowDispatchStore` gains **optional** `settle()` and `read()`. A store without them still deduplicates; it announces once that the refusal cannot fire. +- `FlowDispatchStoreEngine` — the narrow ObjectQL slice the bundled store demands — now **requires** `update` alongside `find` and `insert`. A custom engine adapter typed against it must add the method. +- New exported types: `FlowDispatchClaim` and `FlowDispatchOutcome` from `@objectstack/service-automation`; `ReplayGuard` and `ReplayGuardDecision` from `@objectstack/service-job` (the parameter type of `DbJobAdapter.setReplayGuard`, exported so it can be named); `ScheduleDispatchLedger`, `ScheduleDispatchClaim`, `ScheduleDispatchOutcome`, `ReplayGuard` and `ReplayGuardDecision` from `@objectstack/trigger-schedule`. +- `croner` moves from a devDependency to a **dependency** of `@objectstack/trigger-schedule`, which now imports it at runtime to compute the cron tick window. It is already a runtime dependency of `@objectstack/service-job` at the same range, so the platform's dependency set does not grow. diff --git a/content/docs/permissions/tenant-audit-census.mdx b/content/docs/permissions/tenant-audit-census.mdx index 6ce4bca15b..484d541ffe 100644 --- a/content/docs/permissions/tenant-audit-census.mdx +++ b/content/docs/permissions/tenant-audit-census.mdx @@ -98,7 +98,7 @@ are reported as `undecidable` rather than assumed either way. The same holds twice over for the context. An options argument spelled as a literal can be read; one spelled `options`, `{ ...opts }`, or handed through a -forwarding shim cannot, and **67 of the 222 sites are spelled that way**. A +forwarding shim cannot, and **67 of the 223 sites are spelled that way**. A context resolved from an inline literal or a local `const` can be tested for `isSystem`; one arriving from a helper call cannot. @@ -147,10 +147,10 @@ reproduce them. Where it disagrees, it disagrees on the page: | carried figure | where it survives | this census | | :--- | :--- | ---: | -| 175 write call sites | quoted in the merged changeset | **222** | +| 175 write call sites | quoted in the merged changeset | **223** | | 24 carrying no tenant context | quoted in the merged changeset | **9** provable and tenancy-enabled; **32** more whose options argument is unreadable | -| 127 of 175 statically decidable, 48 runtime-parameter-name sites | restated on the `isSystem`-scoping card | **148 of 222** decidable, **74** undecidable | -| 135 (77%) silenced by the `isSystem` guard before the posture gate | the lost issue body — **no surviving corroboration** | **not reproduced**: 104 decidably elevated, 0 decidably not, 101 undecidable | +| 127 of 175 statically decidable, 48 runtime-parameter-name sites | restated on the `isSystem`-scoping card | **149 of 223** decidable, **74** undecidable | +| 135 (77%) silenced by the `isSystem` guard before the posture gate | the lost issue body — **no surviving corroboration** | **not reproduced**: 105 decidably elevated, 0 decidably not, 101 undecidable | | 141 and 132, two independent re-derivations | the card that filed this work | — | **The differences are not reconciled, and deliberately so.** The old census's @@ -161,17 +161,17 @@ at any commit. Two structural facts do plausibly widen this reading against any hand or regex one, and both are counted in the generated tables below: the 45 sites reached -through an erased (`any`) receiver, and the 39 that name their object through a +through an erased (`any`) receiver, and the 40 that name their object through a `const` rather than inline. An instrument that read either the way a person does would report a smaller number and would not say so. The fourth row is the one worth flagging to anyone citing it. **The 135 / 77% figure has no surviving corroboration anywhere in the tree.** This census reads -104 of 222 (47%) as decidably elevated, with 101 more whose elevation is a +105 of 223 (47%) as decidably elevated, with 101 more whose elevation is a run-time fact — so the claim is neither confirmed nor refuted, and the honest answer is that a static reading cannot settle it. -⇒ **Cite `9 / 222`, and say what it is**: the sites whose options argument was +⇒ **Cite `9 / 223`, and say what it is**: the sites whose options argument was READ and holds no tenant context, against a decidably tenancy-enabled object. That is the control's provable yield surface. ⛔ Do not cite it as "the sites without tenant context" — **32 further sites** have an options argument this @@ -183,29 +183,29 @@ cannot read, and they are neither in nor out. | what | count | | :--- | ---: | -| write call sites on the application surface | **222** | -| …whose object name is statically decidable | 148 | +| write call sites on the application surface | **223** | +| …whose object name is statically decidable | 149 | | …whose object name is chosen at run time | 74 | -| …against an object with tenancy ENABLED | 148 | +| …against an object with tenancy ENABLED | 149 | | …against an object that declares tenancy off | 0 | -| threading a tenant context | 138 | +| threading a tenant context | 139 | | PROVABLY carrying none (options read, no context key) | **17** | | …of those, against a decidably tenancy-enabled object | **9** | | options argument UNREADABLE — may or may not carry one | 67 | | …of those, against a decidably tenancy-enabled object | 32 | -| threading a decidably ELEVATED (`isSystem`) context | 104 | +| threading a decidably ELEVATED (`isSystem`) context | 105 | | threading a context that is decidably NOT elevated | 0 | | threading a context whose elevation is a run-time fact | 101 | | how the instrument reached the site | count | | :--- | ---: | -| receiver carried a readable engine type | 177 | +| receiver carried a readable engine type | 178 | | receiver erased, placed by the object NAME | 19 | | receiver erased, placed by an `object: string` PARAMETER | 15 | | receiver erased, placed by an `UNTYPED_RECEIVERS` row | 11 | | object name spelled inline | 109 | -| object name spelled through a `const` | 39 | +| object name spelled through a `const` | 40 | | object name is an `object: string` parameter | 19 | | object name is some other run-time expression | 55 | @@ -224,13 +224,13 @@ holds still. They are required to be HERE and to say WHEN they were true; their values are not compared. The reasoning, and the measurement behind it, are in `scripts/check-tenant-audit-census.mjs`. -Measured on 2026-09-05 at `63a1a410e`. +Measured on 2026-09-07 at `9cefca9a3`. | corpus scale (not enforced) | count | | :--- | ---: | -| tracked non-test sources scanned | 548 | -| engine-shaped types recognised | 58 | +| tracked non-test sources scanned | 557 | +| engine-shaped types recognised | 59 | | declared objects in the registry | 298 | -| same-named calls subtracted as non-engine | 134 | +| same-named calls subtracted as non-engine | 137 | {/* END GENERATED: tenant-audit-census */} diff --git a/docs/audits/2026-08-tenant-audit-write-call-sites.counts.md b/docs/audits/2026-08-tenant-audit-write-call-sites.counts.md index 619328462f..b2dacec989 100644 --- a/docs/audits/2026-08-tenant-audit-write-call-sites.counts.md +++ b/docs/audits/2026-08-tenant-audit-write-call-sites.counts.md @@ -29,17 +29,17 @@ silent, and `node scripts/tenant-audit-census.mjs --write` is the resolution. | Measure | Value | |---|---:| -| Write call sites | 222 | -| Object name statically decidable | 148 | +| Write call sites | 223 | +| Object name statically decidable | 149 | | Object name chosen at run time | 74 | -| Against a tenancy-enabled object | 148 | +| Against a tenancy-enabled object | 149 | | Against an object declaring tenancy off | 0 | -| Threading a tenant context | 138 | +| Threading a tenant context | 139 | | Provably carrying none | 17 | | …and decidably tenancy-enabled | 9 | | Options argument unreadable | 67 | | …and decidably tenancy-enabled | 32 | -| Threading a decidably elevated context | 104 | +| Threading a decidably elevated context | 105 | | Threading a decidably non-elevated context | 0 | | Threading a context of undecidable elevation | 101 | @@ -52,14 +52,14 @@ holds still. They are required to be HERE and to say WHEN they were true; their values are not compared. The reasoning, and the measurement behind it, are in `scripts/check-tenant-audit-census.mjs`. -Measured on 2026-09-05 at `63a1a410e`. +Measured on 2026-09-07 at `9cefca9a3`. | corpus scale (not enforced) | count | | :--- | ---: | -| tracked non-test sources scanned | 548 | -| engine-shaped types recognised | 58 | +| tracked non-test sources scanned | 557 | +| engine-shaped types recognised | 59 | | declared objects in the registry | 298 | -| same-named calls subtracted as non-engine | 134 | +| same-named calls subtracted as non-engine | 137 | ## Every site @@ -160,6 +160,7 @@ Measured on 2026-09-05 at `63a1a410e`. | `packages/services/service-automation/src/builtin/crud-nodes.ts` | `insert` | `objectName` | undecidable | context, elevation undecidable | 1 | | `packages/services/service-automation/src/builtin/crud-nodes.ts` | `update` | `objectName` | undecidable | context, elevation undecidable | 1 | | `packages/services/service-automation/src/flow-dispatch-store.ts` | `insert` | `sys_flow_dispatch` | enabled | elevated | 1 | +| `packages/services/service-automation/src/flow-dispatch-store.ts` | `update` | `sys_flow_dispatch` | enabled | elevated | 1 | | `packages/services/service-automation/src/suspended-run-store.ts` | `delete` | `sys_automation_run` | enabled | elevated | 3 | | `packages/services/service-automation/src/suspended-run-store.ts` | `insert` | `sys_automation_run` | enabled | elevated | 2 | | `packages/services/service-automation/src/suspended-run-store.ts` | `update` | `sys_automation_run` | enabled | elevated | 2 | diff --git a/packages/services/service-automation/src/engine.ts b/packages/services/service-automation/src/engine.ts index 213c3d650f..10ad655e47 100644 --- a/packages/services/service-automation/src/engine.ts +++ b/packages/services/service-automation/src/engine.ts @@ -1565,17 +1565,57 @@ export interface SuspendedRunStore { loadTerminal?(runId: string): Promise; } +/** What a claimed dispatch turned into (#14501). */ +export type FlowDispatchOutcome = 'succeeded' | 'failed'; + /** - * Persisted claim ledger for trigger dispatch idempotency (#10220). + * One row of the dispatch-claim ledger, as a reader sees it (#14501). + * + * `outcome: null` is the honest third state, not a missing value: the claim + * was taken and never settled — either the dispatch is in flight right now, or + * the process that took it died mid-launch, or the row predates #14501. Every + * consumer reads all three of those as **not delivered**, which is the safe + * direction: an unforced `replay()` re-runs them rather than refusing on a + * claim nobody ever settled. + */ +export interface FlowDispatchClaim { + /** The dispatch key — the ledger row's primary id. */ + key: string; + /** When the key was claimed (ISO-8601), or `null` if the store cannot say. */ + claimedAt: string | null; + /** Terminal outcome, or `null` while the claim is unsettled. */ + outcome: FlowDispatchOutcome | null; + /** When the outcome was recorded (ISO-8601), or `null` while unsettled. */ + settledAt: string | null; +} + +/** + * Persisted claim ledger for trigger dispatch idempotency (#10220), extended + * by #14501 to record what each claim turned into. * * `claim(key)` is check-and-record: `true` means the caller now owns this * dispatch key and should launch the flow; `false` means some earlier sweep — * possibly in a previous process lifetime — already dispatched it. Backed by * `sys_flow_dispatch` in production (see `ObjectStoreFlowDispatchStore`), so * dedup survives kernel rebuild. + * + * `settle` and `read` are OPTIONAL, and their absence is a DECLARED + * degradation rather than a silent one — the same posture + * {@link SuspendedRunStore.claimSuspension} takes. A store that predates + * #14501 implements neither: every claim then reads as unsettled, and the + * `replay()` refusal the #14501 ruling adds simply never fires. Dedup is + * unchanged; only the refusal is lost, and the engine says so once. */ export interface FlowDispatchStore { claim(key: string): Promise; + /** + * Record the outcome of an already-claimed key. A no-op for a key that was + * never claimed — settling is a transition on an existing row, never a way + * to create one. + */ + settle?(key: string, outcome: FlowDispatchOutcome): Promise; + /** The claim for `key`, or `null` when the key was never claimed. */ + read?(key: string): Promise; } /** @@ -1933,11 +1973,15 @@ export class AutomationEngine implements IAutomationService { */ private flowDispatchStore: FlowDispatchStore | null = null; /** - * In-process dispatch-claim fallback: key → claim time (epoch ms). Used - * when no persisted ledger is attached, and per-key when the ledger - * errors. Entries expire after {@link IN_PROCESS_DISPATCH_CLAIM_TTL_MS}. + * In-process dispatch-claim fallback: key → claim time (epoch ms) and the + * outcome it settled to (#14501), `null` while unsettled. Used when no + * persisted ledger is attached, and per-key when the ledger errors. + * Entries expire after {@link IN_PROCESS_DISPATCH_CLAIM_TTL_MS}. */ - private readonly inProcessDispatchClaims = new Map(); + private readonly inProcessDispatchClaims = new Map< + string, + { at: number; outcome: FlowDispatchOutcome | null } + >(); /** * Whether this engine has already said its dispatch dedup is in-process * only (#10220). Once per instance: a silent fallback hides a permanently @@ -1953,6 +1997,13 @@ export class AutomationEngine implements IAutomationService { * per resume is log spam. */ private advanceClaimDegradationWarned = false; + /** + * [#14501] Whether this engine has already said that its attached ledger + * records no claim OUTCOMES — same once-per-instance reason as the two + * fields above: a silent fallback hides a permanently weakened guarantee, + * repeating it per replay is log spam. + */ + private dispatchOutcomeDegradationWarned = false; constructor(logger: Logger, store?: SuspendedRunStore, options?: AutomationEngineOptions) { this.logger = logger; @@ -2027,15 +2078,111 @@ export class AutomationEngine implements IAutomationService { /** In-process half of {@link claim}: TTL-pruned check-and-record. */ private claimInProcess(key: string): boolean { const now = Date.now(); - const cutoff = now - IN_PROCESS_DISPATCH_CLAIM_TTL_MS; - for (const [k, t] of this.inProcessDispatchClaims) { - if (t < cutoff) this.inProcessDispatchClaims.delete(k); - } + this.pruneInProcessDispatchClaims(now); if (this.inProcessDispatchClaims.has(key)) return false; - this.inProcessDispatchClaims.set(key, now); + this.inProcessDispatchClaims.set(key, { at: now, outcome: null }); return true; } + private pruneInProcessDispatchClaims(now: number): void { + const cutoff = now - IN_PROCESS_DISPATCH_CLAIM_TTL_MS; + for (const [k, rec] of this.inProcessDispatchClaims) { + if (rec.at < cutoff) this.inProcessDispatchClaims.delete(k); + } + } + + /** + * Record what a claimed dispatch turned into (#14501) — the second half of + * {@link claim}, and the half `IJobService.replay()` reads to decide + * whether a window was DELIVERED or merely attempted. + * + * Best-effort by construction, in both directions. A settle that throws is + * logged and swallowed: the dispatch already happened, and turning a + * delivered flow into a thrown one to report a bookkeeping failure is + * strictly worse than leaving the row unsettled — an unsettled row reads + * as "not delivered", so the only cost is that a later replay is allowed + * through instead of refused. A ledger without `settle()` is the same + * cost, said once by {@link readDispatch}. + * + * ⚠️ Not every call writes. `succeeded` is ABSORBING: a claim that already + * recorded success stays `succeeded` even if a later forced replay throws, + * because rewriting it would silently reopen the unforced re-delivery door + * the #14501 ruling closed. The store enforces that (`isSettleAllowed`) and + * refuses by not writing, never by throwing — a refusal is the invariant + * working, not a failure to report. + */ + async settleDispatch(key: string, outcome: FlowDispatchOutcome): Promise { + const store = this.flowDispatchStore; + if (store && typeof store.settle === 'function') { + try { + await store.settle(key, outcome); + return; + } catch (err) { + this.logger.warn( + `[automation] flow-dispatch settle '${key}' → '${outcome}' failed against the persisted ledger — ` + + `the claim stays UNSETTLED, which reads as 'not delivered': a later replay of this window is ` + + `allowed through rather than refused. The store failure is in this record's meta.`, + describeThrownForLog(err), + ); + } + } + const existing = this.inProcessDispatchClaims.get(key); + // Same write rule as the persisted ledger: `succeeded` is absorbing, so + // the fallback cannot reopen a re-delivery door the durable path keeps + // shut. Kept here rather than imported so the engine stays free of a + // dependency on the store module it merely drives. + if (existing && !(existing.outcome === 'succeeded' && outcome === 'failed')) { + this.inProcessDispatchClaims.set(key, { ...existing, outcome }); + } + } + + /** + * Read one dispatch claim (#14501): `null` when the key was never claimed. + * + * A ledger that cannot answer — none attached, one predating #14501, or a + * read that throws — reports the key as UNCLAIMED rather than guessing. + * That is the availability-over-strict-once direction this ledger already + * takes everywhere else: the caller re-runs, it never refuses on a reading + * it could not make. + */ + async readDispatch(key: string): Promise { + const store = this.flowDispatchStore; + if (store && typeof store.read === 'function') { + try { + return await store.read(key); + } catch (err) { + this.logger.warn( + `[automation] flow-dispatch read '${key}' failed against the persisted ledger — ` + + `reporting the key as UNCLAIMED (availability over strict-once: a replay proceeds rather ` + + `than being refused on a reading we could not make). The store failure is in this record's meta.`, + describeThrownForLog(err), + ); + return null; + } + } + if (store && !this.dispatchOutcomeDegradationWarned) { + this.dispatchOutcomeDegradationWarned = true; + // The ledger predates the outcome half of the claim contract. + this.logger.warn( + '[automation] the attached flow-dispatch ledger has no read() — this message describes the ' + + 'ledger that has no read()/settle() AT ALL, where dispatch claims are still deduplicated ' + + 'and the in-process fallback still records outcomes and still refuses WITHIN one process ' + + 'lifetime, but nothing outlives a restart. A ledger that has settle() but no read() is ' + + 'weaker still: outcomes are written durably and never read back, so no replay is ever ' + + 'refused, in this lifetime or any other.', + ); + } + this.pruneInProcessDispatchClaims(Date.now()); + const rec = this.inProcessDispatchClaims.get(key); + if (!rec) return null; + return { + key, + claimedAt: new Date(rec.at).toISOString(), + outcome: rec.outcome, + settledAt: null, + }; + } + /** * Generate a process-unique run id. Includes a random component so ids do * not collide with runs persisted by a previous process lifetime (a plain diff --git a/packages/services/service-automation/src/flow-dispatch-store.ts b/packages/services/service-automation/src/flow-dispatch-store.ts index 6f0873701f..7ab87d1ead 100644 --- a/packages/services/service-automation/src/flow-dispatch-store.ts +++ b/packages/services/service-automation/src/flow-dispatch-store.ts @@ -1,50 +1,123 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. -import type { FlowDispatchStore } from './engine.js'; +import type { FlowDispatchStore, FlowDispatchOutcome, FlowDispatchClaim } from './engine.js'; /** - * Durable claim ledger for trigger dispatch idempotency (#10220). + * Durable claim ledger for trigger dispatch idempotency (#10220), and the + * record of what each claim turned into (#14501). * - * A {@link FlowDispatchStore} answers exactly one question, atomically enough - * for a sweep: "has this dispatch key been claimed before?" — recording the - * claim in the same call. The time-relative trigger computes a key from the - * matched window's identity and calls `claim()` before launching the flow; a - * `false` means some earlier sweep (possibly in a previous process lifetime) - * already dispatched this exact (flow, record, window). + * A {@link FlowDispatchStore} answers one question atomically enough for a + * sweep — "has this dispatch key been claimed before?" — recording the claim in + * the same call. The time-relative trigger computes a key from the matched + * window's identity and calls `claim()` before launching; a `false` means some + * earlier sweep (possibly in a previous process lifetime) already dispatched + * this exact (flow, record, window). The schedule trigger does the same with a + * `(flow, tick-window)` key. + * + * `claim()` alone cannot answer the second question the #14501 ruling asks — + * *did that dispatch land?* — because a row's mere existence says only that + * someone took the key. So a claim is settled after the launch returns + * ({@link FlowDispatchStore.settle}) and read back before an operator replay + * ({@link FlowDispatchStore.read}). * * Two implementations: - * - {@link InMemoryFlowDispatchStore} — a Set (tests / explicit + * - {@link InMemoryFlowDispatchStore} — a Map (tests / explicit * `suspendedRunStore: 'memory'` hosts). Sharable across two engine * instances to simulate a kernel rebuild against one surviving ledger. * - {@link ObjectStoreFlowDispatchStore} — persists to `sys_flow_dispatch` * via the ObjectQL engine, so dedup survives kernel rebuild (the #10220 - * fix requirement the in-process Set cannot meet). + * fix requirement the in-process Map cannot meet). */ const TABLE = 'sys_flow_dispatch'; const SYSTEM_CTX = { isSystem: true, positions: [], permissions: [] } as const; /** - * The exact ObjectQL slice `claim()` needs: a keyed read and an insert. - * Narrower than `SuspendedRunStoreEngine` on purpose — the ledger never - * updates or deletes (rows are immutable claims; the platform Reaper owns - * deletion via the object's declared retention), and demanding only what is - * used keeps every test double honest about that. + * The exact ObjectQL slice this store needs: a keyed read, an insert, and — + * since #14501 — a keyed update. + * + * Narrower than `SuspendedRunStoreEngine` on purpose, and the narrowness is + * load-bearing: the ledger still never DELETES (the platform Reaper owns + * deletion via the object's declared retention), and `update` only ever writes + * `outcome`/`settled_at`, never a claim column. Two of the three transitions + * those columns admit reach it — `null → succeeded` and `null → failed` from an + * ordinary run, and `failed → succeeded` when a replay repairs a window. The + * third, `succeeded → failed`, is REFUSED by {@link ObjectStoreFlowDispatchStore.settle} + * and never reaches the engine at all. A test double that implements only what + * is used stays honest about that. */ export interface FlowDispatchStoreEngine { find(object: string, options?: any): Promise; insert(object: string, data: any, options?: any): Promise; + update(object: string, data: any, options?: any): Promise; +} + +/** + * The ledger's write rule for `outcome`, in one predicate — **`succeeded` is + * absorbing** (#14501, seat ruling in the contract review of this change). + * + * Three transitions are allowed and one is refused: + * + * | from | to | | + * |:---|:---|:---| + * | `null` | `succeeded` / `failed` | an ordinary run settling its own claim | + * | `failed` | `succeeded` | REQUIRED — a replay repaired the window, and the next unforced replay must now be refused | + * | `succeeded` | `failed` | **refused** | + * + * The refusal is not fussiness about monotonicity. The ruling guarantees that + * a window whose claim succeeded is refused, and "latest attempt wins" erodes + * exactly that: a FORCED replay that throws would rewrite a delivered window + * to `failed` and silently reopen the *unforced* re-delivery door — the + * duplicate-delivery harm this whole card exists to close. An operator whose + * forced replay failed has to force again, which is louder and safer than a + * door that reopens itself. + * + * `succeeded → succeeded` and `failed → failed` are allowed and simply refresh + * `settled_at`. + */ +export function isSettleAllowed( + current: FlowDispatchOutcome | null, + next: FlowDispatchOutcome, +): boolean { + return !(current === 'succeeded' && next === 'failed'); +} + +/** Shape both stores write and read back — see {@link FlowDispatchClaim}. */ +function toClaim(row: Record): FlowDispatchClaim { + const outcome = row.outcome === 'succeeded' || row.outcome === 'failed' ? row.outcome : null; + return { + key: String(row.id), + claimedAt: typeof row.dispatched_at === 'string' ? row.dispatched_at : null, + outcome, + settledAt: typeof row.settled_at === 'string' ? row.settled_at : null, + }; } /** In-memory {@link FlowDispatchStore} — process-lifetime dedup only. */ export class InMemoryFlowDispatchStore implements FlowDispatchStore { - private readonly keys = new Set(); + private readonly claims = new Map(); async claim(key: string): Promise { - if (this.keys.has(key)) return false; - this.keys.add(key); + if (this.claims.has(key)) return false; + this.claims.set(key, { + key, + claimedAt: new Date().toISOString(), + outcome: null, + settledAt: null, + }); return true; } + + async settle(key: string, outcome: FlowDispatchOutcome): Promise { + const existing = this.claims.get(key); + if (!existing) return; + if (!isSettleAllowed(existing.outcome, outcome)) return; + this.claims.set(key, { ...existing, outcome, settledAt: new Date().toISOString() }); + } + + async read(key: string): Promise { + return this.claims.get(key) ?? null; + } } /** @@ -85,6 +158,57 @@ export class ObjectStoreFlowDispatchStore implements FlowDispatchStore { } } + /** + * Record what the claimed dispatch turned into (#14501). + * + * Called after the launch returns — a settle failure must never turn a + * delivered dispatch into a thrown one, so callers treat this as + * best-effort; the cost of losing it is a row stuck at `outcome: null`, + * which reads as "not delivered" and lets an operator replay through. + * + * Reads the row first because {@link isSettleAllowed} needs the current + * outcome: `succeeded` is absorbing, and a downgrade is a silent no-op here + * rather than a throw — refusing to write is the invariant working, not an + * error, and reporting it as one would turn the trigger's honest "could not + * record the outcome" warning into a lie. + * + * ⚠️ **The absorbing rule is enforced read-then-write here, so it is not + * atomic.** Between the read above and the `update` below a concurrent + * settle on the same key can land: a `succeeded` and a `failed` for one + * window finishing at the same instant — a replay and a tick, or two + * replicas — can interleave and leave the row `failed`, which is exactly + * the reopened unforced-replay door the rule exists to close. The reach is + * narrow and the engine's by-id `update` shape cannot express a conditional + * write, so this is named rather than closed. ⛔ Do not read the persisted + * store as giving the guarantee {@link InMemoryFlowDispatchStore} does, + * whose check and write share a turn; closing it needs a conditional update + * the engine does not have yet. + */ + async settle(key: string, outcome: FlowDispatchOutcome): Promise { + const current = await this.read(key); + if (!current) return; // never claimed — settling does not create rows + if (!isSettleAllowed(current.outcome, outcome)) return; + // `update(object, { id, …fields }, options)` — the id rides in the PAYLOAD, + // which is the by-id dispatch shape the ObjectQL engine actually takes. + // ⛔ Not a 4-argument `update(object, id, data, options)`: no engine here + // dispatches on that, and a double loose enough to accept it is exactly + // what `pnpm check:engine-double-contract` exists to catch. + await this.engine.update( + TABLE, + { id: key, outcome, settled_at: new Date().toISOString() }, + { context: SYSTEM_CTX }, + ); + } + + /** The claim row for `key`, or `null` when the key was never claimed. */ + async read(key: string): Promise { + const rows = await this.engine.find(TABLE, { + where: { id: key }, limit: 1, context: SYSTEM_CTX, + }); + const row = Array.isArray(rows) ? rows[0] : undefined; + return row ? toClaim(row) : null; + } + /** * Read the backing table once so a misconfiguration surfaces at BOOT rather * than as a per-claim failure at sweep time. Throws the driver error diff --git a/packages/services/service-automation/src/flow-dispatch.test.ts b/packages/services/service-automation/src/flow-dispatch.test.ts index 23b48af6b6..d80b9b3fcb 100644 --- a/packages/services/service-automation/src/flow-dispatch.test.ts +++ b/packages/services/service-automation/src/flow-dispatch.test.ts @@ -2,10 +2,19 @@ // // Trigger dispatch idempotency (#10220): the persisted `sys_flow_dispatch` // claim ledger and the `AutomationEngine.claim()` surface triggers consume. +// +// #14501 extends the same ledger with the OUTCOME half the maintainer's +// A + a2 ruling requires: a claim is settled after the launch returns, and +// read back before an operator replay. import { describe, it, expect, vi } from 'vitest'; +import { assertEngineUpdateDispatch } from '@objectstack/metadata-core'; import { AutomationEngine } from './engine.js'; -import { InMemoryFlowDispatchStore, ObjectStoreFlowDispatchStore } from './flow-dispatch-store.js'; +import { + InMemoryFlowDispatchStore, + ObjectStoreFlowDispatchStore, + isSettleAllowed, +} from './flow-dispatch-store.js'; import type { FlowDispatchStoreEngine } from './flow-dispatch-store.js'; function testLogger() { @@ -44,6 +53,22 @@ function fakeQl() { rows.set(id, data as Record); return data; }, + async update(_table, data, options) { + // Routed through ObjectQL's OWN dispatch predicate, so this fake + // cannot be looser than the engine it stands in for — the gate is + // `pnpm check:engine-double-contract`, and a fake that accepted a + // shape the engine rejects is how a dead write path ships green. + const dispatch = assertEngineUpdateDispatch(data, options); + if (dispatch.kind !== 'by-id') { + throw new Error(`fake driver: the ledger only ever writes by id, got ${dispatch.kind}`); + } + const id = String(dispatch.id); + const row = rows.get(id); + if (!row) throw new Error(`fake driver: no row ${id}`); + const { id: _ignored, ...fields } = data as Record; + rows.set(id, { ...row, ...fields }); + return rows.get(id); + }, }; return { engine, rows }; } @@ -81,6 +106,10 @@ describe('ObjectStoreFlowDispatchStore', () => { async insert() { throw new Error('UNIQUE constraint failed: sys_flow_dispatch.id'); }, + async update(_t: string, data: any, options?: any) { + assertEngineUpdateDispatch(data, options); + throw new Error('not reached'); + }, }; const store = new ObjectStoreFlowDispatchStore(engine); await expect(store.claim('k1')).resolves.toBe(false); @@ -90,6 +119,10 @@ describe('ObjectStoreFlowDispatchStore', () => { const engine: FlowDispatchStoreEngine = { async find() { return []; }, async insert() { throw new Error('no such table: sys_flow_dispatch'); }, + async update(_t: string, data: any, options?: any) { + assertEngineUpdateDispatch(data, options); + throw new Error('not reached'); + }, }; const store = new ObjectStoreFlowDispatchStore(engine); await expect(store.claim('k1')).rejects.toThrow('no such table'); @@ -149,3 +182,194 @@ describe('AutomationEngine.claim (#10220)', () => { ); }); }); + +describe('the claim OUTCOME half (#14501)', () => { + it('ObjectStoreFlowDispatchStore: claim leaves the row unsettled, settle() writes the terminal outcome', async () => { + const { engine, rows } = fakeQl(); + const store = new ObjectStoreFlowDispatchStore(engine); + + await store.claim('schedule:digest:2026-09-07T01:00:00.000Z'); + await expect(store.read('schedule:digest:2026-09-07T01:00:00.000Z')).resolves.toMatchObject({ + outcome: null, + settledAt: null, + }); + + await store.settle('schedule:digest:2026-09-07T01:00:00.000Z', 'succeeded'); + const claim = await store.read('schedule:digest:2026-09-07T01:00:00.000Z'); + expect(claim?.outcome).toBe('succeeded'); + expect(claim?.settledAt).toEqual(expect.any(String)); + // The claim columns are untouched by the settle — it is a transition, + // not a rewrite. + expect(rows.get('schedule:digest:2026-09-07T01:00:00.000Z')?.dispatched_at).toEqual( + expect.any(String), + ); + }); + + it("read() of a key that was never claimed is null, not a fabricated 'absent' claim", async () => { + const { engine } = fakeQl(); + const store = new ObjectStoreFlowDispatchStore(engine); + await expect(store.read('never-claimed')).resolves.toBeNull(); + }); + + it('InMemoryFlowDispatchStore settles and reads back the same three states', async () => { + const store = new InMemoryFlowDispatchStore(); + await expect(store.read('k')).resolves.toBeNull(); + await store.claim('k'); + await expect(store.read('k')).resolves.toMatchObject({ outcome: null }); + await store.settle('k', 'failed'); + await expect(store.read('k')).resolves.toMatchObject({ outcome: 'failed' }); + }); + + it('settle() of a key that was never claimed does not invent a row', async () => { + const store = new InMemoryFlowDispatchStore(); + await store.settle('ghost', 'succeeded'); + await expect(store.read('ghost')).resolves.toBeNull(); + }); + + it('AutomationEngine.settleDispatch/readDispatch carry the outcome through the ledger', async () => { + const { logger, warn } = testLogger(); + const engine = new AutomationEngine(logger); + engine.setFlowDispatchStore(new InMemoryFlowDispatchStore()); + + await expect(engine.claim('k1')).resolves.toBe(true); + await expect(engine.readDispatch('k1')).resolves.toMatchObject({ outcome: null }); + await engine.settleDispatch('k1', 'succeeded'); + await expect(engine.readDispatch('k1')).resolves.toMatchObject({ outcome: 'succeeded' }); + expect(warn).not.toHaveBeenCalled(); + }); + + it('a ledger predating #14501 (no read/settle) reports every claim unclaimed, warned exactly once', async () => { + const { logger, warn } = testLogger(); + const engine = new AutomationEngine(logger); + // A pre-#14501 store: `claim` only. + engine.setFlowDispatchStore({ async claim() { return true; } }); + + await expect(engine.readDispatch('k1')).resolves.toBeNull(); + await expect(engine.readDispatch('k2')).resolves.toBeNull(); + const degradations = warn.mock.calls.filter( + (c) => typeof c[0] === 'string' && (c[0] as string).includes('has no read()/settle()'), + ); + expect(degradations).toHaveLength(1); + }); + + it('a read that THROWS reports the key unclaimed — a refusal is a positive reading, never a failed one', async () => { + const { logger, warn } = testLogger(); + const engine = new AutomationEngine(logger); + engine.setFlowDispatchStore({ + async claim() { return true; }, + async read() { throw new Error('ledger unreachable'); }, + async settle() { throw new Error('ledger unreachable'); }, + }); + + await expect(engine.readDispatch('k1')).resolves.toBeNull(); + expect(warn).toHaveBeenCalledWith( + expect.stringContaining('reporting the key as UNCLAIMED'), + expect.anything(), + ); + }); + + it('a settle that THROWS never fails the caller — the dispatch already happened', async () => { + const { logger, warn } = testLogger(); + const engine = new AutomationEngine(logger); + engine.setFlowDispatchStore({ + async claim() { return true; }, + async settle() { throw new Error('ledger unreachable'); }, + }); + + await expect(engine.settleDispatch('k1', 'succeeded')).resolves.toBeUndefined(); + expect(warn).toHaveBeenCalledWith( + expect.stringContaining('stays UNSETTLED'), + expect.anything(), + ); + }); + + it('with NO ledger attached the in-process fallback still carries an outcome', async () => { + const { logger } = testLogger(); + const engine = new AutomationEngine(logger); + await expect(engine.claim('k1')).resolves.toBe(true); + await engine.settleDispatch('k1', 'succeeded'); + await expect(engine.readDispatch('k1')).resolves.toMatchObject({ outcome: 'succeeded' }); + await expect(engine.readDispatch('never')).resolves.toBeNull(); + }); +}); + +describe('the settle write rule: `succeeded` is ABSORBING (#14501 contract review)', () => { + it('ObjectStoreFlowDispatchStore: failed -> succeeded is allowed — a repaired window must refuse the next replay', async () => { + const { engine, rows } = fakeQl(); + const store = new ObjectStoreFlowDispatchStore(engine); + await store.claim('schedule:digest:w1'); + await store.settle('schedule:digest:w1', 'failed'); + await expect(store.read('schedule:digest:w1')).resolves.toMatchObject({ outcome: 'failed' }); + + await store.settle('schedule:digest:w1', 'succeeded'); + await expect(store.read('schedule:digest:w1')).resolves.toMatchObject({ outcome: 'succeeded' }); + expect(rows.get('schedule:digest:w1')?.outcome).toBe('succeeded'); + }); + + it('ObjectStoreFlowDispatchStore: succeeded -> failed is REFUSED, and refused means no write at all', async () => { + // Counting `update` calls, not rows: "the row still says succeeded" + // would also be true of a store that wrote `failed` and then wrote + // `succeeded` back. The claim under test is that NOTHING is written. + // + // This is a second engine double in its own right — it RESTATES + // `update()` rather than passing the base's through — so it carries the + // dispatch predicate itself, exactly as `fakeQl()`'s does. + let updates = 0; + const { engine, rows } = fakeQl(); + const counting: FlowDispatchStoreEngine = { + find: engine.find, + insert: engine.insert, + update: (table: string, data: any, options?: any) => { + assertEngineUpdateDispatch(data, options); + updates++; + return engine.update(table, data, options); + }, + }; + const store = new ObjectStoreFlowDispatchStore(counting); + await store.claim('schedule:digest:w1'); + await store.settle('schedule:digest:w1', 'succeeded'); + expect(updates).toBe(1); + + // A FORCED replay that throws must not rewrite a delivered window: + // doing so would silently reopen the UNFORCED re-delivery door. + await store.settle('schedule:digest:w1', 'failed'); + expect(updates).toBe(1); + expect(rows.get('schedule:digest:w1')?.outcome).toBe('succeeded'); + }); + + it('the refusal is a no-op, never a throw — it is the invariant working, not an error', async () => { + const { engine } = fakeQl(); + const store = new ObjectStoreFlowDispatchStore(engine); + await store.claim('k'); + await store.settle('k', 'succeeded'); + await expect(store.settle('k', 'failed')).resolves.toBeUndefined(); + }); + + it('InMemoryFlowDispatchStore holds the same rule', async () => { + const store = new InMemoryFlowDispatchStore(); + await store.claim('k'); + await store.settle('k', 'failed'); + await store.settle('k', 'succeeded'); + await expect(store.read('k')).resolves.toMatchObject({ outcome: 'succeeded' }); + await store.settle('k', 'failed'); + await expect(store.read('k')).resolves.toMatchObject({ outcome: 'succeeded' }); + }); + + it('isSettleAllowed is the predicate, and it names exactly one refused transition', () => { + expect(isSettleAllowed(null, 'succeeded')).toBe(true); + expect(isSettleAllowed(null, 'failed')).toBe(true); + expect(isSettleAllowed('failed', 'succeeded')).toBe(true); + expect(isSettleAllowed('failed', 'failed')).toBe(true); + expect(isSettleAllowed('succeeded', 'succeeded')).toBe(true); + expect(isSettleAllowed('succeeded', 'failed')).toBe(false); + }); + + it("the engine's IN-PROCESS fallback obeys the same rule, so it cannot reopen a door the ledger keeps shut", async () => { + const { logger } = testLogger(); + const engine = new AutomationEngine(logger); + await engine.claim('k'); + await engine.settleDispatch('k', 'succeeded'); + await engine.settleDispatch('k', 'failed'); + await expect(engine.readDispatch('k')).resolves.toMatchObject({ outcome: 'succeeded' }); + }); +}); diff --git a/packages/services/service-automation/src/index.ts b/packages/services/service-automation/src/index.ts index e03ee19697..2b4bf8299e 100644 --- a/packages/services/service-automation/src/index.ts +++ b/packages/services/service-automation/src/index.ts @@ -26,6 +26,8 @@ export type { SuspensionParkedAt, SuspensionClaimOutcome, FlowDispatchStore, + FlowDispatchClaim, + FlowDispatchOutcome, // [ADR-0126 §7.2] The packaged-flow activation ledger port and its row — // the durable off-switch that REPLACES the retired process-local // `flowEnabled` map (#10243). Exported so a host can supply its own diff --git a/packages/services/service-automation/src/sys-flow-dispatch.object.ts b/packages/services/service-automation/src/sys-flow-dispatch.object.ts index d0b342a765..187633229a 100644 --- a/packages/services/service-automation/src/sys-flow-dispatch.object.ts +++ b/packages/services/service-automation/src/sys-flow-dispatch.object.ts @@ -4,7 +4,8 @@ import { ObjectSchema, Field } from '@objectstack/spec/data'; /** * sys_flow_dispatch — Persisted idempotency ledger for trigger dispatches - * (#10220). + * (#10220), and — since #14501 — the record of what each claimed dispatch + * turned into. * * A time-relative sweep (`config.timeRelative`) evaluates its date window on * every tick and launches the flow once per matching record — but the sweep @@ -28,9 +29,42 @@ import { ObjectSchema, Field } from '@objectstack/spec/data'; * catch-up sweeps, which this ledger unblocks; widen there if that work needs * more). * - * Writers: the automation engine's {@link FlowDispatchStore} (`claim()`), - * check-and-record under a system context. Readers: the same claim path, and - * operability surfaces ("what did this sweep dispatch?"). + * **The row is two-phase since #14501, and this is the one place it stopped + * being immutable.** #10220 wrote the row once, before the launch, and never + * touched it again — a row's existence was the whole of its meaning. The + * maintainer's A + a2 ruling on #14501 needs a second bit the existence of a + * row cannot carry: `IJobService.replay()` refuses a scheduled flow's window + * only when that window was **delivered**, and re-runs it when the claim is + * absent *or failed*. So `claim()` still writes the row before the launch (the + * race is still won on the primary key), and the dispatcher now settles it + * afterwards with {@link FlowDispatchStore.settle}. + * + * **The write rule, exactly** — `outcome` and `settled_at` are the only columns + * any writer ever updates, and `succeeded` is ABSORBING: + * + * - `null → succeeded` / `null → failed` — an ordinary run settling its claim. + * - `failed → succeeded` — REQUIRED, not an exception: a plain `replay()` + * re-runs a failed window, and when that run lands the window really is + * delivered, so the next unforced replay must be refused. + * - `succeeded → failed` — **refused**. A FORCED replay that throws leaves the + * window recorded `succeeded`, because rewriting it would silently reopen the + * *unforced* re-delivery door this whole ledger exists to shut. The operator + * whose forced replay failed has to force again: louder, and safer. + * + * The predicate is `isSettleAllowed` in `flow-dispatch-store.ts`, and refusing + * is a no-op rather than a throw — it is the invariant working, not an error. + * + * A row left at `outcome: null` is a dispatch whose process died mid-flight, or + * a claim written before #14501, or a `time-relative:` claim (which is never + * settled at all — only `schedule:` keys have an outcome). All of them read as + * **not delivered** — the `replay()` contract's "failed" row — because "we + * claimed it and never saw it finish" is exactly the case an operator replay + * exists to repair. + * + * Writers: the automation engine's {@link FlowDispatchStore} — `claim()` + * (check-and-record) and `settle()` (outcome only) — under a system context. + * Readers: the same claim path, `replay()`'s pre-flight check, and operability + * surfaces ("what did this sweep dispatch, and did it land?"). * * @namespace sys */ @@ -49,10 +83,10 @@ export const SysFlowDispatch = ObjectSchema.create({ retention: { maxAge: '30d' }, }, description: - 'Idempotency ledger for trigger dispatches (#10220): one row per claimed (flow, record, matched-window) key, so a re-scan or a rebuilt kernel never re-launches a flow for a window it already dispatched.', + 'Idempotency ledger for trigger dispatches (#10220): one row per claimed dispatch key — (flow, record, matched-window) for a time-relative sweep, (flow, tick-window) for a scheduled flow — so a re-scan, a rebuilt kernel or an operator replay never re-launches a flow for a window it already delivered.', displayNameField: 'id', nameField: 'id', // [ADR-0079] canonical primary-title pointer (mirrors deprecated displayNameField) - highlightFields: ['id', 'dispatched_at'], + highlightFields: ['id', 'dispatched_at', 'outcome'], fields: { // The dispatch key IS the identity — using it as the primary key makes @@ -67,6 +101,29 @@ export const SysFlowDispatch = ObjectSchema.create({ group: 'State', }), + // [#14501] The claim's outcome, and only for a `schedule:` key. OPTIONAL, + // and its absence is meaningful three ways: the mid-flight state between + // `claim()` and `settle()`, every row a ledger predating the outcome + // columns already holds, and every `time-relative:` row — that trigger + // dedups per (flow, record, window) and never settles, so half this table + // stays null by design. All three read as "not delivered", the safe + // direction: a replay re-runs rather than refusing on a claim nobody + // settled. + outcome: Field.select(['succeeded', 'failed'], { + label: 'Outcome', + required: false, + description: + 'For a scheduled-flow (schedule:) claim, what the dispatch turned into: succeeded (delivered — an unforced replay of this window is refused) or failed (the flow threw; a replay re-runs it). Succeeded is absorbing: a later failed run never overwrites it. Null means not delivered — claimed and never settled, written before this column existed, or a time-relative claim, which never settles.', + group: 'State', + }), + + settled_at: Field.datetime({ + label: 'Settled At', + required: false, + description: 'When the outcome was recorded (immediately after the flow launch this row deduplicates returned). Null wherever outcome is.', + group: 'State', + }), + created_at: Field.datetime({ label: 'Created At', required: true, diff --git a/packages/services/service-job/src/db-job-adapter.replay-guard.test.ts b/packages/services/service-job/src/db-job-adapter.replay-guard.test.ts new file mode 100644 index 0000000000..77add4b37d --- /dev/null +++ b/packages/services/service-job/src/db-job-adapter.replay-guard.test.ts @@ -0,0 +1,172 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// #14501 — the REFUSAL half of the maintainer's A + a2 ruling, pinned where its +// subject lives. `DbJobAdapter.replay()` owns the ADR-0112 envelope, the +// `force` door past it, and the guarantee that a refusal REJECTS rather than +// resolving having done nothing; it deliberately knows nothing about flows, +// tick windows or the `sys_flow_dispatch` ledger, so a guard double here is the +// real contract and not a stand-in for one. +// +// The other side of the seam — the guard `@objectstack/trigger-schedule` +// actually registers, and the `(flow, tick-window)` claim it answers from — +// is pinned in that package's `schedule-dispatch-claim.test.ts`. +// +// The specification is `IJobService.replay`'s TSDoc in `packages/spec` +// (#14766): claim absent/failed → re-run; claim succeeded → RESOURCE_CONFLICT / +// 409 naming the window and the claim; `{ force: true }` → send anyway. + +import { describe, it, expect, vi } from 'vitest'; +import { assertEngineUpdateDispatch } from '@objectstack/metadata-core'; +import { DbJobAdapter } from './db-job-adapter.js'; +import type { ReplayGuard } from './db-job-adapter.js'; + +const JOB = 'flow-schedule:nightly_digest'; +const WINDOW = "cron '0 1 * * *' window starting 2026-09-07T01:00:00.000Z"; + +function fakeEngine() { + return { + async find() { return []; }, + async insert(_t: string, data: any) { return data; }, + // Routed through ObjectQL's OWN dispatch predicate, so this fake cannot be + // looser than the engine it stands in for — the sibling doubles in this + // package do the same, and `pnpm check:engine-double-contract` is the gate. + async update(_t: string, data: any, options?: any) { + assertEngineUpdateDispatch(data, options); + return {}; + }, + }; +} + +async function adapterWith(guard: ReplayGuard | null) { + const runs: string[] = []; + const warn = vi.fn(); + const adapter = new DbJobAdapter({ + engine: fakeEngine() as any, + logger: { info: () => {}, warn, error: () => {} }, + options: { recordRuns: false }, + }); + // An interval far enough out that no timer fires inside a test. + await adapter.schedule(JOB, { type: 'interval', intervalMs: 3_600_000 }, async () => { + runs.push('r'); + }); + if (guard) adapter.setReplayGuard(JOB, guard); + return { adapter, runs, warn }; +} + +const refuse: ReplayGuard = async ({ force }) => + force ? { allow: true } : { allow: false, window: WINDOW, claimedAt: '2026-09-07T01:00:00.000Z' }; + +describe('DbJobAdapter.replay — the #14501 refusal', () => { + it('refuses a delivered window with the ADR-0112 envelope, naming the window and the claim', async () => { + const { adapter, runs } = await adapterWith(refuse); + + // The contract prescribes asserting on `code` and `status`. ⛔ Not + // `toThrow()` alone: a bare `Error` from an unfixed adapter passes that. + const err = await adapter.replay(JOB).then( + () => { throw new Error('replay resolved — the refusal did not fire'); }, + (e: any) => e, + ); + expect(err.code).toBe('RESOURCE_CONFLICT'); + expect(err.status).toBe(409); + expect(err.message).toContain(WINDOW); + expect(err.message).toContain('2026-09-07T01:00:00.000Z'); + expect(err.message).toMatch(/force: true/); + + // Refused means REFUSED — the handler did not run. + expect(runs).toHaveLength(0); + }); + + it('REJECTS rather than resolving having done nothing — the silent no-op the ruling rejected', async () => { + const { adapter } = await adapterWith(refuse); + await expect(adapter.replay(JOB)).rejects.toThrow(/already delivered/); + }); + + it('{ force: true } is the door past it, and the guard is told it is forced', async () => { + const seen: Array<{ force: boolean }> = []; + const guard: ReplayGuard = async (opts) => { seen.push(opts); return refuse(opts); }; + const { adapter, runs } = await adapterWith(guard); + + await expect(adapter.replay(JOB, undefined, { force: true })).resolves.toBeUndefined(); + expect(runs).toHaveLength(1); + expect(seen).toEqual([{ force: true }]); + + // …and an unforced replay after a forced one is refused exactly as before. + await expect(adapter.replay(JOB)).rejects.toMatchObject({ code: 'RESOURCE_CONFLICT' }); + }); + + it('force:false and an omitted options bag are the same call', async () => { + const seen: Array<{ force: boolean }> = []; + const guard: ReplayGuard = async (opts) => { seen.push(opts); return { allow: true }; }; + const { adapter } = await adapterWith(guard); + + await adapter.replay(JOB); + await adapter.replay(JOB, undefined, {}); + await adapter.replay(JOB, undefined, { force: false }); + expect(seen).toEqual([{ force: false }, { force: false }, { force: false }]); + }); + + it('a guard that ALLOWS lets the replay through untouched', async () => { + const { adapter, runs } = await adapterWith(async () => ({ allow: true })); + await expect(adapter.replay(JOB)).resolves.toBeUndefined(); + expect(runs).toHaveLength(1); + }); + + it('a job with NO guard replays exactly as it always did', async () => { + const { adapter, runs } = await adapterWith(null); + await expect(adapter.replay(JOB)).resolves.toBeUndefined(); + await expect(adapter.replay(JOB)).resolves.toBeUndefined(); + expect(runs).toHaveLength(2); + }); + + it('a guard that THROWS lets the replay through, logged — a refusal is a positive reading', async () => { + const { adapter, runs, warn } = await adapterWith(async () => { + throw new Error('ledger unreachable'); + }); + await expect(adapter.replay(JOB)).resolves.toBeUndefined(); + expect(runs).toHaveLength(1); + expect(warn).toHaveBeenCalledWith( + expect.stringContaining('replay pre-flight for job'), + expect.anything(), + ); + }); + + it('the guard is consulted only for a job that HAS one, and only once per replay', async () => { + const guard = vi.fn(async () => ({ allow: true as const })); + const { adapter } = await adapterWith(guard); + await adapter.replay(JOB); + expect(guard).toHaveBeenCalledTimes(1); + }); + + it('an unknown job is still "not found", checked before any guard runs', async () => { + const guard = vi.fn(async () => ({ allow: true as const })); + const { adapter } = await adapterWith(guard); + await expect(adapter.replay('no_such_job')).rejects.toThrow(/not found/); + expect(guard).not.toHaveBeenCalled(); + }); + + it('cancel() withdraws the guard, and setReplayGuard(name, null) does too', async () => { + const { adapter, runs } = await adapterWith(refuse); + await expect(adapter.replay(JOB)).rejects.toMatchObject({ code: 'RESOURCE_CONFLICT' }); + + adapter.setReplayGuard(JOB, null); + await expect(adapter.replay(JOB)).resolves.toBeUndefined(); + expect(runs).toHaveLength(1); + + adapter.setReplayGuard(JOB, refuse); + await adapter.cancel(JOB); + await adapter.schedule(JOB, { type: 'interval', intervalMs: 3_600_000 }, async () => { + runs.push('r'); + }); + await expect(adapter.replay(JOB)).resolves.toBeUndefined(); + expect(runs).toHaveLength(2); + + await adapter.destroy(); + }); + + it('trigger() is NOT gated — only replay() is', async () => { + const { adapter, runs } = await adapterWith(refuse); + await adapter.trigger(JOB); + await adapter.trigger(JOB); + expect(runs).toHaveLength(2); + }); +}); diff --git a/packages/services/service-job/src/db-job-adapter.ts b/packages/services/service-job/src/db-job-adapter.ts index bd101cbc82..ea0d371294 100644 --- a/packages/services/service-job/src/db-job-adapter.ts +++ b/packages/services/service-job/src/db-job-adapter.ts @@ -7,6 +7,7 @@ import type { JobExecution, JobRunOutcome, JobScheduleOptions, + JobReplayOptions, } from '@objectstack/spec/contracts'; import { IntervalJobAdapter } from './interval-job-adapter.js'; import { runWithPolicy } from './run-with-policy.js'; @@ -65,6 +66,62 @@ function withoutPolicy(options?: JobScheduleOptions): JobScheduleOptions | undef return Object.keys(rest).length > 0 ? (rest as JobScheduleOptions) : undefined; } +/** + * What a {@link ReplayGuard} answers (#14501 — the behaviour half of the + * maintainer's A + a2 ruling on #14501; the contract half is + * {@link IJobService.replay}'s TSDoc, landed by #14766). + */ +export type ReplayGuardDecision = + | { readonly allow: true } + | { + readonly allow: false; + /** Human-readable identity of the window that was already delivered. */ + readonly window: string; + /** When that window's claim was taken (ISO-8601), if the ledger knows. */ + readonly claimedAt: string | null; + }; + +/** + * A per-job pre-flight for `replay()`, registered by whoever owns the job's + * idempotency (#14501). + * + * This adapter deliberately knows nothing about flows, tick windows or the + * `sys_flow_dispatch` ledger — it cannot: a job name is an opaque string here, + * and the ledger lives two packages away. What it owns is the REFUSAL: the + * ADR-0112 envelope, the `force` door past it, and the guarantee that a + * refusal is loud rather than a silent no-op. The party that scheduled the job + * — for a scheduled flow, `@objectstack/trigger-schedule` — registers a guard + * that answers for its own window. + * + * A guard is asked once per replay and it also PREPARES: `{ allow: true }` + * means the guard has already armed whatever its owner needs to let the + * replayed run past its own idempotency gate. So this adapter never calls a + * guard for a replay it then abandons. + */ +export type ReplayGuard = (options: { readonly force: boolean }) => Promise; + +/** + * The ADR-0112 envelope `replay()` refuses a delivered window with. + * + * `RESOURCE_CONFLICT` is the standard-catalog member HTTP 409 derives + * (`HttpStatusErrorCodeMap[409]`) and the code the `domain:spec` seat ruled for + * this refusal — ⛔ no service extension code is minted for a case that + * already has one. Consumers assert on `code` and `status`; the message is for + * the operator reading it, and names both halves of the refusal: the window + * that was asked for and the claim that refused it. + */ +function replayConflict(name: string, decision: Extract): Error { + const claimed = decision.claimedAt ? ` (claimed at ${decision.claimedAt})` : ''; + const err = new Error( + `Job "${name}" has already delivered its ${decision.window}${claimed}, and that dispatch claim recorded ` + + `SUCCESS. Replaying it would deliver the same window a second time. Pass { force: true } to replay it ` + + `anyway, knowing the delivery is a duplicate.`, + ) as Error & { code?: string; status?: number }; + err.code = 'RESOURCE_CONFLICT'; + err.status = 409; + return err; +} + function uid(prefix: string): string { const g: any = globalThis as any; if (g.crypto?.randomUUID) return `${prefix}_${g.crypto.randomUUID()}`; @@ -103,6 +160,8 @@ export class DbJobAdapter implements IJobService { private readonly engine: JobEngineLike; private readonly logger?: JobLoggerLike; private readonly recordRuns: boolean; + /** job name → its `replay()` pre-flight (#14501). See {@link ReplayGuard}. */ + private readonly replayGuards = new Map(); constructor(args: { engine: JobEngineLike; @@ -206,6 +265,7 @@ export class DbJobAdapter implements IJobService { } async cancel(name: string): Promise { + this.replayGuards.delete(name); await this.inner.cancel(name); if (this.cron && typeof this.cron.cancel === 'function') { try { await this.cron.cancel(name); } catch { /* ignore */ } @@ -225,6 +285,17 @@ export class DbJobAdapter implements IJobService { return this.inner.listJobs(); } + /** + * Register (or with `null`, clear) the `replay()` pre-flight for one job — + * #14501's behaviour half. NOT part of `IJobService`: a job service without + * this method installs no guard and replays exactly as it always did, which + * is why the caller treats it as optional. See {@link ReplayGuard}. + */ + setReplayGuard(name: string, guard: ReplayGuard | null): void { + if (guard) this.replayGuards.set(name, guard); + else this.replayGuards.delete(name); + } + /** * Replay a job's most recent execution, tagging its run `trigger: 'replay'`. * @@ -234,11 +305,41 @@ export class DbJobAdapter implements IJobService { * this synthetic one, and not the per-attempt row the execution itself * would produce. `sys_job_run` is run history, not an audit trail; an * operator who switched history off gets nothing durable from this path. + * + * **Once-only delivery on the scheduled path (#14501).** A job carrying a + * {@link ReplayGuard} — a scheduled flow, registered by the schedule trigger + * — has its current dispatch window checked first. A window whose + * `(flow, tick-window)` claim recorded SUCCESS is refused with the ADR-0112 + * `RESOURCE_CONFLICT` / 409 envelope the contract declares, and the promise + * REJECTS: an operator who pressed replay and saw nothing happen is the + * experience the ruling rejected outright. Absent, failed and unsettled + * claims re-run exactly as before, and `{ force: true }` skips the check + * entirely — the operator is stating the duplicate is wanted. + * + * A guard that THROWS lets the replay through, logged: this is the same + * availability-over-strict-once posture the claim ledger takes everywhere + * else. A refusal is a positive reading, never the absence of one. */ - async replay(name: string, data?: unknown): Promise { + async replay(name: string, data?: unknown, options?: JobReplayOptions): Promise { // Same execution path as trigger but tag the run as 'replay'. const handlers = (this.inner as any).jobs?.get?.(name); if (!handlers) throw new Error(`Job "${name}" not found`); + + const guard = this.replayGuards.get(name); + if (guard) { + const force = options?.force === true; + let decision: ReplayGuardDecision | undefined; + try { + decision = await guard({ force }); + } catch (err) { + this.logger?.warn( + `DbJobAdapter: replay pre-flight for job "${name}" failed — replaying anyway ` + + '(availability over strict-once: a refusal is a positive reading, never the absence of one)', + err as any, + ); + } + if (decision && decision.allow === false) throw replayConflict(name, decision); + } // Reuse trigger; the wrap function uses a closure flag — simpler: // expose by calling inner.trigger with a marker via data is intrusive, // so we record a synthetic run row before/after to ensure 'replay' tag. diff --git a/packages/services/service-job/src/index.ts b/packages/services/service-job/src/index.ts index c35585e6aa..ae7ceea1fe 100644 --- a/packages/services/service-job/src/index.ts +++ b/packages/services/service-job/src/index.ts @@ -7,7 +7,19 @@ export type { IntervalJobAdapterOptions } from './interval-job-adapter.js'; export { CronJobAdapter } from './cron-job-adapter.js'; export type { CronJobAdapterOptions } from './cron-job-adapter.js'; export { DbJobAdapter } from './db-job-adapter.js'; -export type { DbJobAdapterOptions, JobEngineLike, JobLoggerLike } from './db-job-adapter.js'; +// `ReplayGuard` / `ReplayGuardDecision` are exported for NAMEABILITY, not +// because a caller is expected to import them by hand: `DbJobAdapter` is +// exported from this barrel and its public `setReplayGuard(name, guard)` takes +// a `ReplayGuard`, so leaving the type unnameable from this package would make +// a public parameter type impossible to write down — the same precedent +// `@objectstack/service-automation` records for its own store ports. +export type { + DbJobAdapterOptions, + JobEngineLike, + JobLoggerLike, + ReplayGuard, + ReplayGuardDecision, +} from './db-job-adapter.js'; // JobRunRetention was retired (ADR-0057): sys_job_run declares a `lifecycle` // window and the platform LifecycleService is the one sweeper. export { runWithPolicy, JobTimeoutError } from './run-with-policy.js'; diff --git a/packages/triggers/trigger-schedule/package.json b/packages/triggers/trigger-schedule/package.json index 0a13f5c6f2..164ea4fc4a 100644 --- a/packages/triggers/trigger-schedule/package.json +++ b/packages/triggers/trigger-schedule/package.json @@ -19,12 +19,12 @@ }, "dependencies": { "@objectstack/core": "workspace:*", - "@objectstack/spec": "workspace:*" + "@objectstack/spec": "workspace:*", + "croner": "^10.0.1" }, "devDependencies": { "@objectstack/service-automation": "workspace:*", "@types/node": "^26.2.0", - "croner": "^10.0.1", "typescript": "^6.0.3", "vitest": "^4.1.10" }, diff --git a/packages/triggers/trigger-schedule/src/index.ts b/packages/triggers/trigger-schedule/src/index.ts index 1b569ca04e..262f0e152e 100644 --- a/packages/triggers/trigger-schedule/src/index.ts +++ b/packages/triggers/trigger-schedule/src/index.ts @@ -2,11 +2,22 @@ export { ScheduleTriggerPlugin } from './plugin.js'; export { ScheduleTrigger, normalizeSchedule } from './schedule-trigger.js'; +// `computeTickWindow`, `scheduleDispatchKey` and `TickWindow` are deliberately +// NOT re-exported here: measured 0 consumers outside this package, they appear +// in no exported signature, and the in-package users import them from +// './schedule-trigger.js' directly. An export whose only consumers live inside +// its own package belongs in a non-barrel module. A public key-computing +// surface for operator tooling would be a card with a consumer, not a rider. export type { FlowTrigger, FlowTriggerBinding, JobServiceSurface, TriggerLogger, + ReplayGuard, + ReplayGuardDecision, + ScheduleDispatchLedger, + ScheduleDispatchClaim, + ScheduleDispatchOutcome, } from './schedule-trigger.js'; export { TimeRelativeTriggerPlugin } from './time-relative-plugin.js'; diff --git a/packages/triggers/trigger-schedule/src/plugin.ts b/packages/triggers/trigger-schedule/src/plugin.ts index ac981eaccc..71d879f033 100644 --- a/packages/triggers/trigger-schedule/src/plugin.ts +++ b/packages/triggers/trigger-schedule/src/plugin.ts @@ -2,7 +2,7 @@ import type { Plugin, PluginContext } from '@objectstack/core'; import { ScheduleTrigger } from './schedule-trigger.js'; -import type { FlowTrigger, JobServiceSurface } from './schedule-trigger.js'; +import type { FlowTrigger, JobServiceSurface, ScheduleDispatchLedger } from './schedule-trigger.js'; /** * The slice of the automation engine this plugin needs: register a trigger on @@ -67,6 +67,17 @@ export class ScheduleTriggerPlugin implements Plugin { const trigger = new ScheduleTrigger( () => this.resolveService(ctx, 'job'), ctx.logger, + // #14501 — once-per-(flow, tick-window) delivery goes through + // the SAME automation service this plugin already resolves, + // and the same `sys_flow_dispatch` ledger the time-relative + // trigger claims against (#10220). The trigger computes the + // key and never learns the ledger's table name. An automation + // service predating claim() resolves to null and the trigger + // degrades — honestly, warned once — to no dedup at all. + () => { + const svc = this.resolveService>(ctx, 'automation'); + return svc && typeof svc.claim === 'function' ? (svc as ScheduleDispatchLedger) : null; + }, ); automation.registerTrigger(trigger); ctx.logger.info('ScheduleTriggerPlugin: schedule trigger registered'); diff --git a/packages/triggers/trigger-schedule/src/schedule-dispatch-claim.test.ts b/packages/triggers/trigger-schedule/src/schedule-dispatch-claim.test.ts new file mode 100644 index 0000000000..c0bc68c329 --- /dev/null +++ b/packages/triggers/trigger-schedule/src/schedule-dispatch-claim.test.ts @@ -0,0 +1,495 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// #14501 — once-per-(flow, tick-window) delivery for SCHEDULED (cron) flows, +// the behaviour half of the maintainer's A + a2 ruling (decision batch #13). +// The contract half landed in `packages/spec` via #14766 and is the +// specification these pins are written against: `IJobService.replay`'s TSDoc +// decision table (claim absent/failed → re-run; claim succeeded → ADR-0112 +// RESOURCE_CONFLICT / 409 naming the window and the claim; `{ force: true }` → +// send anyway). +// +// The pins run against the real `ScheduleTrigger` and the real +// `AutomationEngine` claim ledger. The seam to the job service — the +// `ReplayGuard` this trigger registers — is exercised by CALLING the guard the +// trigger actually installed, and then observing what the next fire does with +// the pass it left behind. ⛔ Not by importing `DbJobAdapter`: this package's +// entry in `scripts/check-test-source-alias.mjs` is shrink-only, its tsconfig +// pins `rootDir: ./src` so the `paths` route reports TS6059 for the +// dependency's whole file graph, and the gate's own instruction for that case +// is to reach the subject through in-package source instead. The other side of +// this seam — the ADR-0112 refusal, the `force` door, and what a guard verdict +// does to `replay()` — is pinned in +// `packages/services/service-job/src/db-job-adapter.replay-guard.test.ts`, +// where `DbJobAdapter` IS in-package source. + +import { describe, it, expect, vi } from 'vitest'; +import { AutomationEngine, InMemoryFlowDispatchStore } from '@objectstack/service-automation'; +import { + ScheduleTrigger, + computeTickWindow, + scheduleDispatchKey, + type FlowTriggerBinding, + type JobServiceSurface, + type ReplayGuard, + type ScheduleDispatchLedger, + type TriggerLogger, +} from './schedule-trigger.js'; + +// ─── Harness ──────────────────────────────────────────────────────── + +const FLOW = 'nightly_digest'; +const JOB = `flow-schedule:${FLOW}`; +const CRON: FlowTriggerBinding = { + flowName: FLOW, + schedule: { type: 'cron', expression: '0 1 * * *', timezone: 'UTC' }, +}; + +/** Inside the 2026-09-07T01:00Z window of `0 1 * * *`. */ +const IN_WINDOW = new Date('2026-09-07T09:41:30Z'); +/** Inside the NEXT window. */ +const NEXT_WINDOW = new Date('2026-09-08T02:00:00Z'); + +function recordingLogger() { + const warn = vi.fn(); + const logger: TriggerLogger = { info: () => {}, warn, debug: () => {}, error: () => {} }; + return { logger, warn }; +} + +/** + * A job service that records what the trigger registers and fires it on + * demand — the same slice `DbJobAdapter` exposes, with no timers, so every + * "tick" in these tests is an explicit, deterministic call. + */ +function captureJobService() { + const jobs = new Map(); + const guards = new Map(); + const service: JobServiceSurface = { + async schedule(name, _schedule, handler) { jobs.set(name, handler as JobHandlerLike); }, + async cancel(name) { jobs.delete(name); }, + setReplayGuard(name, guard) { guards.set(name, guard); }, + }; + return { + service, + jobs, + guards, + guard: () => guards.get(JOB) ?? null, + fire: (jobId = 'j1') => jobs.get(JOB)!({ jobId }), + /** What `DbJobAdapter.replay()` does with a guard, in miniature: ask, + * refuse on `allow: false`, otherwise run. The envelope itself is + * pinned on the adapter, not here. */ + async replay(force = false) { + const guard = guards.get(JOB); + if (guard) { + const decision = await guard({ force }); + if (decision.allow === false) { + const err = new Error(`refused: ${decision.window}`) as Error & { window?: string; claimedAt?: string | null }; + err.window = decision.window; + err.claimedAt = decision.claimedAt; + throw err; + } + } + await jobs.get(JOB)!({ jobId: 'replay' }); + }, + }; +} + +type JobHandlerLike = (ctx: { jobId: string }) => Promise; + +/** The claim ledger, exactly as the automation service exposes it. */ +function realLedger(store = new InMemoryFlowDispatchStore()) { + const engine = new AutomationEngine({ + info: () => {}, warn: () => {}, error: () => {}, debug: () => {}, + child: () => undefined, + } as any); + engine.setFlowDispatchStore(store); + return { ledger: engine as unknown as ScheduleDispatchLedger, store, engine }; +} + +const flush = () => new Promise((r) => setTimeout(r, 0)); + +async function rig(opts: { now?: Date; store?: InMemoryFlowDispatchStore; throws?: boolean } = {}) { + let now = opts.now ?? IN_WINDOW; + const { ledger, store } = realLedger(opts.store); + const job = captureJobService(); + const { logger, warn } = recordingLogger(); + const runs: string[] = []; + + const trigger = new ScheduleTrigger(() => job.service, logger, () => ledger, () => now); + trigger.start(CRON, async (ctx) => { + runs.push(String((ctx.params as Record)?.jobId ?? 'run')); + if (opts.throws) throw new Error('digest render blew up'); + }); + await flush(); + + return { + trigger, job, ledger, store, runs, warn, + setNow: (d: Date) => { now = d; }, + tick: () => job.fire(), + replayThroughGuard: (force = false) => job.replay(force), + }; +} + +// ─── The window key ───────────────────────────────────────────────── + +describe('computeTickWindow — one notion of "window", derived from the schedule itself', () => { + it('cron: every instant inside one occurrence maps to the same window start', () => { + const schedule = { type: 'cron' as const, expression: '0 1 * * *', timezone: 'UTC' }; + const atFire = computeTickWindow(schedule, new Date('2026-09-07T01:00:00.000Z')); + const later = computeTickWindow(schedule, new Date('2026-09-07T23:59:59.999Z')); + expect(atFire?.startedAt).toBe('2026-09-07T01:00:00.000Z'); + expect(later?.startedAt).toBe('2026-09-07T01:00:00.000Z'); + // One millisecond BEFORE the fire is still the previous window. + expect(computeTickWindow(schedule, new Date('2026-09-07T00:59:59.999Z'))?.startedAt).toBe( + '2026-09-06T01:00:00.000Z', + ); + }); + + it('cron: the window is computed in the job\'s own timezone, so a DST shift moves fire and window together', () => { + // 02:30 America/New_York — the US spring-forward morning. The window + // key is whatever croner says the previous occurrence was, in that + // zone, which is by construction the same instant the adapter fired. + const schedule = { type: 'cron' as const, expression: '30 2 * * *', timezone: 'America/New_York' }; + const w = computeTickWindow(schedule, new Date('2026-03-08T12:00:00Z')); + expect(w?.startedAt).toEqual(expect.any(String)); + // Same reference, same answer — the property that makes tick and + // replay agree is determinism, not any particular wall-clock hour. + expect(computeTickWindow(schedule, new Date('2026-03-08T12:00:00Z'))?.startedAt).toBe(w?.startedAt); + }); + + it('interval: epoch-anchored buckets, so a restart does not move the window', () => { + const schedule = { type: 'interval' as const, intervalMs: 60_000 }; + expect(computeTickWindow(schedule, new Date(180_000))?.startedAt).toBe( + new Date(180_000).toISOString(), + ); + expect(computeTickWindow(schedule, new Date(239_999))?.startedAt).toBe( + new Date(180_000).toISOString(), + ); + expect(computeTickWindow(schedule, new Date(240_000))?.startedAt).toBe( + new Date(240_000).toISOString(), + ); + }); + + it('once: a single window, whatever the clock says', () => { + const schedule = { type: 'once' as const, at: '2026-01-01T00:00:00.000Z' }; + expect(computeTickWindow(schedule, new Date('2025-01-01T00:00:00Z'))?.startedAt).toBe( + '2026-01-01T00:00:00.000Z', + ); + expect(computeTickWindow(schedule, new Date('2030-01-01T00:00:00Z'))?.startedAt).toBe( + '2026-01-01T00:00:00.000Z', + ); + }); + + it('an unusable descriptor yields no window (and therefore no claim)', () => { + expect(computeTickWindow({ type: 'cron' } as any, IN_WINDOW)).toBeNull(); + expect(computeTickWindow({ type: 'cron', expression: 'not a cron' } as any, IN_WINDOW)).toBeNull(); + expect(computeTickWindow({ type: 'interval', intervalMs: 0 } as any, IN_WINDOW)).toBeNull(); + expect(computeTickWindow({ type: 'once', at: 'nonsense' } as any, IN_WINDOW)).toBeNull(); + }); + + it('the key is namespaced so it can never collide with a time-relative key', () => { + const w = computeTickWindow(CRON.schedule as any, IN_WINDOW)!; + expect(scheduleDispatchKey(FLOW, w)).toBe(`schedule:${FLOW}:2026-09-07T01:00:00.000Z`); + }); +}); + +// ─── PIN 1 — a second tick in the same window is a no-op ──────────── + +describe('PIN: a second tick in the same window is a no-op with a claim hit', () => { + it('fires once per window, and again in the next one', async () => { + const r = await rig(); + await r.tick(); + await r.tick(); + await r.tick(); + expect(r.runs).toHaveLength(1); + + r.setNow(NEXT_WINDOW); + await r.tick(); + expect(r.runs).toHaveLength(2); + }); + + it('a RESTART inside the window is the same case — the key is a pure function of schedule and clock', async () => { + const store = new InMemoryFlowDispatchStore(); + const first = await rig({ store }); + await first.tick(); + expect(first.runs).toHaveLength(1); + + // A rebuilt kernel: brand-new trigger, engine and job adapter over the + // one surviving ledger. + const second = await rig({ store }); + await second.tick(); + expect(second.runs).toHaveLength(0); + }); + + it('the claim it left behind records SUCCESS', async () => { + const r = await rig(); + await r.tick(); + const key = scheduleDispatchKey(FLOW, computeTickWindow(CRON.schedule as any, IN_WINDOW)!); + await expect(r.store.read(key)).resolves.toMatchObject({ outcome: 'succeeded' }); + }); +}); + +// ─── PIN 2/3 — replay refuses a delivered window unless forced ────── + +describe('PIN: replay() on a succeeded window is REFUSED by the guard the trigger registers', () => { + it('the guard answers allow:false and names the window and the claim', async () => { + const r = await rig(); + await r.tick(); + expect(r.runs).toHaveLength(1); + + const decision = await r.job.guard()!({ force: false }); + expect(decision.allow).toBe(false); + if (decision.allow === false) { + expect(decision.window).toBe("cron '0 1 * * *' window starting 2026-09-07T01:00:00.000Z"); + expect(decision.claimedAt).toEqual(expect.any(String)); + } + }); + + it('a job service that honours the verdict does not deliver the window again', async () => { + const r = await rig(); + await r.tick(); + await expect(r.replayThroughGuard()).rejects.toThrow(/refused: cron '0 1 \* \* \*' window/); + expect(r.runs).toHaveLength(1); + }); +}); + +describe('PIN: a FORCED replay sends', () => { + it('the guard allows it, and the fire it authorises actually re-runs the delivered window', async () => { + const r = await rig(); + await r.tick(); + expect(r.runs).toHaveLength(1); + + await expect(r.replayThroughGuard(true)).resolves.toBeUndefined(); + expect(r.runs).toHaveLength(2); + + // The window is still recorded delivered, so an UNFORCED replay after a + // forced one is refused exactly as before. + await expect(r.replayThroughGuard()).rejects.toThrow(/refused:/); + }); + + it('force never consults the ledger at all — a read outage cannot block the operator door', async () => { + const store = new InMemoryFlowDispatchStore(); + let reads = 0; + const counting: ScheduleDispatchLedger = { + claim: (k) => store.claim(k), + settleDispatch: (k, o) => store.settle(k, o), + async readDispatch(k) { reads++; return store.read(k); }, + }; + const job = captureJobService(); + const { logger } = recordingLogger(); + const runs: string[] = []; + const trigger = new ScheduleTrigger(() => job.service, logger, () => counting, () => IN_WINDOW); + trigger.start(CRON, async () => { runs.push('r'); }); + await flush(); + + await job.fire(); + const before = reads; + await job.guard()!({ force: true }); + expect(reads).toBe(before); + }); +}); + +// ─── PIN 4/5 — a throwing run, and the ticker that survives it ────── + +describe('PIN: a throwing run leaves a FAILED claim that a plain replay() re-runs', () => { + it('records the throw on the claim instead of leaving the run indistinguishable from a delivered one', async () => { + const r = await rig({ throws: true }); + await r.tick(); + + const key = scheduleDispatchKey(FLOW, computeTickWindow(CRON.schedule as any, IN_WINDOW)!); + await expect(r.store.read(key)).resolves.toMatchObject({ outcome: 'failed' }); + }); + + it('a plain replay() of that window re-runs it — no force needed, no refusal', async () => { + const r = await rig({ throws: true }); + await r.tick(); + expect(r.runs).toHaveLength(1); + + await expect(r.replayThroughGuard()).resolves.toBeUndefined(); + expect(r.runs).toHaveLength(2); + }); + + it('and the REPAIRED window is then refused: failed -> plain replay() -> succeeded -> next unforced replay refused', async () => { + // The full repair round-trip the contract review asked to be pinned. + // Its second half is what makes `failed -> succeeded` a REQUIRED + // transition rather than a tolerated one. + const store = new InMemoryFlowDispatchStore(); + const key = scheduleDispatchKey(FLOW, computeTickWindow(CRON.schedule as any, IN_WINDOW)!); + + const bad = await rig({ store, throws: true }); + await bad.tick(); + await expect(store.read(key)).resolves.toMatchObject({ outcome: 'failed' }); + + // A fresh binding over the same ledger, this time with a flow that works. + const good = await rig({ store }); + await expect(good.replayThroughGuard()).resolves.toBeUndefined(); + expect(good.runs).toHaveLength(1); + await expect(store.read(key)).resolves.toMatchObject({ outcome: 'succeeded' }); + + await expect(good.replayThroughGuard()).rejects.toThrow(/refused:/); + expect(good.runs).toHaveLength(1); + }); + + it('a FORCED replay that throws leaves the window recorded delivered — it must not reopen the unforced door', async () => { + const store = new InMemoryFlowDispatchStore(); + const key = scheduleDispatchKey(FLOW, computeTickWindow(CRON.schedule as any, IN_WINDOW)!); + + const good = await rig({ store }); + await good.tick(); + await expect(store.read(key)).resolves.toMatchObject({ outcome: 'succeeded' }); + + // The operator forces a re-send and the flow blows up this time. + const bad = await rig({ store, throws: true }); + await expect(bad.replayThroughGuard(true)).resolves.toBeUndefined(); + expect(bad.runs).toHaveLength(1); + + // The claim still says delivered, so an UNFORCED replay is still refused. + await expect(store.read(key)).resolves.toMatchObject({ outcome: 'succeeded' }); + await expect(bad.replayThroughGuard()).rejects.toThrow(/refused:/); + }); + + it('an UNSETTLED claim — the process died mid-launch — reads as not delivered and replays too', async () => { + const store = new InMemoryFlowDispatchStore(); + const key = scheduleDispatchKey(FLOW, computeTickWindow(CRON.schedule as any, IN_WINDOW)!); + // A claim taken by a process that never came back to settle it. + await store.claim(key); + + const r = await rig({ store }); + await expect(r.replayThroughGuard()).resolves.toBeUndefined(); + expect(r.runs).toHaveLength(1); + }); +}); + +describe('PIN: the ticker survives the throw — the error isolation must NOT regress', () => { + // ⚠ These assert on the handler the trigger REGISTERS, not on a fire routed + // through `DbJobAdapter.trigger()`. Measured on `origin/main`: the adapter + // chain swallows too — `IntervalJobAdapter.executeJob` catches every handler + // rejection and records it as a `failed` execution — so a fire driven + // through the adapter resolves whether or not the trigger's own catch + // exists, and a pin written that way would be green against a trigger that + // rethrows. The property the ruling protects is the trigger's, so it is + // pinned where it lives. + function bareJobService() { + const jobs = new Map Promise>(); + const service: JobServiceSurface = { + async schedule(name, _s, handler) { jobs.set(name, handler as any); }, + async cancel(name) { jobs.delete(name); }, + setReplayGuard() { /* not exercised here */ }, + }; + return { service, fire: (id = 'j1') => jobs.get(JOB)!({ jobId: id }) }; + } + + async function throwingRig(now: Date, store = new InMemoryFlowDispatchStore()) { + const { ledger } = realLedger(store); + const { logger, warn } = recordingLogger(); + const bare = bareJobService(); + const runs: string[] = []; + let clock = now; + const trigger = new ScheduleTrigger(() => bare.service, logger, () => ledger, () => clock); + trigger.start(CRON, async () => { runs.push('r'); throw new Error('digest render blew up'); }); + await flush(); + return { fire: bare.fire, runs, warn, setNow: (d: Date) => { clock = d; } }; + } + + it('a throwing flow never rejects out of the handler the trigger registered', async () => { + const r = await throwingRig(IN_WINDOW); + await expect(r.fire()).resolves.toBeUndefined(); + expect(r.warn).toHaveBeenCalledWith(expect.stringContaining('execution failed: digest render blew up')); + }); + + it('and the next window still fires — one bad run does not stop the schedule', async () => { + const r = await throwingRig(IN_WINDOW); + await expect(r.fire()).resolves.toBeUndefined(); + r.setNow(NEXT_WINDOW); + await expect(r.fire()).resolves.toBeUndefined(); + expect(r.runs).toHaveLength(2); + }); + + it('a settle that THROWS still does not break the ticker', async () => { + const store = new InMemoryFlowDispatchStore(); + const hostile: ScheduleDispatchLedger = { + claim: (k) => store.claim(k), + async settleDispatch() { throw new Error('ledger unreachable'); }, + async readDispatch() { return null; }, + }; + const { logger, warn } = recordingLogger(); + const job = captureJobService(); + const runs: string[] = []; + const trigger = new ScheduleTrigger(() => job.service, logger, () => hostile, () => IN_WINDOW); + trigger.start(CRON, async () => { runs.push('r'); }); + await flush(); + + await expect(job.fire()).resolves.toBeUndefined(); + expect(runs).toHaveLength(1); + expect(warn).toHaveBeenCalledWith(expect.stringContaining('could not record dispatch outcome')); + }); +}); + +// ─── Declared degradations ────────────────────────────────────────── + +describe('declared degradations', () => { + it('no ledger at all: every tick fires, and the lost guarantee is said exactly once', async () => { + const job = captureJobService(); + const { logger, warn } = recordingLogger(); + const runs: string[] = []; + const trigger = new ScheduleTrigger(() => job.service, logger, () => null, () => IN_WINDOW); + trigger.start(CRON, async () => { runs.push('r'); }); + await flush(); + + await job.fire(); + await job.fire(); + expect(runs).toHaveLength(2); + const said = warn.mock.calls.filter( + (c) => typeof c[0] === 'string' && c[0].includes('NOT deduplicated'), + ); + expect(said).toHaveLength(1); + }); + + it('a job service with no setReplayGuard keeps ticking correctly and says the refusal is unavailable', async () => { + const jobs = new Map Promise }>(); + const bare: JobServiceSurface = { + async schedule(name, _s, handler) { jobs.set(name, { handler: handler as any }); }, + async cancel(name) { jobs.delete(name); }, + }; + const { ledger } = realLedger(); + const { logger, warn } = recordingLogger(); + const runs: string[] = []; + const trigger = new ScheduleTrigger(() => bare, logger, () => ledger, () => IN_WINDOW); + trigger.start(CRON, async () => { runs.push('r'); }); + await flush(); + + await jobs.get(JOB)!.handler({ jobId: 'j1' }); + await jobs.get(JOB)!.handler({ jobId: 'j2' }); + expect(runs).toHaveLength(1); // ticks are unaffected + + expect(warn).toHaveBeenCalledWith( + expect.stringContaining('no replay guard registration'), + ); + }); + + it('a claim that THROWS dispatches anyway — availability over strict-once', async () => { + const hostile: ScheduleDispatchLedger = { + async claim() { throw new Error('ledger unreachable'); }, + }; + const job = captureJobService(); + const { logger, warn } = recordingLogger(); + const runs: string[] = []; + const trigger = new ScheduleTrigger(() => job.service, logger, () => hostile, () => IN_WINDOW); + trigger.start(CRON, async () => { runs.push('r'); }); + await flush(); + + await job.fire(); + await job.fire(); + expect(runs).toHaveLength(2); + expect(warn).toHaveBeenCalledWith(expect.stringContaining('dispatching anyway')); + }); + + it('stop() withdraws the replay guard, so a re-registered job is not judged by a dead one', async () => { + const r = await rig(); + await r.tick(); + expect(await r.job.guard()!({ force: false })).toMatchObject({ allow: false }); + + r.trigger.stop(FLOW); + await flush(); + expect(r.job.guard()).toBeNull(); + }); +}); diff --git a/packages/triggers/trigger-schedule/src/schedule-trigger.ts b/packages/triggers/trigger-schedule/src/schedule-trigger.ts index ff9eb4d03d..e91602a727 100644 --- a/packages/triggers/trigger-schedule/src/schedule-trigger.ts +++ b/packages/triggers/trigger-schedule/src/schedule-trigger.ts @@ -1,5 +1,6 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. +import { Cron } from 'croner'; import type { AutomationContext } from '@objectstack/spec/contracts'; import type { JobSchedule, JobHandler } from '@objectstack/spec/contracts'; @@ -31,14 +32,177 @@ export interface FlowTrigger { stop(flowName: string): void; } +/** + * What a {@link ReplayGuard} answers when a job service is about to replay a + * job it does not itself understand. Structural mirror of the job adapter's + * own type — see the note on {@link JobServiceSurface}. + */ +export type ReplayGuardDecision = + | { readonly allow: true } + | { + readonly allow: false; + /** Human-readable identity of the window that was already delivered. */ + readonly window: string; + /** When that window's claim was taken (ISO-8601), if the ledger knows. */ + readonly claimedAt: string | null; + }; + +/** + * A per-job pre-flight the job service runs before `replay()` (#14501). + * + * It is asked, and it also PREPARES: a guard that answers `{ allow: true }` + * has already armed whatever its owner needs to let the replayed run through + * its own idempotency gate. So a job service must call it exactly once per + * replay, and must not call it for a replay it then abandons. + */ +export type ReplayGuard = (options: { readonly force: boolean }) => Promise; + /** * The slice of `IJobService` this trigger needs: schedule a named job and * cancel it. Typed structurally so the plugin depends on the spec contract * shape, not a concrete adapter. + * + * `setReplayGuard` is OPTIONAL and is NOT part of the `IJobService` spec + * contract — it is the adapter-local registration `DbJobAdapter` grew for + * #14501, and a job service without it (the bootstrap `IntervalJobAdapter`, + * any third-party adapter) simply never installs the guard. That degradation + * is declared, not silent: see {@link ScheduleTrigger} for what is lost. */ export interface JobServiceSurface { schedule(name: string, schedule: JobSchedule, handler: JobHandler): Promise; cancel(name: string): Promise; + setReplayGuard?(name: string, guard: ReplayGuard | null): void; +} + +/** What a claimed dispatch turned into — mirror of the ledger's own type. */ +export type ScheduleDispatchOutcome = 'succeeded' | 'failed'; + +/** One dispatch-claim row, as this trigger reads it back. */ +export interface ScheduleDispatchClaim { + readonly outcome: ScheduleDispatchOutcome | null; + readonly claimedAt: string | null; +} + +/** + * The slice of the automation service this trigger needs for once-per-window + * delivery (#14501): the same `sys_flow_dispatch` claim ledger the + * time-relative trigger uses for its per-record keys (#10220), plus the + * outcome half the #14501 ruling added. + * + * Typed structurally — like {@link JobServiceSurface} — so this plugin never + * learns the ledger's table name and takes no build dependency on + * `@objectstack/service-automation`. `settleDispatch` / `readDispatch` are + * optional for the same reason `claim` is resolved defensively: an automation + * service predating either one resolves to a partial surface, and the trigger + * degrades honestly rather than throwing at bind time. + */ +export interface ScheduleDispatchLedger { + claim(key: string): Promise; + settleDispatch?(key: string, outcome: ScheduleDispatchOutcome): Promise; + readDispatch?(key: string): Promise; +} + +/** + * One tick window — the unit a scheduled flow is delivered once per, and the + * `(flow, tick-window)` half of the #14501 claim key. + */ +export interface TickWindow { + /** + * The window's start instant as an ISO-8601 string, seconds precision. + * This IS the window's identity: two fires share a window exactly when + * they share this value. + */ + readonly startedAt: string; + /** How the window reads in a log line or a replay refusal. */ + readonly label: string; +} + +/** + * The most recent tick window of `schedule` at or before `now`, or `null` when + * the descriptor admits no window. + * + * ⚠️ There is exactly ONE notion of "window" here and it is derived from the + * schedule descriptor itself, so the tick that takes a claim and the replay + * that reads it back cannot disagree — which is the whole reason the key is + * computed here rather than read off the fire: + * + * - **cron** — the previous occurrence of the very same expression in the + * very same timezone, computed by the very same library the job adapter + * schedules with (`croner`). A DST or leap-second boundary therefore moves + * the fire and the window key together, by construction; there is no second + * calendar to disagree with. `croner` resolves patterns to whole seconds, + * so the reference is advanced to the start of the next second to turn its + * strictly-before answer into the at-or-before one this needs. + * ⚠️ That "one calendar" property rests on an assumption worth naming: an + * absent `schedule.timezone` falls back to `'UTC'` here, which is the same + * default `CronJobAdapter` applies to the fire (`options.timezone ?? 'UTC'`). + * A host that constructed its adapter with a different default timezone + * would fire on its calendar while this keys on UTC, and the two would part + * company at a DST boundary. + * - **interval** — the epoch-anchored bucket `floor(now / intervalMs)`. + * Anchored to the epoch and not to registration time on purpose: a restart + * re-registers the timer at a new offset, and a window that moved with it + * would forget every claim across exactly the restart this ledger exists to + * survive. Consecutive fires are `intervalMs` apart and buckets are + * `intervalMs` wide, so two fires never share one. + * - **once** — the single instant the job is due. One window, forever. + */ +export function computeTickWindow(schedule: JobSchedule, now: Date): TickWindow | null { + const nowMs = now.getTime(); + if (!Number.isFinite(nowMs)) return null; + + if (schedule.type === 'cron') { + const expression = schedule.expression; + if (!expression) return null; + // croner strips milliseconds and answers strictly BEFORE the reference + // second; advancing to the start of the next second makes that the + // at-or-before answer, including when `now` IS the fire instant. + const reference = new Date(Math.floor(nowMs / 1000) * 1000 + 1000); + let previous: Date | undefined; + try { + const pattern = new Cron(expression, { timezone: schedule.timezone ?? 'UTC' }); + previous = pattern.previousRuns(1, reference)[0]; + pattern.stop(); + } catch { + return null; // unparseable expression — the job will not bind either + } + if (!previous) return null; + const startedAt = previous.toISOString(); + return { + startedAt, + label: `cron '${expression}' window starting ${startedAt}`, + }; + } + + if (schedule.type === 'interval') { + const intervalMs = schedule.intervalMs; + if (!intervalMs || intervalMs <= 0) return null; + const startedAt = new Date(Math.floor(nowMs / intervalMs) * intervalMs).toISOString(); + return { + startedAt, + label: `interval ${intervalMs}ms window starting ${startedAt}`, + }; + } + + if (schedule.type === 'once') { + if (!schedule.at) return null; + const at = new Date(schedule.at); + if (!Number.isFinite(at.getTime())) return null; + const startedAt = at.toISOString(); + return { startedAt, label: `one-shot window at ${startedAt}` }; + } + + return null; +} + +/** + * The `(flow, tick-window)` dispatch key (#14501). + * + * Namespaced `schedule:` so it can never collide with the time-relative + * trigger's `time-relative:` keys in the one shared ledger. + */ +export function scheduleDispatchKey(flowName: string, window: TickWindow): string { + return `schedule:${flowName}:${window.startedAt}`; } /** Minimal logger surface (matches core's `ctx.logger`). */ @@ -156,6 +320,26 @@ export function normalizeSchedule(raw: unknown): JobSchedule | null { * The job service is resolved lazily (per `start()`) via the supplied accessor, * so we always pick up the job service's *upgraded* adapter (e.g. the durable * DbJobAdapter that replaces the bootstrap interval adapter on `kernel:ready`). + * + * ## Once-per-window delivery (#14501) + * + * A scheduled flow claims a `(flow, tick-window)` key in the shared + * `sys_flow_dispatch` ledger before it launches, and settles that claim with + * the run's outcome afterwards — the same ledger the time-relative trigger + * claims per `(flow, record, window)` (#10220), with the key shape the + * maintainer's A + a2 ruling named. Three doors close at once: + * + * - a second tick inside one window finds the claim and does nothing; + * - a restart inside a window is that same case, because the key is a pure + * function of the schedule descriptor and the clock, not of process state; + * - an operator `replay()` of a window that was DELIVERED is refused with an + * ADR-0112 `RESOURCE_CONFLICT` / 409 envelope, via the + * {@link ReplayGuard} this trigger registers on the job service. + * + * What did NOT change is the error isolation: a throwing flow is still caught + * and swallowed so the ticker survives. It stopped being SILENT — the throw + * settles the window's claim `failed`, and a plain `replay()` re-runs a failed + * window — but the ticker's protection is unchanged and must stay that way. */ export class ScheduleTrigger implements FlowTrigger { readonly type = 'schedule'; @@ -164,10 +348,49 @@ export class ScheduleTrigger implements FlowTrigger { private readonly logger: TriggerLogger; /** flowName → job name registered for it, so stop() can cancel it. */ private readonly bound = new Map(); + /** Dispatch-claim ledger (#14501), resolved lazily per fire. */ + private readonly getLedger: () => ScheduleDispatchLedger | null; + /** Injectable clock so window math is deterministic under test. */ + private readonly now: () => Date; + /** + * flowName → the ONE dispatch key a {@link ReplayGuard} has authorised for + * re-dispatch (#14501). A replay of a window whose claim is absent or + * failed must actually re-run it — but the handler's own claim gate would + * see the existing row and no-op, so the guard leaves a one-shot pass here + * and the handler consumes it. In-process by construction and correctly + * so: the pass is written and read inside a single `replay()` call chain. + * + * Keyed by FLOW rather than accumulated in a set, so a pass a job service + * asked for and then abandoned is overwritten by the next one instead of + * outliving its window — at most one outstanding pass per bound flow, and + * `stop()` takes it with the binding. + * + * ⚠️ Residue is therefore bounded but not zero: an abandoned pass survives + * until this flow's next guard call replaces it, or `stop()` drops it. A + * later fire does NOT clear it — the handler deletes the entry only when + * the pass MATCHES the window it just computed — so an abandoned pass + * outlives every fire in every other window. It stays inert through all of + * them for the same reason: a pass naming a window that has passed can + * never match again. Its blast radius is one fire of one flow inside the + * window the pass names, and only if that window is still current — a fire + * that would have been a no-op runs instead. + */ + private readonly replayPasses = new Map(); + /** Whether the in-process-only dedup degradation has been said (once). */ + private claimDegradationWarned = false; + /** Whether the "no replay guard could be installed" degradation has been said (once). */ + private replayGuardDegradationWarned = false; - constructor(getJobService: () => JobServiceSurface | null, logger: TriggerLogger) { + constructor( + getJobService: () => JobServiceSurface | null, + logger: TriggerLogger, + getLedger: () => ScheduleDispatchLedger | null = () => null, + now: () => Date = () => new Date(), + ) { this.getJobService = getJobService; this.logger = logger; + this.getLedger = getLedger; + this.now = now; } start(binding: FlowTriggerBinding, callback: (ctx: AutomationContext) => Promise): void { @@ -195,6 +418,27 @@ export class ScheduleTrigger implements FlowTrigger { const jobName = `${JOB_PREFIX}:${binding.flowName}`; const handler: JobHandler = async ({ jobId }) => { + // #14501 — once-per-(flow, tick-window) delivery. The key is + // computed from the schedule descriptor, so it is the same value + // on a re-tick inside the window, after a restart inside the + // window, and at an operator replay. + const window = computeTickWindow(schedule, this.now()); + const key = window ? scheduleDispatchKey(binding.flowName, window) : null; + if (key) { + // A guard-issued pass means this run IS the re-dispatch of a + // window whose claim was absent or failed (or a forced replay + // of a delivered one): claim anyway so the ledger records the + // dispatch, but do not let a claim miss stop it. + const replayPass = this.replayPasses.get(binding.flowName) === key; + if (replayPass) this.replayPasses.delete(binding.flowName); + const claimed = await this.claimDispatch(binding.flowName, key); + if (!claimed && !replayPass) { + this.logger.debug?.( + `[schedule] flow '${binding.flowName}' already dispatched for ${window!.label} — skipping`, + ); + return; + } + } try { const ctx: AutomationContext = { event: 'schedule', @@ -205,15 +449,25 @@ export class ScheduleTrigger implements FlowTrigger { }, }; await callback(ctx); + if (key) await this.settleDispatch(binding.flowName, key, 'succeeded'); } catch (err) { // Error isolation: a scheduled flow failure must not crash the // job runner / ticker. Log and swallow. + // + // #14501 kept the swallow — it is what protects the ticker, + // and the ruling says so explicitly — and took away only its + // silence: the window's claim is settled `failed`, so the run + // is no longer indistinguishable from a delivered one and a + // plain `replay()` re-runs it. this.logger.warn( `[schedule] flow '${binding.flowName}' execution failed: ${(err as Error)?.message ?? String(err)}`, ); + if (key) await this.settleDispatch(binding.flowName, key, 'failed'); } }; + this.installReplayGuard(jobService, jobName, binding.flowName, schedule); + this.bound.set(binding.flowName, jobName); // FlowTrigger.start is sync; the job service's schedule() is async. // Fire-and-forget with error logging. @@ -232,12 +486,138 @@ export class ScheduleTrigger implements FlowTrigger { }); } + /** + * Install the `replay()` pre-flight for this job (#14501), when the job + * service has somewhere to put one. + * + * Degradation contract, declared once: a job service without + * `setReplayGuard` (the bootstrap `IntervalJobAdapter`, any adapter + * predating #14501) keeps every other guarantee here — a second tick in a + * window is still a no-op, a throw is still recorded failed — but an + * operator replay of a DELIVERED window can no longer be refused loudly. + * It hits the handler's claim gate and returns having done nothing, which + * is the silent no-op the ruling exists to prevent, so it is said out loud + * here instead. + * + * ⚠ Said only when a ledger is actually attached. With no ledger nothing is + * ever RECORDED as delivered, so there is no refusal to lose and the line + * would be a false alarm — that deployment's real degradation is the + * "delivery is NOT deduplicated" warning {@link claimDispatch} already + * emits, and stacking a second, vacuous warning on top of it buries the + * one that matters. + */ + private installReplayGuard( + jobService: JobServiceSurface, + jobName: string, + flowName: string, + schedule: JobSchedule, + ): void { + if (typeof jobService.setReplayGuard !== 'function') { + if (!this.replayGuardDegradationWarned && this.getLedger() !== null) { + this.replayGuardDegradationWarned = true; + this.logger.warn( + `[schedule] job service has no replay guard registration — a replay of a scheduled flow's ` + + `ALREADY-DELIVERED tick window cannot be refused and will quietly do nothing instead of ` + + `raising RESOURCE_CONFLICT. Ticks are unaffected.`, + ); + } + return; + } + jobService.setReplayGuard(jobName, async ({ force }) => { + const window = computeTickWindow(schedule, this.now()); + if (!window) return { allow: true }; + const key = scheduleDispatchKey(flowName, window); + const claim = force ? null : await this.readDispatch(flowName, key); + if (claim?.outcome === 'succeeded') { + return { allow: false, window: window.label, claimedAt: claim.claimedAt }; + } + // Absent, failed, unsettled, or forced — this replay re-runs the + // window, so let the handler past its own claim gate exactly once. + this.replayPasses.set(flowName, key); + return { allow: true }; + }); + } + + /** + * Claim one `(flow, tick-window)` dispatch key (#14501): `true` = launch, + * `false` = this window was already dispatched (an earlier tick this + * process, or a previous process lifetime). + * + * Degradation contract, deliberately identical to the time-relative + * trigger's: a ledger call that THROWS dispatches anyway (availability + * over strict-once — a broken ledger must never silently swallow a + * digest), and a missing ledger is warned once because the once-per-window + * guarantee then no longer survives a kernel rebuild. + */ + private async claimDispatch(flowName: string, key: string): Promise { + const ledger = this.getLedger(); + if (ledger && typeof ledger.claim === 'function') { + try { + return await ledger.claim(key); + } catch (err) { + this.logger.warn( + `[schedule] flow '${flowName}' dispatch-claim failed for key '${key}' — dispatching anyway ` + + `(availability over strict-once; the same window may re-fire until the claim store recovers): ` + + `${(err as Error)?.message ?? String(err)}`, + ); + return true; + } + } + if (!this.claimDegradationWarned) { + this.claimDegradationWarned = true; + this.logger.warn( + `[schedule] no dispatch-claim surface (automation service missing or without claim()) — ` + + `scheduled-flow delivery is NOT deduplicated: a restart inside a tick window, or an operator ` + + `replay, can deliver the same window twice.`, + ); + } + return true; + } + + /** Record what a dispatch turned into. Best-effort: never fails the run. */ + private async settleDispatch( + flowName: string, + key: string, + outcome: ScheduleDispatchOutcome, + ): Promise { + const ledger = this.getLedger(); + if (!ledger || typeof ledger.settleDispatch !== 'function') return; + try { + await ledger.settleDispatch(key, outcome); + } catch (err) { + this.logger.warn( + `[schedule] flow '${flowName}' could not record dispatch outcome '${outcome}' for key '${key}' — ` + + `the claim stays unsettled and reads as NOT delivered, so a later replay is allowed through: ` + + `${(err as Error)?.message ?? String(err)}`, + ); + } + } + + /** Read one dispatch claim. A ledger that cannot answer reports `null`. */ + private async readDispatch(flowName: string, key: string): Promise { + const ledger = this.getLedger(); + if (!ledger || typeof ledger.readDispatch !== 'function') return null; + try { + return await ledger.readDispatch(key); + } catch (err) { + this.logger.warn( + `[schedule] flow '${flowName}' dispatch-claim read failed for key '${key}' — treating the window as ` + + `unclaimed so the replay proceeds: ${(err as Error)?.message ?? String(err)}`, + ); + return null; + } + } + stop(flowName: string): void { const jobName = this.bound.get(flowName); if (!jobName) return; this.bound.delete(flowName); + this.replayPasses.delete(flowName); const jobService = this.getJobService(); if (!jobService || typeof jobService.cancel !== 'function') return; + if (typeof jobService.setReplayGuard === 'function') { + try { jobService.setReplayGuard(jobName, null); } catch { /* the cancel below is what matters */ } + } void Promise.resolve(jobService.cancel(jobName)) .then(() => this.logger.debug?.(`[schedule] unbound flow '${flowName}'`)) .catch((err) => { diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 70fa8ee2bf..81670053fb 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -2879,6 +2879,9 @@ importers: '@objectstack/spec': specifier: workspace:* version: link:../../spec + croner: + specifier: ^10.0.1 + version: 10.0.1 devDependencies: '@objectstack/service-automation': specifier: workspace:* @@ -2886,9 +2889,6 @@ importers: '@types/node': specifier: ^26.2.0 version: 26.2.0 - croner: - specifier: ^10.0.1 - version: 10.0.1 typescript: specifier: ^6.0.3 version: 6.0.3 diff --git a/scripts/engine-double-contract.pinned.json b/scripts/engine-double-contract.pinned.json index 6a0b08e46a..df301f791f 100644 --- a/scripts/engine-double-contract.pinned.json +++ b/scripts/engine-double-contract.pinned.json @@ -3441,6 +3441,11 @@ "verb": "update", "pinned": 1 }, + { + "file": "packages/services/service-automation/src/flow-dispatch.test.ts", + "verb": "update", + "pinned": 4 + }, { "file": "packages/services/service-automation/src/record-lookup-expand.integration.test.ts", "verb": "findOne", @@ -3521,6 +3526,11 @@ "verb": "update", "pinned": 1 }, + { + "file": "packages/services/service-job/src/db-job-adapter.replay-guard.test.ts", + "verb": "update", + "pinned": 1 + }, { "file": "packages/services/service-job/src/db-job-adapter.timeout.test.ts", "verb": "update",