refactor(spec)!: retire ActionDescriptor.isAsync (#6748) and correct supportsPause's TSDoc (#6749) - #6862
Merged
Merged
Conversation
…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
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
📓 Docs Drift CheckThis PR changes 3 package(s): 112 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
github-actions
Bot
added
documentation
…irement (#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
os-zhuang
marked this pull request as ready for review
August 9, 2026 02:01
This was referenced Aug 9, 2026
This was referenced Aug 9, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #6748
Fixes #6749
Two cards on one file (
packages/spec/src/automation/node-executor.zod.ts), folded into one PR per #6749's own sanctioned option and the claim comments on both issues. They are the two halves of one fact:supportsPauseandisAsyncwere two spellings of "this node type can suspend the run", and #6667 / PR #6746 split them by evidence —supportsPausetook the enforce leg, soisAsynctakes the remove leg, andsupportsPause's own docblock has to stop saying nothing reads it.Premise verification (both cards verified against
origin/mainbefore any edit)Base:
origin/mainat578bdef(the issues were filed againstcfeb9a0).#6748 — the zero-reader measurement, re-run fresh at pickup. All commands run in the worktree at
578bdef:git grep -nE '\.isAsync'(property reads, objectstack)git grep -nE "\[['\"]isAsync|\"isAsync\"|'isAsync'"(bracket / string access)git grep -nE "\{[^}]*\bisAsync\b[^}]*\}\s*="(destructuring)git grep -ni isasync— objectui @origin/maingit grep -ni isasync— cloud @origin/maingit grep -n isAsync— objectstack, all formsThe 22 are all non-readers, matching the issue's inventory: 1 declaration, 3 generated baselines, 5 shipped writers, 5 fixtures/shape pins, 8 prose (mdx + four ADRs + one code comment). The count moved 19 → 22 only because the issue's third row counted an org-wide code-search page; the read count is unchanged at zero. Premise holds: no reader appeared since filing.
#6749 — both halves confirmed. The old sentence was still on
mainverbatim ("A declaration, not an enforced fact (#5703). No execution path reads it…"), and PR #6746's enforcement is onmain:AutomationEngine.refuseUndeclaredSuspensionplus its call site inexecuteNode, withsupports-pause-runtime-enforcement.test.tsalongside it.Card 1 — #6748: retire
ActionDescriptor.isAsyncRoute per the
spec-property-retirementskill:retiredKey()tombstone, becauseActionDescriptorSchemaisz.object({…}).describe(…)and not.strict()— a plain delete would let the five shipped descriptors (and every third-party one) parse clean and lose the key in silence, the ADR-0104 shape.isAsync: retiredKey(…)with the prescription naming the live two-part mechanism (suspend: truefromexecute()plussupportsPause: trueand aresumeAuthority), and an in-schema comment recording why it is tombstoned rather than deleted.'automation/ActionDescriptor:isAsync'added toRETIRED_KEYS_BY_MAJOR[17](the exactdefKey:namegate (b) reads by set membership), plus a D3SemanticMigrationaction-descriptor-is-async-retiredinMIGRATIONS_BY_MAJOR[17].semantic, and that step'srationaleextended.MetadataConversion, deliberately — a descriptor is published from an executor's TypeScript (registerNodeExecutor/defineActionDescriptor), never stored in stack metadata; there is no collection foros migrate metato rewrite. This is the skill's "response-surface key with no source to rewrite" case, theEnhancedApiError.fieldErrorsdisposition one layer down.builtin/screen-nodes.ts,builtin/map-node.ts,builtin/wait-node.ts,plugin-approvals/approval-node.ts,plugin-approvals/approval-revise-node.ts, each leaving a one-line marker where the key was.approval-revise.test.tspinned exactly the deleted limb inside atoMatchObject, which would have gone on passing if the key ever came back. Replaced with an explicitexpect(descriptor).not.toHaveProperty('isAsync'), so the retirement has a pin that can actually go red.packages/spec/src/automation/node-executor.test.ts) — the prescription is asserted (.toThrow(/ActionDescriptor\.isAsync.*removed.*supportsPause: true/s)),isAsync: falseis asserted to be refused too (the realistic upgrade shape: someone who spelled out the old default must not slip through), and a clean parse is asserted to publish noisAsyncof its own.authorable-surface/automation.jsongainsautomation/ActionDescriptor:isAsync [RETIRED];authorable-defaults/automation.jsonloses its= falsedefault line. Both produced bygen:schema, never hand-edited.gen:spec-changes,gen:upgrade-guide,gen:api-surface,gen:docs.majorfor@objectstack/specwith the FROM → TO mapping;patchfor@objectstack/service-automationand@objectstack/plugin-approvals, whose published descriptors lose the key.*.form.tsinput, noskills/**mention, noexamples/**author site. So noi18n:extractdelta either (check:i18ngreen).docs/adr/0018,0019,0044mentionisAsyncas the decision they made at the time; an ADR is a record, not a reference page, and the repo's precedent is that retired keys survive in ADR prose (triggerPhrasesstill sits in ADR-0010 after its retirement). The one non-ADR code comment that called theisAsyncvariant future work (builtin/http-nodes.ts) was corrected, since it addresses whoever implements that variant next.The four ratchets — expected readings for THIS route
The skill warns that the expected reading depends on the route, so stating it before the numbers: this is a key tombstone, neither an enum-value narrowing nor a whole-def deletion. So
authorable-surface/must move (it does — the[RETIRED]mark) andauthorable-defaults/must lose the default line (it does), whileapi-surface/andjson-schema.manifest/must not: the def is still emitted, no exported schema died with the key, andapi-surfaceprints type references and is structurally blind to key-level narrowing. Measured: both unchanged.authorable-surface.base.jsonis likewise untouched — it is the merge-base snapshot the live → retired transition is judged against, which is exactly how gate (b) saw this removal at all.Card 2 — #6749: correct
supportsPause's TSDocComment-only, as specified. The corrected block states the boundary #6746 drew, in the issue's own three parts:
supportsPause: false— or omitting it, which parses to the samefalse— whose executor returnssuspend: truehas the suspension refused: guard-class, un-routable by afaultedge, nothing durable written;supportsPause: true, never suspends) is legal — a capability, not an obligation;resumeAuthoritydefaults to'any', so every future pausing node ships fail-open — ADR-0044 says this is "tracked separately" and nothing tracks it #5561's resume gate rather than being refused here.Plus the alias hop, since the descriptor consulted is the canonical one an ADR-0018 alias forwards to.
Does the docblock project into generated references? Asked and answered by measurement rather than assumed: no.
gen:docsrenders.describe(), not the TSDoc, and the only line this PR moves incontent/docs/references/automation/node-executor.mdxis theisAsyncrow from card 1.supportsPause's row is byte-identical.The liveness ledger question, answered explicitly
The dispatch expected an
isAsyncrow whose verdict needed advancing. There is none, and there should not be.packages/spec/liveness/is keyed by metadata type —action.jsonisActionSchema(ui/action.zod.ts, what an author writes in a stack), notautomation/ActionDescriptor(what an executor publishes at registration).grep -rn isAsync packages/spec/liveness/returns nothing, and no ledger file coversActionDescriptorat all.That makes this the skill's strict-removal-style ledger discipline by a different road: there is no row to keep and none to delete, so
check:livenesshas nothing to say either way — and it passes, rather than reporting the key UNCLASSIFIED (which is what would happen ifActionDescriptorwere in the walk and I had deleted a row). No ledger file, and noliveness/README.mdcount, is touched by this PR.Reverse verification — direction predicted BEFORE running
Predicted: RED, the usual direction. The key sits in no
??chain and the pins judge a rejection the schema owns by name, so no inversion was available. Restoring the deleted limb (isAsync: z.boolean().default(false)) had to fail all three new pins:isAsync: truewould parse instead of throwing,isAsync: falselikewise, and.default(false)would put the key back on the parsed object sonot.toHaveProperty('isAsync')fails.Measured, with the limb restored (
git checkout origin/main -- …, nevergit stash—refs/stashis shared by every worktree of this repo):versus the committed tree:
Red as predicted, and wider than predicted in an informative way: the second failing file is a generated-baseline pin, so the
[RETIRED]baseline line is independently load-bearing rather than decorative.One honest note on method, because it changed the sequencing of this PR: the restore step was written as
git checkout BRANCH -- PATH, which reads the branch tip commit — and at that moment nothing had been committed yet, so the branch tip wasorigin/mainand the "restore" silently re-applied the revert. The two files were rewritten from the transcript, and the work is now committed before any revert experiment. Recorded because the skill's restore recipe presumes a commit exists and does not say so.Tests and gates
Every build/test run serialized on the shared verification lock (
flock), heap capped, test parallelism capped.@objectstack/spec—tsc --noEmitpass,test346 files / 8880 tests, all pass. Worth naming: the tombstone types the keynever, so the typecheck is also the sweep proving no authoring site in the package survived.@objectstack/service-automation— 71 files / 868 tests pass.@objectstack/plugin-approvals— 19 files / 446 tests pass.@objectstack/dogfood— pass (in a retirement's default consumption radius per the skill, whatever the import graph says)..github/workflows/lint.yml, run one by one: the spec-scoped set (check:authorable-surface,check:generated --reconcile-only,check:docs,check:spec-changes,check:upgrade-guide,check:api-surface,check:skill-refs/-docs/-examples,check:react-blocks,check:dual-source-exports,check:exported-any), pluscheck:liveness,check:empty-state,check:variant-docs(the three the skill flags as having no generator, so a red there is a real finding), the ~40 rootcheck:*gates,check:doc-formula-expressions, the downstream-contract typecheck, andpnpm lint. All green.Two results that needed adjudicating rather than assuming, both recorded:
Cannot find module '@objectstack/core',Failed to resolve entry for package "@objectstack/formula"). Three of those gates diagnose it themselves in so many words: "PREREQUISITE NOT MET — the workspace packages are not built … Nothing was measured." Building the closure exactly aslint.ymldoes (turbo run build --filter='./packages/*' --filter='./packages/*/*', 70/70 successful) turned all nine green. This is the skill's stale-disttrap, and it is why none of them was read as a finding.tsc --noEmitonservice-automationreports 3 errors — pre-existing, and CI does not run that command. The package has notypecheckscript, soturbo run typecheckskips it; it is governed instead by the DEBT ledger, whose entry recordserrors: 5and whose note names these three verbatim: "3 are TS2341 in src/nested-region-parity.test.ts, where the tests dot-read the privateengine.flows". That file is untouched by this PR. The governing gate,check:type-check-debt, passes — "34 ledger entr(ies) re-measured … none above its recorded number". This PR sits at 3 against a ceiling of 5, so it lowers rather than raises, and no note rewrite is owed.check:resume-authority-declareddeserves a specific mention: it keys onsupportsPause: true, never onisAsync, so removing the key cannot move it. It stays green — and its own output now reads "A suspension from a type that declares supportsPause: false is refused by the engine at run time (#6667), not by this gate", which is the evidence for the claim rather than an assertion of it.Scope
No
supportsPausebehaviour changed anywhere — #6749 is comment-only and #6746's engine seam is untouched.packages/spec/src/data/hook.zod.ts'sisasyncis a data-hook tombstone alias mapping toasync, a different key with a similar name, and is deliberately not touched.One out-of-scope finding filed, not fixed here: #6844 — the same #6746 residue in the D3 ledger's
resume-authority-undeclared…acceptanceCriteria, which still tells upgraders thatsupportsPauseis enforced by nothing and to hand-audit the mismatch. That string projects verbatim intospec-changes.jsonanddocs/protocol-upgrade-guide.md, so correcting it has a generated-artifact consequence #6749 explicitly disclaimed.Generated by Claude Code