Skip to content

Commit 1726802

Browse files
committed
fix(service-automation): a switched-off caller guards its packaged subflow only while it holds a parked run
The disable direction of the ADR-0126 7.3 subflow guard now reads reachability, per triage's answer A on #20678. A packaged caller guards the disable of a packaged subflow when it is enabled (step: disable it), or when it is disabled, by the ledger or by its status, and holds a parked run (step: cancel each named run through the ADR-0044 operator cancel door, or let it finish). A disabled caller with no parked run no longer guards, so "disable the caller, then the callee" completes. "Holds a parked run" is one helper, parkedRunsOf, over the one reader of both run stores (readSuspendedRuns, the body listSuspendedRunsDurable already served). The listing keeps its degrade-on-outage posture; the guard takes a throw posture, so an unlistable store propagates its own failure instead of reading as "no parked run". The refusal keeps DELETE_RESTRICTED / 409 and subflowCallers. Rider: in disabledPackagedSubflows the ledger-cycle exemption is asked only of a child whose status does not also disable it, so a child disabled both ways inside a ledger cycle is named with its publish remedy. resume() / resumeInternal() are unchanged. Claude-Session: https://claude.ai/code/session_01XY5uCwTjZj7884yYtyur4H Co-authored-by: Claude <noreply@anthropic.com>
1 parent 97222b8 commit 1726802

1 file changed

Lines changed: 155 additions & 46 deletions

File tree

  • packages/services/service-automation/src

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

Lines changed: 155 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -4618,19 +4618,16 @@ export class AutomationEngine implements IAutomationService {
46184618

46194619
/**
46204620
* [ADR-0126 §7.3] Packaged flows that invoke `name` as a subflow — the
4621-
* callers the DISABLE direction of the guard refuses on.
4621+
* callers the DISABLE direction of the guard judges.
46224622
*
46234623
* Only PACKAGED callers are reported: §7.3's rationale is that a VENDOR
46244624
* flow would break mid-run at its subflow node. A caller the customer
46254625
* authored is theirs to fix, and refusing on it would make the packaged
46264626
* artifact hostage to a tenant's own flow.
46274627
*
4628-
* A caller counts whatever its own activation state. Switching a caller
4629-
* off stops its NEW runs only: a run it had already parked — at a `wait`,
4630-
* an `approval`, a `screen`, or between two items of a `map` — resumes
4631-
* through {@link resume}, which does not consult activation, and walks on
4632-
* into its subflow node. So a switched-off caller is not yet an inert one,
4633-
* and skipping it here would reopen the mid-run failure §7.3 refuses.
4628+
* Every packaged caller is reported, whatever its own activation state.
4629+
* Which of them still GUARD — can still reach the subflow node — is
4630+
* {@link refuseDisableUnderReachingCallers}'s question, not this scan's.
46344631
*/
46354632
private packagedSubflowCallers(name: string): string[] {
46364633
const callers: string[] = [];
@@ -4642,6 +4639,105 @@ export class AutomationEngine implements IAutomationService {
46424639
return callers;
46434640
}
46444641

4642+
/**
4643+
* [ADR-0126 §7.3] The runs each of `flowNames` holds PARKED — suspended
4644+
* and resumable — read from BOTH run stores: this process's hot cache and
4645+
* the durable {@link SuspendedRunStore}, including a run a previous
4646+
* process parked that this one has never loaded.
4647+
*
4648+
* The ONE answer the disable direction of the subflow guard takes, for its
4649+
* verdict and for its refusal text alike
4650+
* ({@link refuseDisableUnderReachingCallers}), so the runs a refusal names
4651+
* are exactly the runs that made it refuse.
4652+
*
4653+
* Read through {@link readSuspendedRuns}, the same merge
4654+
* {@link listSuspendedRunsDurable} serves, with one difference: an
4655+
* enumeration that FAILS throws here instead of degrading to the cache.
4656+
* The listing is an operability read and says so in a warning; this
4657+
* answer decides a write, and "the store could not say" is not "no parked
4658+
* run" — read as one, it would let a disable land under a run a previous
4659+
* process parked, the exact mid-run failure §7.3 refuses.
4660+
*
4661+
* A run that has ENDED — completed, failed, refused, cancelled — is not in
4662+
* either store, so it never counts.
4663+
*/
4664+
private async parkedRunsOf(flowNames: ReadonlySet<string>): Promise<Map<string, string[]>> {
4665+
const parked = new Map<string, string[]>();
4666+
if (flowNames.size === 0) return parked;
4667+
for (const run of await this.readSuspendedRuns('throw')) {
4668+
if (!flowNames.has(run.flowName)) continue;
4669+
parked.set(run.flowName, [...(parked.get(run.flowName) ?? []), run.runId]);
4670+
}
4671+
for (const runIds of parked.values()) runIds.sort();
4672+
return parked;
4673+
}
4674+
4675+
/**
4676+
* [ADR-0126 §7.3, the disable direction] Refuse switching `name` off while
4677+
* a packaged flow can still REACH it as a subflow, naming each such caller
4678+
* and the step that lets the disable complete.
4679+
*
4680+
* Reachability, not the caller's switch — triage's answer A on #20678,
4681+
* the reading of §7.3's own rationale. Switching a caller off stops its
4682+
* NEW runs only: a run it had already parked — at a `wait`, an `approval`,
4683+
* a `screen`, or at a `map` node between two items — resumes through
4684+
* {@link resume}, which does not consult activation, and walks on into its
4685+
* subflow node. So a packaged caller guards when:
4686+
* - it is ENABLED ({@link isFlowEnabled}) — the step is to disable it; or
4687+
* - it is disabled, by the ledger or by its definition's `status` alike,
4688+
* and holds a parked run ({@link parkedRunsOf}) — the step is to cancel
4689+
* each named run through the operator cancel door (ADR-0044) or to let
4690+
* it finish.
4691+
* A disabled caller with no parked run cannot reach the node, and does not
4692+
* guard. The parked-run read happens only when a disabled caller is being
4693+
* judged, so a flow whose callers are all armed never touches the store.
4694+
*
4695+
* ADR-0112 envelope unchanged: `DELETE_RESTRICTED` / 409, and
4696+
* `subflowCallers` lists exactly the callers that guard.
4697+
*
4698+
* @throws the durable suspended-run store's own failure when it cannot be
4699+
* listed while a disabled caller is judged — see {@link parkedRunsOf}.
4700+
*/
4701+
private async refuseDisableUnderReachingCallers(name: string): Promise<void> {
4702+
const callers = this.packagedSubflowCallers(name);
4703+
if (callers.length === 0) return;
4704+
const armed = callers.filter((c) => this.isFlowEnabled(c));
4705+
const parked = await this.parkedRunsOf(new Set(callers.filter((c) => !this.isFlowEnabled(c))));
4706+
const guarding = callers.filter((c) => armed.includes(c) || parked.has(c));
4707+
if (guarding.length === 0) return;
4708+
4709+
const quote = (names: string[]) => names.map((n) => `'${n}'`).join(', ');
4710+
const one = guarding.length === 1;
4711+
const steps = [
4712+
...(armed.length > 0 ? [`disable the calling flow${armed.length === 1 ? '' : 's'} ${quote(armed)} first`] : []),
4713+
...[...parked].map(([caller, runIds]) => {
4714+
const single = runIds.length === 1;
4715+
return (
4716+
`'${caller}' is disabled but still holds ${single ? 'a parked run' : `${runIds.length} parked runs`} ` +
4717+
`that ${single ? 'resumes' : 'resume'} into its subflow node (${quote(runIds)}): cancel ` +
4718+
`${single ? 'it' : 'each'} through the operator cancel door, ` +
4719+
`POST /automation/${caller}/runs/:runId/cancel (ADR-0044), or let ${single ? 'it' : 'them'} finish`
4720+
);
4721+
}),
4722+
].join('; ');
4723+
throw Object.assign(
4724+
new Error(
4725+
`Flow '${name}' cannot be disabled while ${guarding.length} packaged flow${one ? '' : 's'} ` +
4726+
`still call${one ? 's' : ''} it as a subflow: ${quote(guarding)}. Disabling it would break ` +
4727+
`${one ? 'that caller' : 'those callers'} mid-run at ${one ? 'its' : 'their'} subflow node with a ` +
4728+
`late, inexplicable failure (ADR-0126 §7.3). ` +
4729+
`${steps.charAt(0).toUpperCase()}${steps.slice(1)} — or leave this one armed.`,
4730+
),
4731+
// ADR-0112 envelope: code AND status. `DELETE_RESTRICTED` is the
4732+
// standard catalog's "cannot do this due to dependencies" member
4733+
// (409) — ⛔ no new ledger entry is minted here. Its `DELETE_`
4734+
// prefix fits because this repo's own #10243 ruling records that
4735+
// "disabling a shipped flow is functionally equivalent to deleting
4736+
// it for as long as it stays off".
4737+
{ code: 'DELETE_RESTRICTED', status: 409, subflowCallers: guarding },
4738+
);
4739+
}
4740+
46454741
/**
46464742
* [ADR-0126 §7.3, the enable direction] The packaged subflows a packaged
46474743
* flow `name` calls that may not run now ({@link isFlowEnabled}), each
@@ -4663,6 +4759,12 @@ export class AutomationEngine implements IAutomationService {
46634759
* disabled flows closes that loop: a status-disabled link is re-armed by
46644760
* publishing its definition, a door this guard does not sit on, and an
46654761
* enabled link does not refuse at all.
4762+
*
4763+
* ⛔ And the exemption answers the LEDGER bit only, so it is asked only of
4764+
* a child whose status does not disable it too. A child disabled BOTH ways
4765+
* inside such a cycle is still reported: no enable order re-arms a status,
4766+
* so the cycle is no reason to leave its publish remedy unnamed — and
4767+
* skipping it would arm the caller onto a child that stays disabled.
46664768
*/
46674769
private disabledPackagedSubflows(name: string): Array<{ name: string; ledger: boolean; status?: string }> {
46684770
const flow = this.flows.get(name);
@@ -4673,10 +4775,9 @@ export class AutomationEngine implements IAutomationService {
46734775
if (!child || describeFlowContender(child).source !== 'package') continue;
46744776
if (this.isFlowEnabled(target)) continue;
46754777
const ledger = this.flowLedgerDisabled.has(target);
4676-
if (ledger && this.ledgerDisabledChainReaches(target, name)) continue;
4677-
const status = this.flowStatusDisabled.get(target) === true
4678-
? String((child as { status?: unknown }).status)
4679-
: undefined;
4778+
const statusDisabled = this.flowStatusDisabled.get(target) === true;
4779+
if (ledger && !statusDisabled && this.ledgerDisabledChainReaches(target, name)) continue;
4780+
const status = statusDisabled ? String((child as { status?: unknown }).status) : undefined;
46804781
disabled.push({ name: target, ledger, ...(status !== undefined ? { status } : {}) });
46814782
}
46824783
return disabled;
@@ -4792,9 +4893,10 @@ export class AutomationEngine implements IAutomationService {
47924893
* wire goes through the gate.
47934894
*
47944895
* @throws when the flow is unknown, when §7.3's subflow guard refuses in
4795-
* either direction, or when the durable write fails — a reported flip
4796-
* that did not persist is the failure mode this whole leg exists to
4797-
* remove.
4896+
* either direction, when the durable suspended-run store cannot be
4897+
* listed while the disable guard judges a switched-off caller, or when
4898+
* the durable write fails — a reported flip that did not persist is the
4899+
* failure mode this whole leg exists to remove.
47984900
*/
47994901
async toggleFlow(name: string, enabled: boolean): Promise<void> {
48004902
const flow = this.flows.get(name);
@@ -4804,34 +4906,15 @@ export class AutomationEngine implements IAutomationService {
48044906

48054907
// [ADR-0126 §7.3] The subflow guard runs in BOTH directions, because
48064908
// a subflow pair breaks from either end. Disabling a child breaks the
4807-
// callers that still reach it; re-arming a caller whose child is off
4808-
// breaks the caller itself, at its subflow node, on the child's
4809-
// refusal. Arming a flow never breaks the flows that call IT, so
4810-
// enabling a child is not guarded by its callers.
4909+
// callers that still reach it — armed, or holding a parked run;
4910+
// re-arming a caller whose child is off breaks the caller itself, at
4911+
// its subflow node, on the child's refusal. Arming a flow never breaks
4912+
// the flows that call IT, so enabling a child is not guarded by its
4913+
// callers.
48114914
if (enabled) {
48124915
this.refuseEnableOntoDisabledSubflow(name, flow);
48134916
} else {
4814-
const callers = this.packagedSubflowCallers(name);
4815-
if (callers.length > 0) {
4816-
const list = callers.map((c) => `'${c}'`).join(', ');
4817-
throw Object.assign(
4818-
new Error(
4819-
`Flow '${name}' cannot be disabled while ${callers.length} packaged flow` +
4820-
`${callers.length === 1 ? '' : 's'} still call${callers.length === 1 ? 's' : ''} it as a subflow: ${list}. ` +
4821-
`Disabling it would break ${callers.length === 1 ? 'that caller' : 'those callers'} mid-run at ` +
4822-
`${callers.length === 1 ? 'its' : 'their'} subflow node with a late, inexplicable failure ` +
4823-
`(ADR-0126 §7.3). Disable the calling flow${callers.length === 1 ? '' : 's'} first, or leave this one armed.`,
4824-
),
4825-
// ADR-0112 envelope: code AND status. `DELETE_RESTRICTED` is
4826-
// the standard catalog's "cannot do this due to
4827-
// dependencies" member (409) — ⛔ no new ledger entry is
4828-
// minted here. Its `DELETE_` prefix fits because this
4829-
// repo's own #10243 ruling records that "disabling a
4830-
// shipped flow is functionally equivalent to deleting it
4831-
// for as long as it stays off".
4832-
{ code: 'DELETE_RESTRICTED', status: 409, subflowCallers: callers },
4833-
);
4834-
}
4917+
await this.refuseDisableUnderReachingCallers(name);
48354918
}
48364919

48374920
// The durable row FIRST. A store that throws aborts the flip with
@@ -8829,6 +8912,24 @@ export class AutomationEngine implements IAutomationService {
88298912
* the per-id "no row" the strict loader rests on; see the merge below.
88308913
*/
88318914
async listSuspendedRunsDurable(): Promise<Array<{ runId: string; flowName: string; nodeId: string; correlation?: string }>> {
8915+
return this.readSuspendedRuns('degrade');
8916+
}
8917+
8918+
/**
8919+
* The ONE reader of "which runs are parked" over BOTH run stores — the
8920+
* body of {@link listSuspendedRunsDurable}, and the read the §7.3 disable
8921+
* guard takes through {@link parkedRunsOf}, so the listing and the guard
8922+
* can never disagree about a run the store answered for.
8923+
*
8924+
* They differ in ONE thing, the posture on an enumeration that FAILS:
8925+
* - `'degrade'` — the listing: warn, and answer from the cache alone.
8926+
* - `'throw'` — a caller that decides a WRITE on what is ABSENT from the
8927+
* answer: an outage is "unknown", never "not parked", so the store's
8928+
* own failure propagates and nothing is decided on a short list.
8929+
*/
8930+
private async readSuspendedRuns(
8931+
onEnumerationFailure: 'degrade' | 'throw',
8932+
): Promise<Array<{ runId: string; flowName: string; nodeId: string; correlation?: string }>> {
88328933
const byId = new Map<string, { runId: string; flowName: string; nodeId: string; correlation?: string }>();
88338934
// [#15832] Did the ENUMERATION answer? The reconcile below is allowed
88348935
// only when it did — see the merge comment for why a failed listing is
@@ -8841,6 +8942,10 @@ export class AutomationEngine implements IAutomationService {
88418942
}
88428943
enumerated = true;
88438944
} catch (err) {
8945+
// A write-deciding caller is told, and decides nothing: see
8946+
// {@link readSuspendedRuns}. Everything below is the LISTING's
8947+
// posture.
8948+
if (onEnumerationFailure === 'throw') throw err;
88448949
// #6299 — driver text to the structured slot, message one line,
88458950
// same as the two seams above. The SLOT differs: the `Logger`
88468951
// contract declares `warn(message, meta?)`, so `meta` is the
@@ -8867,13 +8972,17 @@ export class AutomationEngine implements IAutomationService {
88678972
// instead. Its scan roots (`packages/metadata`,
88688973
// `metadata-protocol`, `objectql`) do not reach this package, so
88698974
// `check:durability-log-level` reports neither rule here.
8870-
// Reachability is also the weakest of the three: this method has
8871-
// no production consumer in-repo and is not on the
8872-
// `AutomationService` spec contract (only the synchronous
8873-
// `listSuspendedRuns` is), so nothing decides anything on this
8874-
// list today. Raising it to `error` would alarm for the duration
8875-
// of an outage on a read nobody acts on — #4632's mirror-image
8876-
// misuse, the trap #6230 avoided.
8975+
// Reachability is also the weakest of the three: this method is
8976+
// not on the `AutomationService` spec contract (only the
8977+
// synchronous `listSuspendedRuns` is); the in-repo caller that
8978+
// would ADMIT a write on what this list lacks — the §7.3
8979+
// disable guard — takes the `'throw'` posture above and never
8980+
// reaches this line, and plugin-approvals' restored-
8981+
// continuation proof refuses on an absent entry, so a short
8982+
// list costs it a retry. Raising it to `error` would alarm for
8983+
// the duration of an outage on a read no caller acts on
8984+
// wrongly — #4632's mirror-image misuse, the trap #6230
8985+
// avoided.
88778986
this.logger.warn(
88788987
`[automation] the durable suspended-run store could not be listed — this listing DEGRADES to the ` +
88798988
`in-memory cache alone, so every run parked by a previous process is missing from the result ` +

0 commit comments

Comments
 (0)