Skip to content

Commit 7589676

Browse files
committed
fix(service-automation): one enablement gate in activateFlowTrigger, so a trigger registered after ledger hydration never arms a disabled flow
activateFlowTrigger now refuses to arm a flow isFlowEnabled answers false for, so registerFlow, registerTrigger, the enable toggle and any later arming path inherit it; registerFlow's caller-side check is folded into it. The trigger-fired callback says a failure is in the run history only for a run that dispatched and failed (status 'failed'), and a FLOW_DISABLED refusal that still reaches it is logged at info as a refusal, not at error as a failed run. Claude-Session: https://claude.ai/code/session_01XY5uCwTjZj7884yYtyur4H Co-authored-by: Claude <noreply@anthropic.com>
1 parent 1a7ce83 commit 7589676

2 files changed

Lines changed: 113 additions & 17 deletions

File tree

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
---
2+
'@objectstack/service-automation': patch
3+
---
4+
5+
fix(service-automation): a flow switched off in the activation ledger stays unbound after a restart, and a trigger-fired refusal no longer logs an ERROR claiming a run-history row (#20677)
6+
7+
Clause-②: no
8+
9+
**What was wrong.** A packaged flow switched off through the ADR-0126 activation
10+
ledger (`POST /api/v1/automation/:name/toggle` with `enabled: false`) came back
11+
`bound: true` after every cold restart. Its runs were still refused, so the switch
12+
itself held, but its trigger was armed again. At boot the automation service pulls
13+
the flows and applies the ledger, which leaves a switched-off flow unbound. The
14+
trigger plugins register later, at `kernel:ready`, and registering a trigger armed
15+
every matching flow without asking whether it may run. So `GET
16+
/api/v1/automation/_status` reported the flow `enabled: false, bound: true`. Each
17+
matching event also logged `ERROR Trigger-fired run of flow '…' failed`, saying the
18+
failure "is recorded in the flow's run history", while no run row was written.
19+
20+
**What changed.**
21+
22+
- The engine checks whether a flow may run in one place: at the step that arms a
23+
trigger. Every arming path goes through it: flow registration (boot pull,
24+
publish, hot reload), trigger registration, and the enable toggle. A flow that
25+
either disable dimension switches off (the activation ledger, or an `obsolete` /
26+
`invalid` status) is never armed, whenever its trigger registers.
27+
- Re-enabling a flow arms it on its trigger as before. Re-enabling the ledger bit
28+
of a flow whose `status` is still `obsolete` or `invalid` no longer arms it, since
29+
every run it fired would be refused.
30+
- A trigger-fired run refused because the flow is disabled (for example, an event
31+
already in flight when the flow was switched off) is logged at `info`, saying
32+
nothing ran and no run-history row records it. It is no longer an `ERROR`.
33+
- The `ERROR` line for any other trigger-fired failure says the failure is
34+
recorded in the run history only for a run that dispatched and failed. A run
35+
refused before it dispatched gets the same line without that claim.
36+
37+
**What is not affected.** The runtime refusal (`FLOW_DISABLED`) and its message are
38+
unchanged. The enabled flows beside a disabled one arm exactly as before. No export,
39+
option, route or response shape changes.

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

Lines changed: 74 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -3371,6 +3371,10 @@ export class AutomationEngine implements IAutomationService {
33713371
// A trigger may be registered *after* its flows (e.g. AutomationServicePlugin
33723372
// pulls flows at start(); a trigger plugin wires up on kernel:ready, which
33733373
// fires later). Activate any already-registered flow that maps to this type.
3374+
// [ADR-0126 §7.2] A flow that may not run is skipped by
3375+
// `activateFlowTrigger`'s own enablement gate, so the flows the ledger
3376+
// hydration left unbound stay unbound here — ⛔ no second check in this
3377+
// loop: the gate is shared so no arming path can go without it.
33743378
for (const name of this.flows.keys()) {
33753379
if (this.boundFlowTriggers.has(name)) continue;
33763380
const resolved = this.resolveTriggerBinding(name);
@@ -3584,11 +3588,34 @@ export class AutomationEngine implements IAutomationService {
35843588

35853589
/**
35863590
* Bind a flow to its matching registered trigger (idempotent). No-op when
3587-
* the flow has no trigger binding or no trigger is registered for its type
3588-
* yet — {@link registerTrigger} re-attempts activation when one arrives.
3591+
* the flow may not run ({@link isFlowEnabled}), when it has no trigger
3592+
* binding, or when no trigger is registered for its type yet —
3593+
* {@link registerTrigger} re-attempts activation when one arrives.
35893594
*/
35903595
private activateFlowTrigger(flowName: string): void {
35913596
if (this.boundFlowTriggers.has(flowName)) return;
3597+
// [ADR-0126 §7.2] THE enablement gate for arming — here, at the one
3598+
// point every arming path crosses, and not in its callers. A flow
3599+
// either disable dimension switches off (the activation ledger, or an
3600+
// `obsolete` / `invalid` status) is never handed to a trigger, whichever
3601+
// path asks: {@link registerFlow} (boot pull, publish, hot reload),
3602+
// {@link registerTrigger}, the enable half of {@link toggleFlow}, and
3603+
// any path added later.
3604+
//
3605+
// Why it cannot live in the callers: `registerTrigger` runs when a
3606+
// trigger plugin registers at `kernel:ready`, AFTER `start()` pulled
3607+
// the flows and {@link hydrateFlowActivations} unbound the switched-off
3608+
// ones. While only `registerFlow` asked, that later registration
3609+
// re-armed every one of them on every cold boot — `/_status` reported a
3610+
// disabled flow `bound: true`, and each matching event fired a run
3611+
// `execute()` then refused.
3612+
//
3613+
// Silent on purpose: an unarmed disabled flow is the state the switch
3614+
// exists to produce, the host's hydration line already named it, and
3615+
// `getFlowRuntimeStates()` reports it `enabled: false`. Ahead of the
3616+
// scheduled-work policy gate below for the same reason — a flow that
3617+
// may not run has no policy refusal to record.
3618+
if (!this.isFlowEnabled(flowName)) return;
35923619
const resolved = this.resolveTriggerBinding(flowName);
35933620
if (!resolved) return;
35943621
// [#17396] The deployment gate, read HERE rather than only inside the
@@ -3661,18 +3688,44 @@ export class AutomationEngine implements IAutomationService {
36613688
// landed; the only other trace is the passive run-history row.
36623689
// That stderr also survives the CLI's boot-quiet stdout window is
36633690
// stream mechanics, not the verdict.
3691+
//
3692+
// [ADR-0126 §7.2] The line states only what happened. Two kinds of
3693+
// `execute()` answer are not a failed run:
3694+
// - `FLOW_DISABLED`: the flow was switched off after the trigger
3695+
// took the event — an event in flight at the switch-off, or a
3696+
// trigger whose `stop()` failed. The refusal IS the switch
3697+
// working, so it is said at `info`, never as an `error` that
3698+
// reads as a production failure. The enablement gate above
3699+
// keeps a disabled flow from being armed at all, so this is the
3700+
// residue, not the steady state.
3701+
// - every other never-dispatched exit (a `code` or a missing
3702+
// flow, and no `status`): no node ran, so the line claims no
3703+
// run-history row. Only a run that dispatched and failed
3704+
// carries `status: 'failed'` — the verdict that exit also wrote
3705+
// to the run history.
36643706
trigger.start(resolved.binding, (ctx: AutomationContext) =>
36653707
this.execute(flowName, ctx).then((result) => {
3666-
if (!result.success) {
3667-
this.logger.error(
3668-
`Trigger-fired run of flow '${flowName}' failed (trigger '${resolved.triggerType}') — ` +
3669-
`no caller holds this result and nothing retries the run; the terminal failure ` +
3670-
`is recorded in the flow's run history, and the run's failure envelope is in ` +
3671-
`this record's meta.`,
3672-
undefined,
3673-
{ error: result.error ?? 'unknown error' },
3708+
if (result.success) return;
3709+
if (result.code === 'FLOW_DISABLED') {
3710+
this.logger.info(
3711+
`Trigger '${resolved.triggerType}' fired flow '${flowName}', which is disabled — the ` +
3712+
`run was refused before it started, nothing ran, and no run-history row records ` +
3713+
`it. The refusal is in this record's meta.`,
3714+
{ error: result.error },
36743715
);
3716+
return;
36753717
}
3718+
this.logger.error(
3719+
`Trigger-fired run of flow '${flowName}' failed (trigger '${resolved.triggerType}') — ` +
3720+
`no caller holds this result and nothing retries the run; ` +
3721+
(result.status === 'failed'
3722+
? `the terminal failure is recorded in the flow's run history, and the run's ` +
3723+
`failure envelope is in this record's meta.`
3724+
: `it was refused before it dispatched, and the refusal envelope is in this ` +
3725+
`record's meta.`),
3726+
undefined,
3727+
{ error: result.error ?? 'unknown error' },
3728+
);
36763729
}),
36773730
);
36783731
this.boundFlowTriggers.set(flowName, resolved.triggerType);
@@ -4263,14 +4316,15 @@ export class AutomationEngine implements IAutomationService {
42634316
}
42644317

42654318
// Re-bind in case the definition changed its trigger, then (re)activate.
4266-
// [ADR-0126 §7.2] A ledger-disabled flow is NOT re-armed here, which is
4267-
// what makes the unbind survive a republish and a restart: the boot
4268-
// pull re-registers every flow, so a hydrated ledger row has to be
4269-
// able to keep a trigger unbound through exactly this path.
4319+
// [ADR-0126 §7.2] A disabled flow — ledger or status — is NOT re-armed
4320+
// here, which is what makes the unbind survive a republish and a
4321+
// restart: the boot pull re-registers every flow, so a hydrated ledger
4322+
// row has to be able to keep a trigger unbound through exactly this
4323+
// path. The refusal is `activateFlowTrigger`'s own enablement gate,
4324+
// the one every arming path shares — ⛔ not re-asked here, where a
4325+
// caller-side check once stood alone and `registerTrigger` had none.
42704326
this.deactivateFlowTrigger(name);
4271-
if (this.isFlowEnabled(name)) {
4272-
this.activateFlowTrigger(name);
4273-
}
4327+
this.activateFlowTrigger(name);
42744328

42754329
// #12206 (Option A) — hand the caller the canonicalized flow this
42764330
// registration stored: the same object `this.flows` now holds and
@@ -4657,6 +4711,9 @@ export class AutomationEngine implements IAutomationService {
46574711
// A disabled flow should stop receiving trigger events; a re-enabled one
46584712
// should resume. execute() also guards disabled flows, but unbinding
46594713
// avoids firing the trigger (and its event-source subscription) at all.
4714+
// Re-enabling moves only the LEDGER bit: a flow whose `status` still
4715+
// disables it stays unarmed, by `activateFlowTrigger`'s enablement gate
4716+
// — armed, it would only fire runs `execute()` refuses.
46604717
if (enabled) {
46614718
this.activateFlowTrigger(name);
46624719
} else {

0 commit comments

Comments
 (0)