Skip to content

Commit e5bd768

Browse files
os-zhuangclaude
andauthored
refactor(spec)!: retire ActionDescriptor.isAsync (#6748) and correct supportsPause's TSDoc (#6749) (#6862)
* refactor(spec)!: retire ActionDescriptor.isAsync (#6748) and correct supportsPause's TSDoc (#6749) `ActionDescriptor.isAsync` was a second spelling of `supportsPause` that no execution path read — verified zero readers across objectstack, objectui and cloud at pickup. Its sibling took the enforce leg of ADR-0049 in #6667/PR #6746; this one takes the remove leg. Tombstoned via retiredKey() rather than deleted (ActionDescriptorSchema is not .strict(), so a plain delete would silently strip the key). Registered by exact key in RETIRED_KEYS_BY_MAJOR[17] plus an ADR-0087 D3 SemanticMigration — no D2 conversion, because a descriptor is published from executor TypeScript and never stored in stack metadata, so there is no source for `os migrate meta` to rewrite. The five shipped writers stop writing it; generated baselines, spec-changes, the upgrade guide and reference docs regenerated. Also corrects supportsPause's TSDoc, which still described itself as a declaration no execution path reads — false since #6746 (#6749). Comment-only; no schema, baseline or behaviour change on that key. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011sGk4SKHqGRgmmqUok1P8M * chore(changeset): declare the ADR-0087 disposition on the isAsync retirement (#6148 gate) The changeset declares a breaking change (major + bang), so check-adr-0087-registration requires the ledger question to be answered in writing. It IS registered: the D3 SemanticMigration `action-descriptor-is-async-retired` plus the exact-key RETIRED_KEYS_BY_MAJOR[17] entry both land in this PR. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011sGk4SKHqGRgmmqUok1P8M --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 4cc4fb7 commit e5bd768

19 files changed

Lines changed: 236 additions & 30 deletions

File tree

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
---
2+
'@objectstack/spec': major
3+
'@objectstack/service-automation': patch
4+
'@objectstack/plugin-approvals': patch
5+
---
6+
7+
refactor(spec)!: retire `ActionDescriptor.isAsync` — a second spelling of `supportsPause` that nothing ever read (#6748, ADR-0049)
8+
9+
<!-- adr-0087: registered action-descriptor-is-async-retired -->
10+
11+
**FROM → TO:** `isAsync: true` → delete the key; declare `supportsPause: true` (plus the
12+
`resumeAuthority` its pauses need) and return `suspend: true` from `execute()`.
13+
`isAsync: false` → delete the key; there was never anything to preserve.
14+
15+
`ActionDescriptor.isAsync` declared "suspends the flow awaiting an external reply" and no
16+
execution path read it. Measured fresh before removal across all three repos — objectstack,
17+
objectui and cloud — with zero property reads: every hit was the declaration itself, a
18+
generated baseline, one of five shipped descriptors WRITING it, a fixture pinning the
19+
shape, or prose. Declaring it never made a node suspend; omitting it never stopped one.
20+
21+
This is the remove leg of the ADR-0049 disposition its sibling took the other way. The two
22+
keys said the same thing — "this node type can suspend the run" — and #6667 split them by
23+
evidence: `supportsPause` became an enforced fact (`AutomationEngine` now refuses a
24+
suspension whose type does not declare it, at the one seam every suspension passes
25+
through), while `isAsync` had no consumer to grow into. Keeping both would leave the
26+
platform publishing two names for one capability with only one of them honoured — and
27+
`screen` declared BOTH, so a plugin author copying it had no way to tell which.
28+
29+
The retirement kit:
30+
31+
- **Tombstone, not deletion** (`retiredKey()`): `ActionDescriptorSchema` is not `.strict()`,
32+
so a plain delete would let existing descriptors parse clean and lose the key in silence
33+
(the ADR-0104 shape). Authoring `isAsync` now fails `tsc` at the descriptor literal and
34+
fails the parse inside `defineActionDescriptor()` — with the prescription in the message.
35+
- **ADR-0087 D3 `SemanticMigration`** (`action-descriptor-is-async-retired`) plus the exact
36+
`RETIRED_KEYS_BY_MAJOR` entry. No D2 conversion, deliberately: a descriptor is published
37+
from an executor's TypeScript and never stored in stack metadata, so there is no source
38+
for `os migrate meta` to rewrite — the `EnhancedApiError.fieldErrors` disposition.
39+
- The five shipped writers stop writing it (`screen`, `map`, `wait`, `approval`,
40+
`approval_revise`); the descriptors they publish lose the key, which is why the two
41+
runtime packages appear here.
42+
- Generated baselines (`authorable-surface/automation.json` gains `[RETIRED]`,
43+
`authorable-defaults/automation.json` loses the default line), `spec-changes.json`, the
44+
upgrade guide and the reference docs regenerated.
45+
46+
No runtime behaviour changes — that impossibility is the reason for the removal. The same
47+
commit also corrects `supportsPause`'s TSDoc, which still described itself as a declaration
48+
no execution path reads; #6667 made that false (#6749).

content/docs/references/automation/node-executor.mdx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ Canonical cross-paradigm action/node descriptor (ADR-0018)
7070
| **supportsCancellation** | `boolean` || Supports cancellation |
7171
| **supportsRetry** | `boolean` || Supports retry on failure |
7272
| **needsOutbox** | `boolean` || Dispatch via service-messaging outbox (retry/idempotency/dead-letter) |
73-
| **isAsync** | `boolean` | | Suspends the flow awaiting an external reply |
73+
| **isAsync** | `never` | optional | [REMOVED] `ActionDescriptor.isAsync` was removed in @objectstack/spec 17 (#6748, ADR-0049) — no execution path ever read it, so declaring it never made a node suspend and omitting it never stopped one. Delete the key. The live mechanism is two-part: an executor suspends by RETURNING `suspend: true` from `execute()`, and its descriptor must declare `supportsPause: true` (plus the `resumeAuthority` its pauses need) or the engine refuses that suspension (#6667). Declaring `isAsync: true` alongside `supportsPause: true` was always redundant; declaring it alone was always inert. |
7474
| **handlerContract** | `Enum<'none' \| 'pure'>` || Effect contract for author-supplied code this action invokes: 'none' (invokes none) or 'pure' (must not write — it returns a value and the flow graph persists it) |
7575
| **resumeAuthority** | `Enum<'any' \| 'service'>` | optional | Who may resume a run this node suspended: 'any' (the generic resume route) or 'service' (only the owning service, e.g. approvals). Carries no schema default so an omission stays observable — and an omission is fail-CLOSED at run time, equivalent to 'service': a pausing node whose pause is open to the generic route must declare 'any' explicitly (#5561) |
7676
| **maturity** | `Enum<'ga' \| 'beta' \| 'reserved'>` || Runtime maturity: ga (shipped), beta, or reserved (contract only — designers grey this out) |

docs/protocol-upgrade-guide.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,8 @@ Finally it narrows the aggregation vocabulary: `array_agg` and `string_agg` leav
222222

223223
One entry in this step is not a removal at all but a SECURE-DEFAULT FLIP, the shape protocol 12 last used for `api.requireAuth`: an omitted `ActionDescriptor.resumeAuthority` resolves to `'service'` instead of `'any'`, so a pausing node type that never states who may continue its pauses is refused on the generic resume route rather than open to it (#5561, ADR-0044's 2026-07-28 amendment). Nothing is removed and no metadata shape changes — the field has been optional since step one of the same issue — so tsc reports nothing and only the MEANING of silence moved. That is exactly why it needs a ledger entry: a third-party plugin author has no compile error to discover it with, and the one-line prescription (declare `resumeAuthority` on the descriptor) has to arrive before a user meets a run that will not continue.
224224

225+
The same descriptor loses a key in this step, and the pairing is the point (#6748, ADR-0049). `ActionDescriptor.isAsync` and `ActionDescriptor.supportsPause` were two spellings of one capability — "this node type can suspend the run" — and #6667 split them by evidence rather than by preference: `supportsPause` took the ENFORCE leg (the engine now refuses a suspension the descriptor never declared, at the one seam every suspension passes through), and `isAsync` takes the REMOVE leg, because a fresh three-repo measurement found zero readers and no consumer it could grow into. What makes the duplicate worse than an ordinary inert key is that five shipped descriptors WROTE it, so the platform itself modelled a declaration that decided nothing — and a plugin author copying `screen` (which declared BOTH) had no way to tell which of the two the runtime honoured. It is tombstoned rather than deleted, so the answer arrives as a rejection carrying the fix; and because a descriptor lives in executor TypeScript rather than in stored metadata, its prescription is a semantic entry below rather than a conversion `os migrate meta` could replay.
226+
225227
### Mechanical (applied for you)
226228

227229
| Conversion | Surface | Change | Load window |
@@ -396,6 +398,9 @@ One entry in this step is not a removal at all but a SECURE-DEFAULT FLIP, the sh
396398
- **`export-field-meta-constraints-retired`**`@objectstack/rest: ExportFieldMeta.required / .system / .readonly / .hasDefault / .min / .max / .minLength / .maxLength (the map built by `buildFieldMetaMap`, reached as `PreparedImport.metaMap` from `prepareImportRequest`)` → the object schema you already hold — read `fields[name].required` / `.system` / `.readonly` / `.defaultValue` / `.min` / `.max` / `.minLength` / `.maxLength` off the same `ObjectSchema` you passed to `buildFieldMetaMap`, which is where the ENGINE reads them and therefore the only copy that cannot drift
397399
- Why not automatic: ADR-0049 enforce-or-remove. These eight were never a source of truth: `buildFieldMetaMap(schema)` DERIVED each one from the very `schema` its caller passed in, so the map carried a second copy of facts the caller already held. They existed for exactly one consumer — the import dry run's hand-copied pre-check mirror (`firstMissingRequiredField` / `firstConstraintViolation`, framework#3956) — and #4633 ruling D retired that mirror (PR #6532): the dry run now asks `DataProtocol.validateData` for the engine's verdict, which reads the object's own schema. That left all eight computed on every import and read by NOTHING, which is the declared-and-unread shape ADR-0049 exists for; a constraint vocabulary standing next to the presentation one with no enforcer behind it is precisely the thing an AI-authored consumer mistakes for a contract. Verified zero-reader before removal, per key and by type, across this repo (`packages/rest` itself, and all five in-repo dependents of `@objectstack/rest`: runtime, cli, verify, plugin-auth, plugin-dev) and the `objectui` sibling; plugin-auth's identity import forwards `prepared.metaMap` into `runImport` but reads only the presentation keys through `coerceRow`. Why this needs a ledger entry despite that sweep: it is the `findStream` (#4484) / `IStorageService.list` (#5540) / `actor-user-roles-to-positions` (#6011) disposition — a published TS surface with NO spec schema, so there is no `retiredKey()` tombstone and no parse rejection that could carry a prescription, and the ledger is the only channel that reaches an upgrader. It is if anything blinder than those three: the keys shipped in a FINAL release (`@objectstack/rest` 14.5.0) and have been published in every release since, and because they were OPTIONAL keys on an interface that itself survives, a JavaScript consumer reading `meta.required` after the upgrade gets `undefined` with no error at all — tsc reports at the read site only for a typed consumer. Why D3 semantic and not a D2 conversion: there is nothing to convert. No authored or stored metadata changes shape — `required` / `min` / `maxLength` and the rest remain fully authorable on a field definition and fully enforced by the engine, which is where they always lived. The only place these eight are ever spelled is inside a consumer's own TypeScript, so no `objectstack migrate meta` transform can reach them. ADR-0049 / ADR-0087, #6536 (the sweep PR #6532 deliberately deferred).
398400
- Done when: No code of yours reads any of the eight off a `buildFieldMetaMap` / `prepareImportRequest` result. Grep your sources for `.required` / `.hasDefault` / `.minLength` / `.maxLength` / `.min` / `.max` / `.system` / `.readonly` on an `ExportFieldMeta`-typed value; each hit moves to the object schema you already passed in. ⚠️ Prove it against a RUN, not against tsc: these were optional keys, so an untyped or `any`-typed read compiles clean and silently becomes `undefined` — assert that the constraint your code acts on is still observed on a real import, not merely that the build is green. Note `hasDefault` has no one-to-one replacement key: it was the derived predicate `defaultValue != null`, mirroring the engine's `applyFieldDefaults` gate, so read `fields[name].defaultValue` and apply that same `!= null` test yourself.
401+
- **`action-descriptor-is-async-retired`**`ActionDescriptor.isAsync (the descriptor an executor publishes via `registerNodeExecutor` / `defineActionDescriptor`)` → nothing to re-declare — delete the key. Suspension is `execute()` RETURNING `suspend: true`, and permission to suspend is `supportsPause: true` on the same descriptor (with the `resumeAuthority` its pauses need)
402+
- Why not automatic: ADR-0049 enforce-or-remove. `isAsync` declared "this action suspends the flow awaiting an external reply" and NOTHING read it: a fresh three-repo measurement (#6748, re-run at pickup) found zero property reads across objectstack, objectui and cloud — every hit was the declaration itself, a generated baseline, one of five shipped descriptors WRITING it, a test fixture pinning the shape, or prose. So declaring it never made a node suspend and omitting it never stopped one, which is the silently-inert declaration ADR-0049 exists to end. It was always a second, weaker spelling of the capability `supportsPause` states, and the two diverged in exactly the way a duplicated declaration does: `screen` declared both, `map` and `wait` declared `isAsync` alongside `supportsPause`, and nothing anywhere reconciled them. The sibling took the ENFORCE leg of the same ruling in #6667 — `AutomationEngine` now refuses a suspension whose type does not declare `supportsPause: true` — so the capability this key gestured at is now a real, enforced fact under one name. This one had no consumer to grow into and takes the remove leg. Why D3 semantic and not a D2 conversion: an ActionDescriptor is published from an executor's TypeScript, never stored in stack metadata — no stack, example or template carries the key — so there is no source for the chain to rewrite and `os migrate meta` cannot reach it. The schema tombstones it via `retiredKey()` and descriptor authors delete the key themselves; that rejection (a `tsc` error at the authoring site, and a parse error inside `defineActionDescriptor`) is the channel a third-party plugin author actually meets. The `EnhancedApiError.fieldErrors` disposition, one layer down.
403+
- Done when: No descriptor declares `isAsync` — not the five that shipped it (`screen`, `map`, `wait`, `approval`, `approval_revise`), not a plugin's. Every node type that returns `suspend: true` from `execute()` declares `supportsPause: true` on its descriptor together with a `resumeAuthority`, and its runs still pause and resume as before: the behaviour never depended on `isAsync`, so deleting the key changes no run. Authoring `isAsync` fails `tsc` at the descriptor literal and fails `defineActionDescriptor()` at runtime with the prescription, instead of parsing clean and being stripped.
399404

400405
---
401406

packages/plugins/plugin-approvals/src/approval-node.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -111,8 +111,7 @@ export function registerApprovalNode(
111111
paradigms: ['flow'],
112112
source: 'plugin',
113113
// Human decision: the run suspends here awaiting an external reply.
114-
supportsPause: true,
115-
isAsync: true,
114+
supportsPause: true, // (`isAsync` stood here — retired in #6748, ADR-0049: nothing read it)
116115
// #3801: this pause is NOT resumable through the generic run-resume
117116
// route. Continuing an approval is a side effect of a DECISION, and the
118117
// decision is the thing that must be authorized (the approver slate),

packages/plugins/plugin-approvals/src/approval-revise-node.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,8 +76,7 @@ export function registerApprovalReviseNode(
7676
category: 'human',
7777
paradigms: ['flow'],
7878
source: 'plugin',
79-
supportsPause: true,
80-
isAsync: true,
79+
supportsPause: true, // (`isAsync` stood here — retired in #6748, ADR-0049: nothing read it)
8180
// #3823 / amended ADR-0044: THE point of this node type. The revise
8281
// window is a service-owned continuation, so the #3801 gate must refuse
8382
// a raw resume of it — which it does for any node type declaring this.

packages/plugins/plugin-approvals/src/approval-revise.test.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -448,8 +448,13 @@ describe('Send back for revision (ADR-0044)', () => {
448448
type: APPROVAL_REVISE_NODE_TYPE,
449449
resumeAuthority: 'service',
450450
supportsPause: true,
451-
isAsync: true,
452451
});
452+
// `isAsync: true` stood alongside those two until #6748 retired it
453+
// (ADR-0049, zero readers). Asserted as ABSENT rather than dropped: the
454+
// line above is `toMatchObject`, which would go on passing if the key
455+
// came back, and the point of the retirement is that the descriptor
456+
// publishes ONE spelling of "this type can suspend".
457+
expect(descriptor).not.toHaveProperty('isAsync');
453458
// The generic `wait` stays open to its external producer — this fix must
454459
// not gate every author-placed wait in the system.
455460
expect(automation.getActionDescriptors().find(d => d.type === 'wait')?.resumeAuthority)

packages/services/service-automation/src/builtin/http-nodes.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,10 @@ import { parseNodeConfig } from './parse-config.js';
2828
* - **Request/response (default)** — a synchronous `fetch()` returning
2929
* `{ response, status }` to the flow, preserving the historical `http_request`
3030
* behavior so existing flows that read the response keep working. (The ADR's
31-
* `isAsync` suspend-and-resume variant is future work.)
31+
* suspend-and-resume variant is future work. ADR-0018 called it the `isAsync`
32+
* path; that descriptor key was retired in #6748 — a suspending HTTP node
33+
* would declare `supportsPause: true` plus a `resumeAuthority` and return
34+
* `suspend: true`, which is the mechanism the engine actually enforces.)
3235
*/
3336

3437
/** Structural view of `service-messaging`'s HTTP outbox surface (ADR-0018 M3). */

packages/services/service-automation/src/builtin/map-node.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,7 @@ export function registerMapNode(engine: AutomationEngine, ctx: PluginContext): v
5050
category: 'logic',
5151
source: 'builtin',
5252
// Each item's subflow may pause, so the map suspends and resumes per item.
53-
supportsPause: true,
54-
isAsync: true,
53+
supportsPause: true, // (`isAsync` stood here — retired in #6748, ADR-0049: nothing read it)
5554
// As with `subflow`, `'any'` here is not the authority that applies: the
5655
// #3801 gate follows the `map:` correlation to the in-flight item's child
5756
// run and judges that node instead — judging the loop rather than the item

packages/services/service-automation/src/builtin/screen-nodes.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ export function registerScreenNodes(engine: AutomationEngine, ctx: PluginContext
4747
description: 'Collect user input via a screen (human-input element).',
4848
icon: 'window', category: 'human', source: 'builtin',
4949
// Human-input nodes suspend the flow awaiting input.
50-
supportsPause: true, isAsync: true,
50+
supportsPause: true, // (`isAsync` stood here — retired in #6748, ADR-0049: nothing read it)
5151
// The generic resume route IS this node's intended door: the flow-runner
5252
// collects the inputs and hands them back as the continuation, so there
5353
// is no service decision to route around (#3801). Stated rather than

packages/services/service-automation/src/builtin/wait-node.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -189,8 +189,7 @@ export function registerWaitNode(engine: AutomationEngine, ctx: PluginContext):
189189
category: 'logic',
190190
source: 'builtin',
191191
// Durable pause — the run suspends and resumes later (timer/signal).
192-
supportsPause: true,
193-
isAsync: true,
192+
supportsPause: true, // (`isAsync` stood here — retired in #6748, ADR-0049: nothing read it)
194193
// An external producer is *meant* to resume a signal wait, so the generic
195194
// route is the door (#3801). Stated rather than inherited from a default:
196195
// #3823 is what inheriting it costs — ADR-0044 pointed a revise edge at a

0 commit comments

Comments
 (0)