From eceb4854709678857f3ca8139eeaff59420b06c4 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 9 Aug 2026 01:05:09 +0000 Subject: [PATCH 1/2] refactor(spec)!: retire ActionDescriptor.isAsync (#6748) and correct supportsPause's TSDoc (#6749) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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 Claude-Session: https://claude.ai/code/session_011sGk4SKHqGRgmmqUok1P8M --- .../action-descriptor-is-async-retired.md | 46 +++++++++++++ .../references/automation/node-executor.mdx | 2 +- docs/protocol-upgrade-guide.md | 5 ++ .../plugin-approvals/src/approval-node.ts | 3 +- .../src/approval-revise-node.ts | 3 +- .../src/approval-revise.test.ts | 7 +- .../src/builtin/http-nodes.ts | 5 +- .../src/builtin/map-node.ts | 3 +- .../src/builtin/screen-nodes.ts | 2 +- .../src/builtin/wait-node.ts | 3 +- .../service-automation/src/engine.test.ts | 3 +- .../src/plugin-suspended-run-wiring.test.ts | 2 +- .../src/resume-authority-gate.test.ts | 2 +- .../spec/authorable-defaults/automation.json | 1 - .../spec/authorable-surface/automation.json | 2 +- packages/spec/spec-changes.json | 14 ++++ .../spec/src/automation/node-executor.test.ts | 30 +++++++++ .../spec/src/automation/node-executor.zod.ts | 65 ++++++++++++++---- packages/spec/src/migrations/registry.ts | 66 ++++++++++++++++++- 19 files changed, 234 insertions(+), 30 deletions(-) create mode 100644 .changeset/action-descriptor-is-async-retired.md diff --git a/.changeset/action-descriptor-is-async-retired.md b/.changeset/action-descriptor-is-async-retired.md new file mode 100644 index 0000000000..08fbd34613 --- /dev/null +++ b/.changeset/action-descriptor-is-async-retired.md @@ -0,0 +1,46 @@ +--- +'@objectstack/spec': major +'@objectstack/service-automation': patch +'@objectstack/plugin-approvals': patch +--- + +refactor(spec)!: retire `ActionDescriptor.isAsync` — a second spelling of `supportsPause` that nothing ever read (#6748, ADR-0049) + +**FROM → TO:** `isAsync: true` → delete the key; declare `supportsPause: true` (plus the +`resumeAuthority` its pauses need) and return `suspend: true` from `execute()`. +`isAsync: false` → delete the key; there was never anything to preserve. + +`ActionDescriptor.isAsync` declared "suspends the flow awaiting an external reply" and no +execution path read it. Measured fresh before removal across all three repos — objectstack, +objectui and cloud — with zero property reads: every hit was the declaration itself, a +generated baseline, one of five shipped descriptors WRITING it, a fixture pinning the +shape, or prose. Declaring it never made a node suspend; omitting it never stopped one. + +This is the remove leg of the ADR-0049 disposition its sibling took the other way. The two +keys said the same thing — "this node type can suspend the run" — and #6667 split them by +evidence: `supportsPause` became an enforced fact (`AutomationEngine` now refuses a +suspension whose type does not declare it, at the one seam every suspension passes +through), while `isAsync` had no consumer to grow into. Keeping both would leave the +platform publishing two names for one capability with only one of them honoured — and +`screen` declared BOTH, so a plugin author copying it had no way to tell which. + +The retirement kit: + +- **Tombstone, not deletion** (`retiredKey()`): `ActionDescriptorSchema` is not `.strict()`, + so a plain delete would let existing descriptors parse clean and lose the key in silence + (the ADR-0104 shape). Authoring `isAsync` now fails `tsc` at the descriptor literal and + fails the parse inside `defineActionDescriptor()` — with the prescription in the message. +- **ADR-0087 D3 `SemanticMigration`** (`action-descriptor-is-async-retired`) plus the exact + `RETIRED_KEYS_BY_MAJOR` entry. No D2 conversion, deliberately: a descriptor is published + from an executor's TypeScript and never stored in stack metadata, so there is no source + for `os migrate meta` to rewrite — the `EnhancedApiError.fieldErrors` disposition. +- The five shipped writers stop writing it (`screen`, `map`, `wait`, `approval`, + `approval_revise`); the descriptors they publish lose the key, which is why the two + runtime packages appear here. +- Generated baselines (`authorable-surface/automation.json` gains `[RETIRED]`, + `authorable-defaults/automation.json` loses the default line), `spec-changes.json`, the + upgrade guide and the reference docs regenerated. + +No runtime behaviour changes — that impossibility is the reason for the removal. The same +commit also corrects `supportsPause`'s TSDoc, which still described itself as a declaration +no execution path reads; #6667 made that false (#6749). diff --git a/content/docs/references/automation/node-executor.mdx b/content/docs/references/automation/node-executor.mdx index d0e42ad243..59836fa45b 100644 --- a/content/docs/references/automation/node-executor.mdx +++ b/content/docs/references/automation/node-executor.mdx @@ -70,7 +70,7 @@ Canonical cross-paradigm action/node descriptor (ADR-0018) | **supportsCancellation** | `boolean` | ✅ | Supports cancellation | | **supportsRetry** | `boolean` | ✅ | Supports retry on failure | | **needsOutbox** | `boolean` | ✅ | Dispatch via service-messaging outbox (retry/idempotency/dead-letter) | -| **isAsync** | `boolean` | ✅ | Suspends the flow awaiting an external reply | +| **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. | | **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) | | **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) | | **maturity** | `Enum<'ga' \| 'beta' \| 'reserved'>` | ✅ | Runtime maturity: ga (shipped), beta, or reserved (contract only — designers grey this out) | diff --git a/docs/protocol-upgrade-guide.md b/docs/protocol-upgrade-guide.md index b09fc54aed..f5c1680452 100644 --- a/docs/protocol-upgrade-guide.md +++ b/docs/protocol-upgrade-guide.md @@ -222,6 +222,8 @@ Finally it narrows the aggregation vocabulary: `array_agg` and `string_agg` leav 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. +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. + ### Mechanical (applied for you) | Conversion | Surface | Change | Load window | @@ -395,6 +397,9 @@ One entry in this step is not a removal at all but a SECURE-DEFAULT FLIP, the sh - **`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 - 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). - 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. +- **`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) + - 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. + - 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. --- diff --git a/packages/plugins/plugin-approvals/src/approval-node.ts b/packages/plugins/plugin-approvals/src/approval-node.ts index 5a0786a7be..a6be011a25 100644 --- a/packages/plugins/plugin-approvals/src/approval-node.ts +++ b/packages/plugins/plugin-approvals/src/approval-node.ts @@ -111,8 +111,7 @@ export function registerApprovalNode( paradigms: ['flow'], source: 'plugin', // Human decision: the run suspends here awaiting an external reply. - supportsPause: true, - isAsync: true, + supportsPause: true, // (`isAsync` stood here — retired in #6748, ADR-0049: nothing read it) // #3801: this pause is NOT resumable through the generic run-resume // route. Continuing an approval is a side effect of a DECISION, and the // decision is the thing that must be authorized (the approver slate), diff --git a/packages/plugins/plugin-approvals/src/approval-revise-node.ts b/packages/plugins/plugin-approvals/src/approval-revise-node.ts index 91302ad87f..f24e4e4c91 100644 --- a/packages/plugins/plugin-approvals/src/approval-revise-node.ts +++ b/packages/plugins/plugin-approvals/src/approval-revise-node.ts @@ -76,8 +76,7 @@ export function registerApprovalReviseNode( category: 'human', paradigms: ['flow'], source: 'plugin', - supportsPause: true, - isAsync: true, + supportsPause: true, // (`isAsync` stood here — retired in #6748, ADR-0049: nothing read it) // #3823 / amended ADR-0044: THE point of this node type. The revise // window is a service-owned continuation, so the #3801 gate must refuse // a raw resume of it — which it does for any node type declaring this. diff --git a/packages/plugins/plugin-approvals/src/approval-revise.test.ts b/packages/plugins/plugin-approvals/src/approval-revise.test.ts index ff412b89dc..a975a9157e 100644 --- a/packages/plugins/plugin-approvals/src/approval-revise.test.ts +++ b/packages/plugins/plugin-approvals/src/approval-revise.test.ts @@ -448,8 +448,13 @@ describe('Send back for revision (ADR-0044)', () => { type: APPROVAL_REVISE_NODE_TYPE, resumeAuthority: 'service', supportsPause: true, - isAsync: true, }); + // `isAsync: true` stood alongside those two until #6748 retired it + // (ADR-0049, zero readers). Asserted as ABSENT rather than dropped: the + // line above is `toMatchObject`, which would go on passing if the key + // came back, and the point of the retirement is that the descriptor + // publishes ONE spelling of "this type can suspend". + expect(descriptor).not.toHaveProperty('isAsync'); // The generic `wait` stays open to its external producer — this fix must // not gate every author-placed wait in the system. expect(automation.getActionDescriptors().find(d => d.type === 'wait')?.resumeAuthority) diff --git a/packages/services/service-automation/src/builtin/http-nodes.ts b/packages/services/service-automation/src/builtin/http-nodes.ts index a8346ee36c..13e9ef48e3 100644 --- a/packages/services/service-automation/src/builtin/http-nodes.ts +++ b/packages/services/service-automation/src/builtin/http-nodes.ts @@ -28,7 +28,10 @@ import { parseNodeConfig } from './parse-config.js'; * - **Request/response (default)** — a synchronous `fetch()` returning * `{ response, status }` to the flow, preserving the historical `http_request` * behavior so existing flows that read the response keep working. (The ADR's - * `isAsync` suspend-and-resume variant is future work.) + * suspend-and-resume variant is future work. ADR-0018 called it the `isAsync` + * path; that descriptor key was retired in #6748 — a suspending HTTP node + * would declare `supportsPause: true` plus a `resumeAuthority` and return + * `suspend: true`, which is the mechanism the engine actually enforces.) */ /** Structural view of `service-messaging`'s HTTP outbox surface (ADR-0018 M3). */ diff --git a/packages/services/service-automation/src/builtin/map-node.ts b/packages/services/service-automation/src/builtin/map-node.ts index 330e8ff78f..79ccc472dc 100644 --- a/packages/services/service-automation/src/builtin/map-node.ts +++ b/packages/services/service-automation/src/builtin/map-node.ts @@ -50,8 +50,7 @@ export function registerMapNode(engine: AutomationEngine, ctx: PluginContext): v category: 'logic', source: 'builtin', // Each item's subflow may pause, so the map suspends and resumes per item. - supportsPause: true, - isAsync: true, + supportsPause: true, // (`isAsync` stood here — retired in #6748, ADR-0049: nothing read it) // As with `subflow`, `'any'` here is not the authority that applies: the // #3801 gate follows the `map:` correlation to the in-flight item's child // run and judges that node instead — judging the loop rather than the item diff --git a/packages/services/service-automation/src/builtin/screen-nodes.ts b/packages/services/service-automation/src/builtin/screen-nodes.ts index 2015944299..180b6453d9 100644 --- a/packages/services/service-automation/src/builtin/screen-nodes.ts +++ b/packages/services/service-automation/src/builtin/screen-nodes.ts @@ -47,7 +47,7 @@ export function registerScreenNodes(engine: AutomationEngine, ctx: PluginContext description: 'Collect user input via a screen (human-input element).', icon: 'window', category: 'human', source: 'builtin', // Human-input nodes suspend the flow awaiting input. - supportsPause: true, isAsync: true, + supportsPause: true, // (`isAsync` stood here — retired in #6748, ADR-0049: nothing read it) // The generic resume route IS this node's intended door: the flow-runner // collects the inputs and hands them back as the continuation, so there // is no service decision to route around (#3801). Stated rather than diff --git a/packages/services/service-automation/src/builtin/wait-node.ts b/packages/services/service-automation/src/builtin/wait-node.ts index fb1b80b1f4..4ab55194e2 100644 --- a/packages/services/service-automation/src/builtin/wait-node.ts +++ b/packages/services/service-automation/src/builtin/wait-node.ts @@ -189,8 +189,7 @@ export function registerWaitNode(engine: AutomationEngine, ctx: PluginContext): category: 'logic', source: 'builtin', // Durable pause — the run suspends and resumes later (timer/signal). - supportsPause: true, - isAsync: true, + supportsPause: true, // (`isAsync` stood here — retired in #6748, ADR-0049: nothing read it) // An external producer is *meant* to resume a signal wait, so the generic // route is the door (#3801). Stated rather than inherited from a default: // #3823 is what inheriting it costs — ADR-0044 pointed a revise edge at a diff --git a/packages/services/service-automation/src/engine.test.ts b/packages/services/service-automation/src/engine.test.ts index b57473a411..80f82fa784 100644 --- a/packages/services/service-automation/src/engine.test.ts +++ b/packages/services/service-automation/src/engine.test.ts @@ -2592,7 +2592,6 @@ describe('Action Descriptor Registry (ADR-0018)', () => { supportsCancellation: false, supportsRetry: true, needsOutbox: true, - isAsync: false, source: 'plugin', deprecated: false, maturity: 'ga', @@ -2616,7 +2615,7 @@ describe('Action Descriptor Registry (ADR-0018)', () => { type: 'send_sms', version: '1.0.0', name: 'Send SMS', category: 'io', paradigms: ['flow'], supportsPause: false, supportsCancellation: false, supportsRetry: true, - needsOutbox: false, isAsync: false, source: 'plugin', deprecated: false, maturity: 'ga', + needsOutbox: false, source: 'plugin', deprecated: false, maturity: 'ga', }, async execute() { return { success: true }; }, }); diff --git a/packages/services/service-automation/src/plugin-suspended-run-wiring.test.ts b/packages/services/service-automation/src/plugin-suspended-run-wiring.test.ts index 2730042922..78a8a88bf4 100644 --- a/packages/services/service-automation/src/plugin-suspended-run-wiring.test.ts +++ b/packages/services/service-automation/src/plugin-suspended-run-wiring.test.ts @@ -120,7 +120,7 @@ async function pauseARun(engine: AutomationEngine) { type: 'test_pause', descriptor: defineActionDescriptor({ type: 'test_pause', version: '1.0.0', name: 'Test Pause', - supportsPause: true, isAsync: true, + supportsPause: true, }), async execute() { return { success: true, suspend: true, correlation: 'areq_1' }; }, }); diff --git a/packages/services/service-automation/src/resume-authority-gate.test.ts b/packages/services/service-automation/src/resume-authority-gate.test.ts index e044d3b752..19c14683e3 100644 --- a/packages/services/service-automation/src/resume-authority-gate.test.ts +++ b/packages/services/service-automation/src/resume-authority-gate.test.ts @@ -64,7 +64,7 @@ function registerPausers(engine: AutomationEngine): void { type: 'gated_pause', descriptor: defineActionDescriptor({ type: 'gated_pause', version: '1.0.0', name: 'Gated Pause', - supportsPause: true, isAsync: true, resumeAuthority: 'service', + supportsPause: true, resumeAuthority: 'service', }), async execute() { return { success: true, suspend: true, correlation: 'req_1' }; }, }); diff --git a/packages/spec/authorable-defaults/automation.json b/packages/spec/authorable-defaults/automation.json index 7b62499363..57526ccbdc 100644 --- a/packages/spec/authorable-defaults/automation.json +++ b/packages/spec/authorable-defaults/automation.json @@ -5,7 +5,6 @@ "automation/ActionDescriptor:category = \"custom\"", "automation/ActionDescriptor:deprecated = false", "automation/ActionDescriptor:handlerContract = \"none\"", - "automation/ActionDescriptor:isAsync = false", "automation/ActionDescriptor:maturity = \"ga\"", "automation/ActionDescriptor:needsOutbox = false", "automation/ActionDescriptor:paradigms = [\"flow\"]", diff --git a/packages/spec/authorable-surface/automation.json b/packages/spec/authorable-surface/automation.json index 961510ed77..1da490999a 100644 --- a/packages/spec/authorable-surface/automation.json +++ b/packages/spec/authorable-surface/automation.json @@ -9,7 +9,7 @@ "automation/ActionDescriptor:description", "automation/ActionDescriptor:handlerContract", "automation/ActionDescriptor:icon", - "automation/ActionDescriptor:isAsync", + "automation/ActionDescriptor:isAsync [RETIRED]", "automation/ActionDescriptor:maturity", "automation/ActionDescriptor:name", "automation/ActionDescriptor:needsOutbox", diff --git a/packages/spec/spec-changes.json b/packages/spec/spec-changes.json index 90b17a99fe..14f86a9e64 100644 --- a/packages/spec/spec-changes.json +++ b/packages/spec/spec-changes.json @@ -706,6 +706,13 @@ "migrationId": "export-field-meta-constraints-retired", "toMajor": 17, "rationale": "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)." + }, + { + "surface": "ActionDescriptor.isAsync (the descriptor an executor publishes via `registerNodeExecutor` / `defineActionDescriptor`)", + "replacement": "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)", + "migrationId": "action-descriptor-is-async-retired", + "toMajor": 17, + "rationale": "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." } ], "removed": [] @@ -1471,6 +1478,13 @@ "migrationId": "export-field-meta-constraints-retired", "toMajor": 17, "rationale": "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)." + }, + { + "surface": "ActionDescriptor.isAsync (the descriptor an executor publishes via `registerNodeExecutor` / `defineActionDescriptor`)", + "replacement": "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)", + "migrationId": "action-descriptor-is-async-retired", + "toMajor": 17, + "rationale": "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." } ], "removed": [] diff --git a/packages/spec/src/automation/node-executor.test.ts b/packages/spec/src/automation/node-executor.test.ts index 9d9971e43a..63396ea652 100644 --- a/packages/spec/src/automation/node-executor.test.ts +++ b/packages/spec/src/automation/node-executor.test.ts @@ -308,3 +308,33 @@ describe('ActionDescriptorSchema.handlerContract', () => { expect(() => ActionDescriptorSchema.parse({ ...base, handlerContract: 'sandboxed' })).toThrow(); }); }); + +// --------------------------------------------------------------------------- +// ActionDescriptorSchema — retired `isAsync` (#6748, ADR-0049) +// --------------------------------------------------------------------------- +describe('ActionDescriptorSchema.isAsync retirement', () => { + const base = { type: 'demo', version: '1.0.0', name: 'Demo' }; + + it('REJECTS an authored isAsync, with the live mechanism in the message', () => { + // Tombstoned, not deleted: ActionDescriptorSchema is not `.strict()`, so a + // plain deletion would let the five shipped descriptors (and every plugin + // one) keep writing the key, parse clean, and lose it in silence — the + // ADR-0104 silent-strip shape. `retiredKey()` makes it audible. + expect(() => ActionDescriptorSchema.parse({ ...base, isAsync: true })) + .toThrow(/ActionDescriptor\.isAsync.*removed.*supportsPause: true/s); + }); + + it('rejects `isAsync: false` too — absence is the only accepted spelling', () => { + // The realistic upgrade shape: a descriptor that spelled out the schema + // default. It must not slip through as "well, that was the default anyway". + expect(() => ActionDescriptorSchema.parse({ ...base, isAsync: false })) + .toThrow(/ActionDescriptor\.isAsync.*removed/s); + }); + + it('parses cleanly once the key is gone, and publishes no isAsync of its own', () => { + const desc = defineActionDescriptor({ ...base, supportsPause: true, resumeAuthority: 'any' }); + expect(desc).not.toHaveProperty('isAsync'); + // The capability the key gestured at survives under its ONE enforced name. + expect(desc.supportsPause).toBe(true); + }); +}); diff --git a/packages/spec/src/automation/node-executor.zod.ts b/packages/spec/src/automation/node-executor.zod.ts index 9bf0c8742e..84c944342d 100644 --- a/packages/spec/src/automation/node-executor.zod.ts +++ b/packages/spec/src/automation/node-executor.zod.ts @@ -24,6 +24,7 @@ import { z } from 'zod'; * Mirrors the `waitEventConfig.eventType` in flow.zod.ts. */ import { lazySchema } from '../shared/lazy-schema'; +import { retiredKey } from '../shared/retired-key'; export const WaitEventTypeSchema = lazySchema(() => z.enum([ 'timer', // Resume after duration/datetime 'signal', // Resume on named signal dispatch @@ -285,14 +286,36 @@ export const ActionDescriptorSchema = lazySchema(() => z.object({ /** * Supports async pause/resume (e.g. wait, human_task). * - * **A declaration, not an enforced fact** (#5703). No execution path reads - * it: a run pauses because the executor's `execute()` returned - * `suspend: true`, and the #3801 resume gate keys on the suspended node's - * `resumeAuthority` alone. What it does drive is authoring-time: the designer - * palette, the registration warning below, and the - * `check:resume-authority-declared` gate. So an executor that suspends - * while leaving this `false` is invisible to both of those — #5703 tracks - * closing that seam. + * **Declared = enforced, since #6667** (which closed the #5703 seam). This + * was a declaration no execution path read until then; `AutomationEngine` + * now enforces it at the ONE seam every suspension passes through, so the + * three authoring-time consumers it always had — the designer palette, the + * registration warning below, and the `check:resume-authority-declared` + * gate — are no longer the whole of its effect. + * + * What the runtime half does, and the boundary #6667 drew deliberately: + * + * - **Mismatch is refused.** A node type whose descriptor declares + * `supportsPause: false` — or OMITS the key, which parses to the same + * `false` by the default above — and whose executor returns + * `suspend: true` has that suspension REFUSED. It is a guard-class + * refusal: a metadata defect, not a runtime one, so a `fault` edge does + * not route it, and nothing durable is written for a pause nothing could + * have continued (a type declaring no pause declares no `resumeAuthority` + * either, and an unclaimed pause is fail-closed since #5561). + * - **The inverse is legal.** `supportsPause: true` on a type that never + * suspends is not a mismatch — the declaration is a CAPABILITY, not an + * obligation. `wait` legitimately returns without suspending when its + * condition is already met. + * - **Silence is not `false`.** An executor that publishes NO descriptor + * declares nothing for this gate to enforce (`NodeExecutor.descriptor` is + * optional by contract), so its pauses are not refused here; they stay + * governed by #5561's resume gate, which refuses an undeclared + * `resumeAuthority` at the resume end instead. + * + * Alias hop included: the descriptor consulted is the canonical one a + * deprecated ADR-0018 alias forwards to, never the synthesized alias + * descriptor carrying schema defaults. */ supportsPause: z.boolean().default(false).describe('Supports async pause/resume'), /** Supports mid-execution cancellation. */ @@ -302,9 +325,29 @@ export const ActionDescriptorSchema = lazySchema(() => z.object({ /** Dispatch through the ADR-0012 service-messaging outbox. */ needsOutbox: z.boolean().default(false) .describe('Dispatch via service-messaging outbox (retry/idempotency/dead-letter)'), - /** Request/response action that suspends the flow until a reply. */ - isAsync: z.boolean().default(false) - .describe('Suspends the flow awaiting an external reply'), + /** + * Tombstoned, not deleted (ADR-0104): `ActionDescriptorSchema` is not + * `.strict()`, so a plain deletion would let the five shipped descriptors — + * and every third-party one — keep writing `isAsync`, parse clean, and lose + * the value in silence. `retiredKey()` turns that into a rejection carrying + * the fix, in both channels a descriptor author meets: `tsc` (the input type + * is `never`) and the parse `defineActionDescriptor()` runs. + * + * Retired under ADR-0049 enforce-or-remove (#6748): a fresh three-repo + * measurement found ZERO readers. `isAsync` was a second, weaker spelling of + * the thing `supportsPause` above now enforces — and where `supportsPause` + * grew a runtime consumer in #6667, this one never had one to grow into, so + * it took the remove leg of the same ruling rather than the enforce leg. + */ + isAsync: retiredKey( + '`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.', + ), /** * The effect contract this action places on the AUTHOR-SUPPLIED code it diff --git a/packages/spec/src/migrations/registry.ts b/packages/spec/src/migrations/registry.ts index 3ee2712346..d4f9e7e19b 100644 --- a/packages/spec/src/migrations/registry.ts +++ b/packages/spec/src/migrations/registry.ts @@ -1138,7 +1138,21 @@ const step17: MigrationStep = { + '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.', + + 'a user meets a run that will not continue.\n\n' + + '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.', conversionIds: [ 'action-execute-to-target', 'field-conditionalRequired-to-requiredWhen', @@ -2804,6 +2818,47 @@ const step17: MigrationStep = { + "engine's `applyFieldDefaults` gate, so read `fields[name].defaultValue` and apply " + 'that same `!= null` test yourself.', }, + { + id: 'action-descriptor-is-async-retired', + surface: 'ActionDescriptor.isAsync (the descriptor an executor publishes via `registerNodeExecutor` / `defineActionDescriptor`)', + replacement: + '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)', + reason: + '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.', + acceptanceCriteria: + '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.', + }, ], }; @@ -2932,6 +2987,15 @@ export const RETIRED_KEYS_BY_MAJOR: Readonly> 'ui/ElementRecordPickerProps:multiple', 'ui/ElementRecordPickerProps:searchFields', 'ui/PageCardProps:body', + // #6748 — ADR-0049 enforce-or-remove on the action-descriptor capability + // block. `isAsync` was a second spelling of `supportsPause` with ZERO + // readers on a fresh three-repo measurement; its sibling took the enforce + // leg in #6667 and this one takes the remove leg. Descriptors are published + // from executor TypeScript, not from stack metadata, so the D2 side is a D3 + // `SemanticMigration` (`action-descriptor-is-async-retired`) rather than a + // MetadataConversion — there is no stored source for `os migrate meta` to + // rewrite. The `EnhancedApiError.fieldErrors` precedent. + 'automation/ActionDescriptor:isAsync', ], }; From a2a8941d090ec601a4d8e02a9caf7b2e9cee6b96 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 9 Aug 2026 01:39:30 +0000 Subject: [PATCH 2/2] 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 Claude-Session: https://claude.ai/code/session_011sGk4SKHqGRgmmqUok1P8M --- .changeset/action-descriptor-is-async-retired.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.changeset/action-descriptor-is-async-retired.md b/.changeset/action-descriptor-is-async-retired.md index 08fbd34613..d72c0d0ee6 100644 --- a/.changeset/action-descriptor-is-async-retired.md +++ b/.changeset/action-descriptor-is-async-retired.md @@ -6,6 +6,8 @@ refactor(spec)!: retire `ActionDescriptor.isAsync` — a second spelling of `supportsPause` that nothing ever read (#6748, ADR-0049) + + **FROM → TO:** `isAsync: true` → delete the key; declare `supportsPause: true` (plus the `resumeAuthority` its pauses need) and return `suspend: true` from `execute()`. `isAsync: false` → delete the key; there was never anything to preserve.