Repository navigation
fix(spec)!: refuse a connector_action node its executor cannot dispatch — no connectorConfig block, or a blank connectorId / actionId — at all three doors (#20418) - #20453
Conversation
…h at the flow parse (wip) Claude-Session: https://claude.ai/code/session_014EJ1ED8X4MMrT18BhVx4tx Co-authored-by: Claude <noreply@anthropic.com>
…ector_action node; the guard row reaches the executor past the doors Claude-Session: https://claude.ai/code/session_014EJ1ED8X4MMrT18BhVx4tx Co-authored-by: Claude <noreply@anthropic.com>
…lock requirement Claude-Session: https://claude.ai/code/session_014EJ1ED8X4MMrT18BhVx4tx Co-authored-by: Claude <noreply@anthropic.com>
…nnector-action-config-required
📓 Docs Drift CheckThis PR changes 1 package(s): 17 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: ⛔ 6 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails. What this run could not see
Coarse fallback — 137 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 0eb5b2b728cc7a7793abdac1508ac9727427674e && git checkout 0eb5b2b728cc7a7793abdac1508ac9727427674e
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 7fa3e3e07cc67877ae2f5d49eac42665c0515fda c23d0a32119f795ef7c1344981d542bb0c5f66c2 && git checkout -B drift-repro 7fa3e3e07cc67877ae2f5d49eac42665c0515fda && git merge --no-ff c23d0a32119f795ef7c1344981d542bb0c5f66c2
node scripts/docs-audit/affected-docs.mjs --json 7fa3e3e07cc67877ae2f5d49eac42665c0515fda
|
…itEventConfig and boundaryConfig are required on their node type Claude-Session: https://claude.ai/code/session_014EJ1ED8X4MMrT18BhVx4tx Co-authored-by: Claude <noreply@anthropic.com>
|
Seat note for reviewers — REWORK round 1 at The PR body predates round 1 and is not edited. Round 1 adds one file:
The seat's verdicts are on #20418: REWORK Generated by Claude Code |
Contract reviewServed-tier: Inputs, and nothing else: card #20418 (body and all six comments — triage Check-runs on the head: 35 runs — 33 ① Derived judgments
② Semver level
③ Boundary flagsThe dev's report
Out-of-scope findings, one line each: [0] the objectui designer seed — carrier objectui#10948, the seat's disposition stands. [1] lint One reading for the seat, non-blocking and outside this card: the dev's control — a Implemented-by: VERDICT: PASS Generated by Claude Code |
…c now requires, and pins that each unconfigured seed is a located save error (objectui#10948) (objectstack-ai#11247) Fixes objectstack-ai#10948 Clause-②: no ## What this does The flow node inspector now marks the config keys the installed `@objectstack/spec` (17.5.0) refuses a node without, so an author sees the requirement before the save-time error names it. The marker is the metadata form's own required marker (the `*` with `data-required-marker` that `SchemaForm`'s `FieldRow` draws), reused through a new `RequiredMarker` atom in `inspectors/_shared.tsx`. No new visual idiom. **Source of requiredness: the installed spec, asked at render time. There is no second list.** `inspectors/flow-required-keys.ts` probes the node as it stands: it removes the key from a copy and hands the copy to the judges the flow parse runs. The key is marked exactly when one of them then names it. - `flowNodeConfigRefusals` is the ONE config judge that `FlowSchema.parse`, `registerFlow` and `objectstack validate` share (objectstack-ai/objectstack#20316). It covers a key the executor contract requires, and a decision branch `label`. - `resolveFlowNodeExpressions` + `predicateSlotRefusal` is the flow parse's predicate-slot walk. It covers a decision branch `expression`, which the ledger marks `required`. - `FlowNodeSchema` is the node contract. It covers the `connectorConfig` / `waitEventConfig` / `boundaryConfig` blocks and the `end` node's config. Because the answer comes from the node's own configuration, a rule-dependent requirement is marked only while it applies: - `notify` requires `title` only while it has no `template`; - a `loop` requires `collection` once it has a body; - a refused `end` requires `message`. The marker is presence only. The spec also refuses a BLANK branch label or connector id, but that stays the save-time error's job. What gets marked, measured (pinned as an equality over the drawn labels, so a pin fails both for a missing marker and for an extra one): | node | marked | |---|---| | create / update / get / delete record | Object | | http | URL | | notify (seed) | Recipients, Title | | script | Function | | subflow | Flow | | map | Collection, Per-item flow | | connector_action | Connector, Action | | wait (timer seed) | Wait for, Duration | | boundary_event | Attached to, Event type | | decision branch row | Label, Expression (not Target) | | screen field row | Name (not Label / Type / Required / Visible when) | | loop (no body), approval, assignment, end (completed), try_catch | nothing | `aria-required` goes on each control the inspector owns: text / textarea / expression inputs, selects, numbers, and a row's scalar cells. The marker itself is `aria-hidden`, as in `SchemaForm`. ## Measure first (restart steps 1 and 2 of the triage) Measured on the installed 17.5.0, and kept as a regression pin: `previews/flow-canvas-seeds.saveErrors-10948.test.tsx`. - **17.5.0 carries objectstack-ai/objectstack#20453.** Its CHANGELOG `## 17.5.0` section lists `2304b16` (the objectstack-ai/objectstack#20418 connector refusal) beside `7dc45eb` (objectstack-ai/objectstack#20316). The live pass refuses the seeded blank `connectorConfig` at `nodes.0.connectorConfig.connectorId` and `.actionId`. - **Each freshly added node of each seeded kind is a located error.** The live `FlowSchema` pass (`validateMetadataDraft('flow', …)`) reports it at the node's config path, and `buildFlowProblems` puts it on that node's canvas badge, never on the flow as a whole: - `nodes.0.config.objectName` for the four CRUD kinds; - `…config.url` for `http`; - `…config.title` for `notify`; - `…config.function` for `script`; - `…config.flowName` for `subflow`; - `…config.collection` and `…config.flowName` for `map`; - `…config.branches` for `parallel`; - `…config.try` for `try_catch`; - the two `connectorConfig` ids for `connector_action`. `decision`, `loop`, `assignment`, `approval`, `screen`, `wait` and `end` seeds save clean. - **Control:** the same kinds, configured, make a flow that saves clean (`ok: true`, no issues). - **A blank branch label and a blank screen field name**, typed through the inspector's real row editor, are committed without the key. `FlowObjectListField`'s `rowsToList` drops a blank cell; `FlowStringListField`'s `rowsToList` is the primitive-list one and is not involved. They are refused at `nodes.0.config.conditions.0.label` and `nodes.0.config.fields.0.name`. - **Step 2, an unsaved draft kept at all:** not a regression. The live pass is advisory. `ResourceEditPage` gates no save door (button, ⌘S, autosave) on it, as pinned by `ResourceEditPage.schemaAdvisory.test.tsx` (re-run green on this head), so the draft stays in the editor with located errors. - **Mechanism assumption 3 holds.** The per-seed ratchet `flow-canvas-seeds.spec-parse.test.tsx` (`FlowNodeSchema.safeParse` per seed) stays green: 25/25. The new rules live in the `FlowSchema` walk, not in the node schema. ## Pins - `previews/flow-canvas-seeds.saveErrors-10948.test.tsx` covers the measurement above: every palette kind plus `map`, the clean control, and the two blank row keys through the real inspector. - `inspectors/FlowNodeInspector.requiredMarkers-10948.test.tsx` covers: - the markers per seeded kind (an equality, with the unmarked labels asserted present, so an empty set is never a form that drew nothing); - the decision and screen rows; - the three rule-dependent cases, plus `wait` on a signal; - `aria-required` on a text input, a select trigger and a row cell; - the module's per-key and per-column answers. ## Verification All runs below are at head `e962853fb`. Each test run went through the shared verify lock, and its pass count is quoted from the run's own output. - **Tests.** `pnpm exec vitest run` covered every test file under `packages/app-shell/src/views/metadata-admin/inspectors/` and `…/previews/`, in three chunks. The population is 205 files, enumerated with `git ls-tree` at head. - Chunk 1, the flow suites and both new files: 53 files, 726 passed, 1 skipped. The skip is the existing `it.skip` in `flow-node-config.spec-reconciliation.test.ts`. - Chunk 2, the other inspectors: 78 files, 1109 passed. - Chunk 3, the other previews: 74 files, 956 passed. - **Type-check.** `pnpm --filter @object-ui/app-shell type-check` exited 0. It ran after building the closure with `pnpm --workspace-concurrency=2 --filter '@object-ui/app-shell^...' build` (29 of 47 projects, exit 0). `tsc --listFilesOnly` confirms the new module is in the `tsconfig.json` pass and both new test files are in the `tsconfig.test.json` pass. - **Ablation.** It was run with `ablation-replace.mjs` in wrap mode. It deletes the marker in `FlowReferenceField`: the anchor count went 1 to 0 and the blob went from `e815546c2bd1` to `c2a3f7207f94`. - The marker pin went red: 8 failed and 21 passed. The 8 are exactly the reference-kind rows: the four CRUD kinds, `subflow`, `map`, `connector_action` and `boundary_event`. - Restored: the blob equals HEAD (`e815546c2bd1`) and `git diff HEAD` is empty. The same pin re-run on the restored tree passed 29/29. - **ESLint** on the 11 changed source and test files: 0 errors. Every pre-existing file carries the same warning set as at the merge-base, and the new files carry none. - **Gates.** All of these exited 0: - `node scripts/check-changeset-presence.mjs` - `node scripts/check-control-bytes.mjs` - `pnpm check:new-line-citations`, with 0 new citations - `pnpm check:i18n-keys`; no string is added, the marker is `*` - `node scripts/markdown-test-inputs.mjs --audit` - `check-changeset-no-major` - `check-changeset-overwrite` - `check-changeset-fixed` - `check-pending-changeset-literals` - `check-changeset-claims`, which is report-only - **Beside the two directories:** - `ResourceEditPage.schemaAdvisory.test.tsx` (the step-2 pin) and `studio-design/ObjectGroupInspector.test.tsx` (the one `_shared.tsx` consumer outside them): 2 files, 6 passed. - The root `scripts/__tests__/` suite, run once because the diff adds a changeset (markdown): 177 files passed and 2 skipped; 5371 tests passed and 2 skipped. - **Declared narrowing.** The full `@object-ui/app-shell` suite and the full lint farm are CI's. ## Acceptance notes These are notes, not filed. - Several flow-inspector labels are not programmatically bound to their control: `FlowReferenceField`, `FlowKeyValueField`, `FlowStringListField`, the list row labels, and the inline text labels. This is pre-existing and not widened here. Those fields get the visual marker. Where the inspector renders the input itself, the input also gets `aria-required`. The reference combobox and the list editors carry no `aria-required`. - The `boolean` and condition-builder kinds are not threaded. No field of either kind is spec-required today. - `specRequiredColumns` answers per column from an empty-row probe. A column required only by a rule on a sibling cell (a screen `lookup` field's `reference`) would not show in the row label; the save-time error still names it. The static screen table has no such column today. - Not measured live: the server's `saveMetaItem` runs the same spec schema on a `?mode=draft` save too. So against a 17.5.0 server, an autosave of a flow holding an unconfigured node is refused (422) with located issues, and the designer holds autosave for that slice until it changes. That is the ruled loud-at-save direction, and the draft stays in the editor. --- _Generated by [Claude Code](https://claude.ai/code/session_011p7ikEivgXefNDaE5S5Uec)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
Fixes #20418
Clause-②: no
A
connector_actionflow node its executor cannot dispatch is now refused at all three build doors (FlowSchema.parse,AutomationEngine.registerFlow,objectstack validate), at any depth including an ADR-0031 region body:connectorConfigblock — acustomissue atnodes.N.connectorConfig;connectorIdoractionIdblank (empty, or only whitespace) — acustomissue atnodes.N.connectorConfig.connectorId/.actionId.The changeset carries
Clause-②: no (narrowing)and the ADR-0087 dispositionregistered connector-action-config-required.What was wrong, measured on
origin/maine4d3f2cabefore the changeProbes drive door 1 (
FlowSchema.safeParse, specdist), door 2 (AutomationEngine.registerFlowwithinstallBuiltinNodesand a registeredprobeconnector, thenexecute), and door 3 (the BUILT CLI,node packages/cli/bin/run.js validate --json, in a stack directory holding oneobjectstack.config.ts).connectorConfigsuccess=truevalid: true, exit 0success=false:connector_action 'call': connectorConfig.connectorId and .actionId are required{ connectorId: 'probe', actionId: 'ping', input: {} }success=truevalid: true, exit 0success=true{ connectorId: '', actionId: '', input: {} }success=truevalid: true, exit 0success=false, the same guard messageactionId: ''onlysuccess=truesuccess=false, the same guard message' 'success=truesuccess=false:no handler for ' . ' — is the connector plugin registered?loopbody, noconnectorConfigsuccess=truevalid: true, exit 0success=false, the same guard messageloopbody, block completesuccess=truesuccess=trueAfter, measured on this branch (spec
distbuilt fromc81e639dbd; the merge ofmainsince touched no spec, service-automation or CLI validate source)customatnodes.1.connectorConfigZodError, the same issuevalid: false, exit 1,customatflows.0.nodes.1.connectorConfigsuccess=truesuccess=truevalid: true, exit 0customatnodes.1.connectorConfig.connectorIdand.actionIdvalid: false, exit 1, the same two paths underflows.0.actionId: ''onlycustomatnodes.1.connectorConfig.actionId' 'customat both idsloopbodycustomatnodes.1.config.body.nodes.0.connectorConfigvalid: false, exit 1,customatflows.0.nodes.1.config.body.nodes.0.connectorConfigsuccess=truesuccess=trueconfig: { connectorId, actionId }, no blockcustomatnodes.1.connectorConfig(a direct parse meets the pre-conversion spelling)flow-node-connector-config-liftD2 conversion lifts the complete pair first; runsuccess=trueconnectorConfig: {}invalid_typeat both ids only, as before (no second issue)Envelope per door: door 1 and door 2 answer the parse's Zod issue (
code+path;registerFlowhas no HTTPstatusof its own); door 3 answersvalid: false, exit 1 and the samecode+pathunderflows.K..The fix
packages/spec/src/automation/flow.zod.ts:connectorActionConfigRefusals(node)(module-private, besiderequireTypeScopedConfig), called from a new block in theFlowSchemasuperRefine that walkscollectFlowGraphs, like theflowNodeConfigRefusalswalk above it. It judges strings only, so a block the node shape already refuses ({}, a non-string id) gets no second issue. The messages carry no tracker number and prescribe a minimal block; the absent-block one also says keys left underconfigare not read.registerFlowandobjectstack validate: no change. Both parse throughFlowSchemaafter the ADR-0087 conversions, so the parse's issue is what they answer.connector-nodes.tsis unchanged. It is still the refusal a node meets past the doors, andguard-refusal-inventory.test.tsstill classifies it as un-routable.Route choices
The rule runs in the flow walk, not in
requireTypeScopedConfig. That was the dispatch's suggested route, and a better route was measured. A node-level refusal does not reach a region-nested node at the flow parse, becauseparseFlowNodeRegionsleaves a refused region raw. The control on this tree is a block-lessboundary_event, whichrequireTypeScopedConfigrefuses today. At the top level it answerscustomatnodes.1.boundaryConfig. In aloopbody,FlowSchema.safeParseanswerssuccess=true, and onlyLoopConfigSchemarefuses it. So the suggested route would leave shape (c) admitted at all three doors. The walk refuses it at the path the author wrote.FlowNodeSchemaalone still parses the designer seed, which objectui's seed ratchet (flow-canvas-seeds.spec-parse.test.tsx) requires of every seed. The flow the seed is saved into is refused. That is the posture fix(spec)!: refuse a flow node config its executor cannot run — a required key left out, or a decision branch list it cannot read — at all three doors (#20316) #20416 took for itshttp/notifyseeds, and a test here pins the split.Blank ids are refused, not only an absent block. This is the rule fix(spec)!: refuse a flow node config its executor cannot run — a required key left out, or a decision branch list it cannot read — at all three doors (#20316) #20416 applied to a
decisionbranchlabel. fix(spec)!: refuse a flow node config its executor cannot run — a required key left out, or a decision branch list it cannot read — at all three doors (#20316) #20416 has two arms:NON_BLANK_STRING) and non-text values. Its reason: "refusing only the absent key would leavelabel: '', which misroutes identically".connector_action's executor reads its block raw (!cfg?.connectorId || !cfg?.actionId) and parses no contract, so the decision-arm rule applies. Refusing absence alone would leave the designer seed admitted, and the seed fails every run identically (row (b) above).!valuelets' 'through, but a connectornamemust match^[a-z_][a-z0-9_]*$, so whitespace names nothing a dispatch can reach (row (b),no handler).keyisz.string(), so an action keyed by whitespace alone would become unreachable. No such key is declared anywhere in this repo.No lint-side copy.
validateStackExpressions(lint) carriesflowNodeConfigRefusalsfor a stack handed to it with no parse in front. Thewait/boundary_eventblock rule has no lint copy either, and every door the card names parses first.Rider (same code table)
node-config-key-missinginflow-node-config-refusals.tsnow says the flow "used to register, and then every run that reached this node failed there". That is past tense, at a door that refuses the flow. Its one quoting pin,KEY_MISSINGinflow-slot-refusal-codes.test.ts, moves with it. A repo-wide grep offlow registers, and thenfinds those two sites only.ADR-0087 kit
D3 entry
packages/spec/src/migrations/entries/semantic/18.connector-action-config-required.ts(protocol 18; no tracker number in any author-shown field; no backticks insurface), and the step-18 tails ofregistry.tsregenerated bygen:migration-registry..changeset/20418-connector-action-config-required.mdcontains:@objectstack/specatminor(the launch-window convention);Clause-②: no (narrowing)and theregistereddisposition marker;**BREAKING**banner, a FROM → TO table and a one-line fix.check-adr-0087-registrationreads it as[BREAKING+bang+clause-②-narrowing] registered connector-action-config-required.No D2 conversion: the platform cannot know the connector or the action the author left out.
Acceptance notes
Fixture triage (a disposition per fixture, not a batch rename)
specflow.test.ts, "should validate a complete parallel approval flow". Its twoconnector_actionstand-ins getconnectorConfig: { connectorId: 'finance_desk' | 'legal_desk', actionId: 'request_review' }.service-automationconnector-nodes.test.ts, "fails the step when connectorConfig is missing required fields". It registered a block-less node and asserted that the run failed. The new tests assert:registerFlowrefuses the block-less node (customatnodes.1.connectorConfig, and the flow is absent fromlistFlows());guard-refusal-inventory.test.ts, row "connector_action without connectorId/actionId". It registered{ config: {} }. It now registers a complete block and deletes it after registration (stripBlock, the sibling-block twin of [finding] a decision branch with no label registers and validates clean, then at run time the decision takes EVERY out-edge; a non-object conditions element also registers #20316'sstrip), so the row still classifies the executor's own guard.run-summary.test.tsspells the trio underconfigon three nodes, andregisterFlow's D2 lift completes the block.Producer census: who writes a
connector_actionnode without a complete blockRead at this head from every
type: 'connector_action'literal repo-wide (git grep):examples/app-showcase/src/automation/flows/index.ts: 4 nodes (:330,:468,:526,:579), all with a complete block. 0 refusals.packages/connectors/connector-{slack,rest,mcp}plugin tests: 4 nodes, all complete.The dispatch's single-hit leads:
trigger-record-change,service-messagingandcreate-objectstack: CHANGELOG / README prose only;runtime/src: a comment;qa/dogfood: a test name and comment over the showcase flow, which carries the block.None of them writes a node.
lintlint-flow-patterns.test.ts:2077(config: { connectorId: 'c', action: 'a' }, no block) is a lint-pattern fixture that never meetsFlowSchema. The lint suite is green, so it is left as is.FlowSchema's own@exampledocblock (flow.zod.ts):update_recordnode had noobjectName, which is already refused since fix(spec)!: refuse a flow node config its executor cannot run — a required key left out, or a decision branch list it cannot read — at all three doors (#20316) #20416 (measured one4d3f2ca:customatnodes.2.config.objectName).Both are fixed in the same literal. This is a bounded in-place fix: same defect family, same example, mechanical, a file in this claim, no new gate. Measured: the corrected literal parses
success=true.The D2 conversion
flow-node-connector-config-lift(conversions/registry.ts): its completeness-guard comment said an incomplete pair keeps failing at run time rather than "fails to load". That is false after this change. The comment is corrected; behaviour is unchanged.objectui (not edited), measured at
origin/main328abeband atb120b66:defaultNodeExtras('connector_action')seedsconnectorConfig: { connectorId: '', actionId: '', input: {} }(packages/app-shell/src/views/metadata-admin/previews/flow-canvas-parts.tsx:397).FlowSchemapass (clientValidation.ts:681) flags it atnodes.N.connectorConfig.connectorId/.actionId.FlowNodeSchema.safeParseper seed) stays green by construction.Reported for the seat; objectui#10948 carries the family. The pinned
.objectui-shaf8a9d0fbis not in this container's shallow objectui clone, so it is NOT MEASURED there.cloud: NOT MEASURED (no checkout in this container).
Docs not edited
content/docs/automation/flows.mdx's node-key table listsconnectorConfigas "optional", as it doeswaitEventConfig, which is required forwait. Per key and across node types, "optional" stays true. Tightening that table is a docs change outside this card's surface.Tests
Final head
992656cea5:spec, targeted onsrc/automation src/conversions src/migrations: 39 files, 1484 tests passed.service-automation, targeted onconnector-nodes,guard-refusal-inventory,run-summary,node-config-required-keys,connector-materializationandengine: 6 files, 324 tests passed..tsfiles (--no-inline-config --format json): 10 files reported, 0 errors, 0 warnings. Three pieces of evidence for this narrowing:eslint.config.mjs's**/*.{ts,…}blocks minusNEVER_LINTED, and all 10 files are inside it.parserOptions.project, no typed rules; stated in the config itself), so this diff cannot move any untouched file's verdict.Full package suites, at the branch's pre-merge heads. The merge of
maintouched none of these packages:@objectstack/specvitest run --project local: 565 files, 16651 passed (1 todo).typecheck(tsc --noEmit, scripts typecheck, test typecheck): exit 0.@objectstack/service-automation: 149 files, 1837 passed.typecheck: exit 0.@objectstack/lint: 115 files, 5331 passed.@objectstack/connector-slack3/10,connector-rest4/26,connector-mcp3/23,connector-openapi4/36 (files/tests), all passed.@objectstack/example-showcase: 29 files, 385 passed. The first attempt failed to resolve the unbuilt@objectstack/connector-slack(not a reading); it was rerun after building the showcase closure.@objectstack/dogfoodtest/showcase-declarative-mcp.dogfood.test.ts(the flowconnector_actiondispatch end to end): 2 passed.@objectstack/speccheck:generated: all 15 generated artifacts up to date.Ablation (one-shot, not kept): run through
scripts/ablation-replace.mjson the committed tree, with atraprestore.connectorActionConfigRefusals(node)was replaced byconnectorActionConfigRefusals(null). The anchor count went 1 to 0, the replacement 0 to 1, and the blobbae1a5ccto20017fad.connector-action-config-required.test.tsthen read 7 failed, 4 passed: every refused row red, every control green.bae1a5cc, andgit diff HEADis empty.Gates:
node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commandsat992656cea5derived 90 commands. All 90 were run and all exit 0;--ranwith recorded exit codes reports 90 derived, 90 run, 0 NOT-MEASURED.check:dual-build-cjs-loadsandcheck:type-check-debtfirst answered PREREQUISITE NOT MET (exit 3, unbuilt packages). They exited 0 after those packages were built.The session that built this is linked in the footer.
Generated by Claude Code