Skip to content

service-automation: three more readers of suspended-run state still prefer the per-process map over the shared store #14332

Description

@claude

Found while fixing #13617 (multi-replica approval resume), filed rather than folded in.

What

#13617 made the RESUME path store-authoritative: AutomationEngine.loadSuspendedRunStrict
now reads the shared sys_automation_run row and consults the per-process map only for a run
whose durable save failed. Three other readers of the same state still prefer the per-process
map and were deliberately left alone, because they are the same class but not the same fix:

  1. AutomationEngine.cancelRunthis.suspendedRuns.get(runId) ?? await this.store.load(runId).
    On a replica holding a stale entry, the cancel acts on the stale SuspendedRun object: the
    row deletion is still by id and therefore correct, but forgetSuspendedRun notifies the
    WRONG node executor that its pause is over, so whatever that node armed is not the thing
    torn down.
  2. AutomationEngine.failAncestors — the same ?? chain while walking the $parentRunId
    chain. This one has the harm shape of automation/approvals: 多副本集群下审批流每级节点(除首级)被重复创建 —— approve 后恢复读到滞后一拍的流运行态,同级要批两次(单副本零重复) #13617 itself: a stale parent means failing an
    ancestor at a node it has already left.
  3. AutomationEngine.listSuspendedRunsDurable — merges the durable list with the in-process
    map under the comment In-memory entries win — they are the freshest copy, which is exactly
    backwards once several replicas share one store. Lowest harm of the three: its own docblock
    records that it has no in-repo production consumer.

Why it was not fixed in that PR

Sites 1 and 2 carry bespoke degradation contracts with their own recorded #4632/#6299 verdicts
in long docblocks — the posture at each was a deliberate judgment, so routing them through the
shared loader changes documented logging and degradation behaviour rather than being mechanical.
That failed the in-scope test for a bounded drive-by fix.

Suggested shape

One answer to where is this run parked, not four: route all of them through
loadSuspendedRun / loadSuspendedRunStrict and keep each site’s existing degradation posture
by choosing the degrading or strict loader deliberately, rather than by re-deriving the read.
The getSuspendedScreen docblock already states the principle (one loader, two callers).

Repro shape is the two-engines-over-one-shared-store harness added in
packages/services/service-automation/src/multi-replica-resume-staleness.test.ts.

Generated by Claude Code


Generated by Claude Code

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions