Repository navigation
Commit 679f95e
fix(service-automation)!: refuse re-enabling a packaged flow onto a disabled packaged subflow (#20711)
Part of #20678
Clause-②: no (narrowing)
**Which half landed.** The card's direction has two halves. This PR
lands the second, the inverse guard: re-enabling a packaged caller whose
packaged subflow is still disabled is refused. The first half, where
`packagedSubflowCallers` skips a disabled caller so that "disable the
caller, then the callee" completes, is **not built**. Its premise was
measured false (below), and the dispatch's fork rule for that case is to
stop and return the choice to the maintainer. So #20678 remains open for
the disable-direction half.
## The measurement that stopped the disable half
The direction rests on this premise: a disabled caller is inert, so
disabling its callee cannot break it mid-run. That premise does not
hold. `toggleFlow(caller, false)` stops new runs only. `resume()` /
`resumeInternal()` never consult `isFlowEnabled`, so a run the caller
parked before the switch-off still resumes and walks into its subflow
node.
Measured on the engine at `defc7f7b` with a throwaway probe (not
committed). The probe used the real subflow, map and wait executors and
the in-memory ledger. The callee was switched off out of band, which is
the state the proposed skip would let `toggleFlow` produce.
- Caller `start → wait → subflow(callee)`: parked at the wait, caller
disabled, callee disabled, then `resume(runId)`:
`{"success":false,"status":"stranded","error":"Node 'call' failed:
subflow 'callee' failed: Flow 'callee' is disabled — it is switched off
for this installation in the packaged-metadata activation ledger …"}`
- The same with a `map` node: `"Node 'call' failed: map 'call': item 0
(subflow 'callee') failed: Flow 'callee' is disabled …"`
- **The shipped map pair's own shape.** `showcase_release_signoff` maps
over `showcase_one_task_signoff`, which pauses on an approval. The probe
used a caller that maps over a pausing child, with 2 items, parked at
item 0. The caller was disabled, then the callee. Then the child's pause
was woken (the approval decision or timer path). `resume(childRunId)`
answered `{"success":true}`, and the disabled parent's run row reads
`failed`: `"Node 'each' failed: map 'each': item 1 (subflow 'per_item')
failed: Flow 'per_item' is disabled …"`. This is ADR-0126 §7.3's "late,
inexplicable failure". The person who decided the approval is told it
succeeded.
- Control: the caller disabled with the callee still enabled. The parked
run resumes to `{"success":true}`, so a disabled caller's parked run
really does continue.
The options, analysed on the four axes, are in the dev report on #20678.
The recommendation there is to keep counting a disabled caller only
while it holds a parked run, and to name those runs and the operator
cancel door in the refusal.
## What this PR changes
(`packages/services/service-automation/src/engine.ts`)
- **`toggleFlow(name, true)` on a packaged flow the ledger holds off**
now refuses when a packaged flow it calls is disabled. "Calls" means the
`flowName` of a `subflow` node or of a `map` node. The refusal comes
before any write: the row still reads off, the trigger stays unbound,
and `execute()` still answers `FLOW_DISABLED`.
- **Envelope: `RESOURCE_CONFLICT` / 409**, with the message naming the
flow and each disabled subflow. That is the standard catalog's generic
"conflicts with the current state" member, and no ledger entry was
minted. I measured the catalog's 409 members. `DELETE_RESTRICTED` rests
on "disabling a shipped flow is functionally equivalent to deleting it",
and an enable is the opposite act. `CONCURRENT_MODIFICATION`,
`DUPLICATE_RECORD` and `LOCK_CONFLICT` name other conflicts.
`PRECONDITION_REQUIRED` (428) is about a request that is missing
something, and this request is complete. Precedent:
`IJobService.replay()` refuses a delivered window with
`RESOURCE_CONFLICT` / 409.
- **The remedy follows each subflow's real state** (dispatch assumption
A2, binding). The activation switch moves only the ledger bit, so:
- switched off in the activation ledger: `Enable 'X' first, then enable
this flow — or leave it disabled.`
- disabled by its definition's `status` (`obsolete` / `invalid`):
`Publish 'X' with status 'active' (this switch never changes a
definition's status; for a package that is read-only here, that takes a
package version that ships it active) first, then enable this flow …`.
It never says "enable 'X'", because that toggle lands and changes
nothing. A pin follows it and shows the child still refuses.
- both: both steps, in that order.
- **A cycle of switched-off flows does not guard itself.** If A calls B
and B calls A, and both are switched off, each would refuse the other,
and no remedy could complete. A subflow is skipped when the flow being
enabled is reachable from it through ledger-disabled packaged flows. A
flow that calls itself is the one-node case. A chain back through an
enabled flow is not a cycle: there the subflow is still named, and
enabling it first completes (pinned).
- **Not refused:** enabling a flow that is already enabled (nothing is
re-armed), a customer-authored caller, and a customer-authored subflow
(the ruling scopes the guard to a packaged subflow).
- The "which flows does this flow call" scan is extracted into
`subflowTargets()`, which both directions now share. The disable
direction's behaviour is unchanged. Its docblock now records the
measured reason a disabled caller still counts.
- A4: the in-code sentence "Enable is never guarded — arming a flow
cannot break a caller" is rewritten. The rewrite says arming a flow
never breaks the flows that call it, but re-arming a caller onto a
disabled child breaks the caller.
## The wire (A6)
Measured on a live showcase boot (`pnpm dev -- -p 39141 --database
file:…`, two boots over one SQLite file; built at `08ed6d7b0`). The
route is `packages/runtime/src/domains/automation.ts`, `POST
/:name/toggle` arm, which calls `automationService.toggleFlow(name,
enabled)` and lets the throw reach the dispatcher's error mapping. **No
route change was needed.** Between the boots, with the server stopped, a
ledger row `active: 0` was written for `showcase_notify_owner` and for
`showcase_one_task_signoff`. That is the state a package upgrade that
adds a caller after the callee was switched off leaves. Boot 2 hydrated
both rows (`/_status`: `enabled:false, bound:false`).
- `POST /api/v1/automation/showcase_task_done_notify_owner/toggle
{"enabled":true}` answered `HTTP 409` with
`{"success":false,"error":{"code":"RESOURCE_CONFLICT","message":"Flow
'showcase_task_done_notify_owner' cannot be enabled while 1 packaged
subflow it calls is disabled: 'showcase_notify_owner' (switched off in
the activation ledger). … Enable 'showcase_notify_owner' first, then
enable this flow — or leave it disabled.","httpStatus":409}}`
- Enabling `showcase_notify_owner` answered 200, then re-enabling the
caller answered 200.
- Map pair: disabling `showcase_release_signoff` answered 200.
Re-enabling it answered 409 `RESOURCE_CONFLICT` naming
`showcase_one_task_signoff`. Enabling the callee answered 200, then the
caller answered 200.
- The card's own reproduction, the disable direction, is unchanged on
this branch: after disabling the caller (200), disabling
`showcase_notify_owner` still answers 409 `DELETE_RESTRICTED` naming
`showcase_task_done_notify_owner`.
## Pins (`flow-activation-ledger.test.ts`, new describe "ADR-0126 §7.3
(enable direction)")
Every refusal pin asserts `code: 'RESOURCE_CONFLICT'`, `status: 409`,
`Flow 'NAME' cannot be enabled`, and the named subflow.
- The subflow pair and the map pair: the caller is refused first, naming
the subflow. Nothing moved (row, binding, `execute()`). Then
callee-first completes, and a real run goes through the node into the
re-armed child.
- Every disabled subflow is named.
- A status-disabled subflow gets the publish remedy, never the toggle.
The pin follows the remedy and the caller then enables.
- A subflow disabled both ways gets both reasons and both steps, and the
steps complete.
- A cycle, a self-call, and a chain back through an enabled flow.
- Controls: a non-packaged caller, a non-packaged subflow, and an
already-enabled caller.
- The existing test "ENABLE is never guarded — arming a flow cannot
break a caller" is renamed. It now reads: enabling a CALLEE is never
guarded by its callers. The body is unchanged.
**Red first:** `73333a32c` against the unfixed engine gave `Tests 6
failed | 34 passed (40)`, each failure `enabling 'X' was accepted`. The
fix is `58f0aef61`.
**Ablations.** Each leg went through `scripts/ablation-replace.mjs` in
wrap mode: the anchor hits once, the mutation is proven on disk, and the
restore is proven. The script also carried an outer `trap` on
EXIT/INT/TERM. The subject resolves from `src/` through the relative
`./engine.js` import, so no `dist/` leg applies. Every leg ended with
`ok restored: blob == HEAD (bb22908) and git diff HEAD is empty`.
| mutation | red |
|---|---|
| M1 the enable guard call removed | 6: both pairs, every-name, status,
both-ways, chain |
| M2 every subflow reported as ledger-disabled | 2: status, both-ways |
| M3 cycle skip removed | 2: cycle, self-call |
| M4 the cycle walk crosses enabled flows | 1: chain |
| M5 the packaged-caller check removed | 1: non-packaged caller |
| M6 the packaged-subflow check removed | 1: non-packaged subflow |
| M7 the re-enable precondition removed | 1: already-enabled |
| M8 only the first subflow scanned | 1: every-name |
| M9 a callee guarded by its callers on enable | 7, incl. the renamed
callee pin |
The first M9 attempt was a no-op: its replacement re-contained the
anchor, the tool refused it before running anything (its report: the
anchor count moved from 1 to 1), and nothing was measured. It was re-run
with a distinct replacement, and that re-run is the row above.
## Tests and gates (all at `08ed6d7b0`, the head)
- `pnpm --filter @objectstack/service-automation test`: `Test Files 155
passed (155)`, `Tests 1935 passed (1935)`.
- `pnpm --filter @objectstack/service-automation run typecheck`: exit 0
(`tsc --noEmit`, then `check:test-typecheck: OK`). `--listFiles` counts
the test file in both `tsconfig.json` and `tsconfig.test.json` (1 each).
- `node scripts/pm/dispatch-gates.mjs --commands --repo
objectstack-ai/objectstack`, re-derived against this diff, gives 62
commands. All 62 ran with the exit captured before any pipe, and all
exited 0. `--ran`: `62 derived famil(ies) accounted for — 62 run, 0
NOT-MEASURED (a DERIVED zero …)`.
- The roster families printed outside the runnable list all exited 0:
`node scripts/check-changeset-fixed.mjs`, `pnpm check:authz-resolver`,
`pnpm check:error-code-casing` ("no unlisted lowercase error codes in
6984 scanned file(s)") and `pnpm check:filter-alias-parity`. Also run
and exit 0: `pnpm check:durability-log-level` and `pnpm
check:startup-registry-verdict`.
- `check-adr-0087-registration`: `1 declared-breaking changeset(s), each
carrying an ADR-0087 disposition` (`not-required
(no-migration-prescription)`).
- NOT MEASURED: the six value-bearing CI invocations `dispatch-gates`
lists as not runnable locally (`check-issue-citations --census`, the
shard attestations, `check-test-completeness`). Reason: their argv comes
from the workflow.
- **Lint, a declared narrowing** (`pnpm lint` over the repo is CI's):
`eslint --no-inline-config --format json` on the three changed files.
Result: 3 files, 0 errors, 1 warning, and the warning is the changeset's
"File ignored because no matching configuration was supplied". The
population is from `eslint.config.mjs` (`files:
['**/*.{ts,tsx,mts,cts,js,jsx,mjs,cjs}']`), so `.md` is outside it. The
config never enables type-aware linting: its own comment says so, and
there are 0 hits for `projectService` / `parserOptions.project`. So this
diff cannot move a verdict on an untouched file.
- `pnpm check:nul-bytes`: exit 0. A control-byte self-scan of the three
files found nothing.
## Changeset, and a declared deviation
`.changeset/20678-subflow-disable-sequence.md` bumps
`@objectstack/service-automation` `minor`. It carries **BREAKING** under
the launch-window convention, `Clause-②: no (narrowing)`, and the
ADR-0087 marker `not-required (no-migration-prescription)`.
This deviates from the dispatch in two places. The dispatch named a
`patch` changeset, and the claim's line reads `Clause-②: yes`. The
claim's reasoning named both halves: "a disable that is refused today is
accepted, and an enable that is accepted today becomes refusable". Only
the second half lands here, and that is a narrowing with no widening.
`yes` would declare a widening this diff does not contain, and it would
leave the narrowing undeclared. Also, `check-changeset-no-major` grades
`yes` at least `minor`, so `yes` with `patch` cannot pass either way. If
the seat reads it differently, this line and the changeset level are the
two things to change.
## Acceptance notes
- **The registration door can still arm a caller onto a disabled
child.** `registerFlow` (a package upgrade, a publish or a hot reload)
arms a packaged caller whose packaged subflow is disabled. The guard
sits on `toggleFlow` only, the door this card scopes, and the pins use
exactly that path to build their state. Observation, not filed: the
reach through a public door was not measured. Carrier: none.
- **`subflowCallers` on the disable refusal reaches no reader.** The
`DELETE_RESTRICTED` arm in `packages/rest/src/error-response.ts` relays
only `developerMessage` / `dependentObject` / `dependentCount`. The live
409 body above carries no `subflowCallers`, and outside tests nothing
in-process reads it. That is why the enable refusal adds no structured
list beside `code`/`status`. Zero-pull, not filed. Carrier: none.
- **A status-disabled caller.** Re-enabling the ledger bit of a caller
that its own `status` also disables is still judged against its
subflows. The enable would not arm it, but a later publish would, and
the remedy named still completes. Boundary, not filed.
- The Setup packaged-automation board renders the server's message on a
failed switch (per the acceptance run on #20674). How it renders this
409 was not measured in a browser. NOT MEASURED.
- `origin/main` has moved 3 commits since the base (`735594bea`,
`e4e5222b7`, `fa0a4b661`). None touches
`packages/services/service-automation`,
`packages/runtime/src/domains/automation.ts`,
`packages/rest/src/error-response.ts` or the error catalog, so the
branch is not merged forward. CI and the merge queue validate the merge
ref.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01XY5uCwTjZj7884yYtyur4H)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent 10c36cc commit 679f95e
3 files changed
Lines changed: 423 additions & 24 deletions
File tree
- .changeset
- packages/services/service-automation/src
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
4580 | 4580 | | |
4581 | 4581 | | |
4582 | 4582 | | |
4583 | | - | |
4584 | | - | |
4585 | | - | |
4586 | | - | |
4587 | | - | |
4588 | | - | |
4589 | | - | |
| 4583 | + | |
| 4584 | + | |
| 4585 | + | |
| 4586 | + | |
| 4587 | + | |
| 4588 | + | |
| 4589 | + | |
| 4590 | + | |
| 4591 | + | |
| 4592 | + | |
| 4593 | + | |
4590 | 4594 | | |
4591 | 4595 | | |
4592 | 4596 | | |
| |||
4599 | 4603 | | |
4600 | 4604 | | |
4601 | 4605 | | |
| 4606 | + | |
| 4607 | + | |
| 4608 | + | |
| 4609 | + | |
| 4610 | + | |
| 4611 | + | |
| 4612 | + | |
| 4613 | + | |
| 4614 | + | |
| 4615 | + | |
| 4616 | + | |
| 4617 | + | |
| 4618 | + | |
| 4619 | + | |
| 4620 | + | |
| 4621 | + | |
4602 | 4622 | | |
4603 | 4623 | | |
4604 | 4624 | | |
4605 | 4625 | | |
4606 | 4626 | | |
| 4627 | + | |
| 4628 | + | |
| 4629 | + | |
| 4630 | + | |
| 4631 | + | |
| 4632 | + | |
| 4633 | + | |
4607 | 4634 | | |
4608 | 4635 | | |
4609 | 4636 | | |
4610 | 4637 | | |
4611 | 4638 | | |
4612 | 4639 | | |
4613 | | - | |
4614 | | - | |
4615 | | - | |
4616 | | - | |
4617 | | - | |
4618 | | - | |
4619 | | - | |
4620 | | - | |
| 4640 | + | |
4621 | 4641 | | |
4622 | 4642 | | |
4623 | 4643 | | |
4624 | 4644 | | |
| 4645 | + | |
| 4646 | + | |
| 4647 | + | |
| 4648 | + | |
| 4649 | + | |
| 4650 | + | |
| 4651 | + | |
| 4652 | + | |
| 4653 | + | |
| 4654 | + | |
| 4655 | + | |
| 4656 | + | |
| 4657 | + | |
| 4658 | + | |
| 4659 | + | |
| 4660 | + | |
| 4661 | + | |
| 4662 | + | |
| 4663 | + | |
| 4664 | + | |
| 4665 | + | |
| 4666 | + | |
| 4667 | + | |
| 4668 | + | |
| 4669 | + | |
| 4670 | + | |
| 4671 | + | |
| 4672 | + | |
| 4673 | + | |
| 4674 | + | |
| 4675 | + | |
| 4676 | + | |
| 4677 | + | |
| 4678 | + | |
| 4679 | + | |
| 4680 | + | |
| 4681 | + | |
| 4682 | + | |
| 4683 | + | |
| 4684 | + | |
| 4685 | + | |
| 4686 | + | |
| 4687 | + | |
| 4688 | + | |
| 4689 | + | |
| 4690 | + | |
| 4691 | + | |
| 4692 | + | |
| 4693 | + | |
| 4694 | + | |
| 4695 | + | |
| 4696 | + | |
| 4697 | + | |
| 4698 | + | |
| 4699 | + | |
| 4700 | + | |
| 4701 | + | |
| 4702 | + | |
| 4703 | + | |
| 4704 | + | |
| 4705 | + | |
| 4706 | + | |
| 4707 | + | |
| 4708 | + | |
| 4709 | + | |
| 4710 | + | |
| 4711 | + | |
| 4712 | + | |
| 4713 | + | |
| 4714 | + | |
| 4715 | + | |
| 4716 | + | |
| 4717 | + | |
| 4718 | + | |
| 4719 | + | |
| 4720 | + | |
| 4721 | + | |
| 4722 | + | |
| 4723 | + | |
| 4724 | + | |
| 4725 | + | |
| 4726 | + | |
| 4727 | + | |
| 4728 | + | |
| 4729 | + | |
| 4730 | + | |
| 4731 | + | |
| 4732 | + | |
| 4733 | + | |
| 4734 | + | |
| 4735 | + | |
| 4736 | + | |
| 4737 | + | |
| 4738 | + | |
| 4739 | + | |
| 4740 | + | |
| 4741 | + | |
| 4742 | + | |
| 4743 | + | |
| 4744 | + | |
| 4745 | + | |
| 4746 | + | |
| 4747 | + | |
| 4748 | + | |
| 4749 | + | |
| 4750 | + | |
| 4751 | + | |
| 4752 | + | |
| 4753 | + | |
| 4754 | + | |
| 4755 | + | |
| 4756 | + | |
| 4757 | + | |
| 4758 | + | |
| 4759 | + | |
| 4760 | + | |
| 4761 | + | |
| 4762 | + | |
| 4763 | + | |
| 4764 | + | |
| 4765 | + | |
| 4766 | + | |
| 4767 | + | |
| 4768 | + | |
4625 | 4769 | | |
4626 | 4770 | | |
4627 | 4771 | | |
| |||
4647 | 4791 | | |
4648 | 4792 | | |
4649 | 4793 | | |
4650 | | - | |
4651 | | - | |
4652 | | - | |
| 4794 | + | |
| 4795 | + | |
| 4796 | + | |
| 4797 | + | |
4653 | 4798 | | |
4654 | 4799 | | |
4655 | 4800 | | |
4656 | 4801 | | |
4657 | 4802 | | |
4658 | 4803 | | |
4659 | 4804 | | |
4660 | | - | |
4661 | | - | |
4662 | | - | |
| 4805 | + | |
| 4806 | + | |
| 4807 | + | |
| 4808 | + | |
| 4809 | + | |
| 4810 | + | |
| 4811 | + | |
| 4812 | + | |
| 4813 | + | |
4663 | 4814 | | |
4664 | 4815 | | |
4665 | 4816 | | |
| |||
0 commit comments