Repository navigation
fix(spec/automation)!: refuse a $ name at every remaining flow binding — loop / map iterator and index, screen idVariable and field name, declared variables, assignment targets - #22746
Conversation
A loop or map iteratorVariable / indexVariable, an object-form screen's idVariable, a screen field's name, a declared flow variable's name and an assignment node's targets now refuse a name that starts with `$`, by the one rule (`flowBoundVariableNameSchema`) outputVariable and errorVariable already compose. ADR-0087 D3 entry `flow-binding-name-dollar-refused`. Claude-Session: https://claude.ai/code/session_01KNKBCRDJCu5tGy3TEbvtrF Co-authored-by: Claude <noreply@anthropic.com>
…umerate them Claude-Session: https://claude.ai/code/session_01KNKBCRDJCu5tGy3TEbvtrF Co-authored-by: Claude <noreply@anthropic.com>
…ed refinement The `$` rule on a bare legacy `assignment` config's top-level keys is a superRefine (a catchall sees values, never keys), so the published JSON Schema cannot state it; the ledger names the new site. Claude-Session: https://claude.ai/code/session_01KNKBCRDJCu5tGy3TEbvtrF Co-authored-by: Claude <noreply@anthropic.com>
… rule Claude-Session: https://claude.ai/code/session_01KNKBCRDJCu5tGy3TEbvtrF Co-authored-by: Claude <noreply@anthropic.com>
…llar-binding-keys
The `screen` and `script` executors trim a binding name before they bind it, so `idVariable: ' $id'` passed a first-character rule and its screen then named `$id`, refused on resume. The rule now refuses a name whose first non-blank character is `$`, at every binding key. Claude-Session: https://claude.ai/code/session_01KNKBCRDJCu5tGy3TEbvtrF Co-authored-by: Claude <noreply@anthropic.com>
…llar-binding-keys
Contract reviewServed-tier: Reviewed read-only on the net diff of PR #22746 against its merge base with ① Derived judgmentsThe accept-set narrowings, each judged against the executor that binds the name:
The first-non-blank-character change ( The enumeration / discovery pin — right, with its limit stated. It walks The ADR-0087 kit — right on this head. Public surface: no new export — ② Semver level
③ Boundary flagsEach deviation and out-of-scope finding the ACCEPT
Out-of-scope findings, each confirmed at the merge base and left noted as the ACCEPT says: the executor New flags from this review, escalated to the dispatching seat:
Implemented-by: VERDICT: PASS Generated by Claude Code |
…llar-binding-keys Resolved by hand to the #22706 model: packages/spec/src/migrations/registry.ts leaves git (deleted on main, generated at build); the order-93 STEP18_RATIONALE fragment for flow-binding-name-dollar-refused moves into packages/spec/src/migrations/registry.ts.template, sorted by key before flow-binding-variable-dollar-name-refused. The entry file stays. Claude-Session: https://claude.ai/code/session_01KNKBCRDJCu5tGy3TEbvtrF Co-authored-by: Claude <noreply@anthropic.com>
📓 Docs Drift CheckThis PR changes 1 package(s): 2 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 3 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 139 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 e5f49596a4d4d5f155dbf307de8901ef2914a6d6 && git checkout e5f49596a4d4d5f155dbf307de8901ef2914a6d6
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin e84aeb36ce14169a633670f14ce8280fc998e2b9 7b175fa88b17b4f12020832308cdacf93a57d0cc && git checkout -B drift-repro e84aeb36ce14169a633670f14ce8280fc998e2b9 && git merge --no-ff 7b175fa88b17b4f12020832308cdacf93a57d0cc
node scripts/docs-audit/affected-docs.mjs --json e84aeb36ce14169a633670f14ce8280fc998e2b9
|
|
Contract reviewServed-tier: Round 2, scoped to the sync hop. Round 1 ( ① Derived judgments1. The resolution — right, and exactly the fragment round 1 judged.
2. Nothing else moved — right, proved per blob, not per diffstat.
3. The red
4. Round 1's judgments stand. The accept-set narrowings (eight sites, the first-non-blank rule, the ② Semver levelUnchanged from round 1, on the same bytes: ③ Boundary flagsRound 1's flags, re-read on this head:
Dev-report items this round:
New flags, escalated to the dispatching seat:
Implemented-by: VERDICT: PASS Generated by Claude Code |
…llar-binding-keys
Landing pre-checks at
|
Fixes #22572
Clause-②: no (narrowing)
What this does
This is the close-out of the family "the
$names are the flow engine's at every binding door", after #22477 (the read side) and #22502 (outputVariable,errorVariable). Every remaining place a flow binds a variable by name now refuses a name whose first non-blank character is$. It uses the one rule PR #22569 added,flowBoundVariableNameSchemainpackages/spec/src/automation/flow-bound-variable-name.ts. The rule stays package-internal, so there is no new export (check:api-surfaceis green).loopiteratorVariable/indexVariableLoopConfigSchemapattern(defaultitemandminLength: 1kept)mapiteratorVariable/indexVariableMapConfigSchemapattern(defaultitemkept)screenidVariableScreenConfigSchemapatternscreenfieldname(an in-place addition, see below)ScreenFieldConfigSchemapatternnameFlowVariableSchemapatternassignmenttarget, a key of theassignmentsmapAssignmentConfigSchema(key schema)propertyNames.patternassignmenttarget, a top-level key of a bare configAssignmentConfigSchema(superRefine)dropped-refinements.baseline.jsonassignmenttarget, a legacy[{ variable, value }]itemFlowSchemaarmOne sentence at every door. The refusal is the rule's own message. It says the
$names are reserved for the engine, names the key, and gives the remedy: the same name without the$, read as{{ name }}.loop,mapandscreenkeys. These are refused throughflowNodeConfigRefusals, soFlowSchema.parse,registerFlow,objectstack validateand the run itself (parseNodeConfig) all refuse them atnodes.N.config.KEY.FlowVariableSchema.name.FlowSchemarefuses it directly, atvariables.N.name.assignmenttargets. No executor contract parses anassignmentconfig, becausegetBuiltinNodeConfigContracts()has noassignmententry. SoFlowSchema.parsenever appliedAssignmentConfigSchema. A new arm in theFlowSchemasuperRefine walkscollectFlowGraphs, region bodies included. It judgesflowAssignmentTargets(config), which covers the three shapes the executor binds (service-automationbuiltin/logic-nodes.ts) and only those.The first non-blank character, at every binding key. The
screenandscriptexecutors trim a name before they bind it (cfg.idVariable.trim(),cfg.outputVariable?.trim()). SoidVariable: ' $id'passed a first-character rule. The rule's regex is now^\s*(?:[^$\s][\s\S]*)?$(with\$error|forerrorVariable). JavaScript's\sis exactly the setString.prototype.trimremoves, so the rule refuses a name exactly when its trimmed form starts with$. This applies tooutputVariableanderrorVariabletoo. #22502's changeset is still unreleased on the same18.0.0-nextline.Additions beyond the card's list (bounded in-place fixes; all four conditions hold)
The fixes below meet all four conditions: the same defect class, a mechanical fix (compose the one rule), the claim's own file with no other claim on it, and the same gate family.
ScreenFieldConfigSchema.name. Its own describe says "the flow variable the value binds to". The resume that submits a flat screen writes each field's value under itsname. A$name there makes the screen impossible to submit (measured below).idVariable: ' $id'registered. The paused screen then named$id, and its resume answeredINVALID_SIGNAL.The claim's file surface needs two more entries:
ScreenFieldConfigSchema(inbuiltin-node-config.zod.ts, already listed) andpackages/spec/dropped-refinements.baseline.json. That file is a forced debt of the bare-shape key rule:build-schemas.tsrefuses the build until the site is declared, and its header total moves from 704 to 705. The four regenerated reference pages are within "ascheck:generateddecides".The PM's mechanism assumptions, measured at
origin/main0f77ff5202The sites. Every one accepted a
$name, measured with each contract'ssafeParseandFlowSchema.safeParseagainst the built dist:0f77ff5202LoopConfigSchema.iteratorVariablecontrol-flow.zod.ts:220,z.string().min(1).default('item')'$row'LoopConfigSchema.indexVariable:222'$i'MapConfigSchema.iteratorVariablebuiltin-node-config.zod.ts:1059,z.string().default('item')(where [v18] retire the{var}template dialect in flow assignment slots: refuse at registration with per-spelling remedies (the C half of #11182 ruling D, on the v18 train) #19939 S1 left the block)'$row'MapConfigSchema.indexVariable:1061'$i'ScreenConfigSchema.idVariable:862'$id'ScreenFieldConfigSchema.name:700(not on the card)'$f'AssignmentConfigSchema, theassignmentsmap key:1163,z.record(z.string().min(1), ...)'$y'AssignmentConfigSchema, a bare top-level key.catchall()sees values, never keys'$y'FlowVariableSchema.nameflow.zod.ts:218,z.string()'$x'FlowSchemawithvariables: [{ name: '$x' }]andassignment{ assignments: { $y: 1 } }Defaults. Both
iteratorVariabledefaults stayitem(pinned). The published JSON carriespatternbesidedefault: "item".check:authorable-surfaceis green, andauthorable-defaults/automation.jsonis unchanged.The engine's own names. No shipped flow or fixture binds a
$name on these positions (item 4). The run-time effect was measured withAutomationEngineat0f77ff5202, through a scratch test that is not committed:loopwithiteratorVariable: '$record'andindexVariable: '$runId': after the loop, a screen titledrecord is {{ $record }}, runId is {{ $runId }}renderedrecord is B, runId is 1.mapwithiteratorVariable: '$record':record is B.assignmentwith{ assignments: { $record: 'clobbered' } }:record is clobbered.$recordwithdefaultValue: 'mine': the engine's own seeding overwrote it, and the title showed the trigger record.screenwithidVariable: '$id': the run paused, and the resume the console sends ({ $id: id },FlowRunner.tsxonObjectFormSavedat the pinned objectui20c6d351ad) answeredINVALID_SIGNAL.screenfieldname: '$x': the sameINVALID_SIGNAL.registerFlowrefuses all six flows: aZodErrorfromcanonicalizeStoredFlowat the binding path.The reach: zero newly refused bindings.
app-crm1,app-todo4,app-showcase30) were run throughFlowSchema.safeParse. All parse OK, and the before and after listings are byte-identical.examples(234 files),packages/platform-objects(167 files; it ships no flow),packages/qa/dogfood(280), the rest ofpackages,skills,appsandscripts, andhotcrmat1d7148bf2d(570 files; a read-only clone, deleted afterwards). It found no$-led binding on any position. Its only hits wereerrorVariable: '$error', which stays legal, and filter operator keys.20c6d351ad. None. The control (errorVariable: '$error') hit.The enumeration pin. The pin finds binding keys by name. It walks the JSON Schema of every Zod export of
automation/index.ts(72 schemas) for any property spelled*Variable, at any depth. Each one must publish the rule'spattern, be in the rule's vocabulary, and have a row in the pin's site table. A floor keeps the walk from passing over nothing.*Variablecannot be found this way: a field'sname, a variable'sname, anassignmentskey. Those three are hand-listed. They are the rule's exported vocabularyFLOW_BINDING_KEYS, which the pin holds equal to the site table.ADR-0087 disposition
packages/spec/src/migrations/entries/semantic/18.flow-binding-name-dollar-refused.ts, on protocol 18. Its step-18 rationale fragment hasorder: 93, after spec(automation): try_catch's errorVariable and a node's outputVariable accept a $-named variable that a flow text slot now refuses to read (two doors of one contract disagree after #22477) #22502's92.registry.tswas regenerated withgen:migration-registry. It is still tracked, because PR build(spec): the migration registry is generated at build and leaves git #22706 had not landed atf59a73c395..changeset/22572-flow-binding-name-dollar-refused.md:@objectstack/specmajorin pre mode, withClause-②: no (narrowing)and theregistered flow-binding-name-dollar-refusedmarker.check-adr-0087-registrationreads[major+BREAKING+clause-②-narrowing] registered flow-binding-name-dollar-refused (new here).Tests (at
af752544fb)New pins, in
packages/spec/src/automation/flow-bound-variable-name.test.ts. The file went from 26 to 115 tests. The pin table has 18 binding sites, and each site:FlowSchemaat exactly its path, and nowhere else;$record,$runId,$loopItemsand$;' $x', a tab, a newline);x,a$band' x'.The file also pins:
defineStackrefusing a declared$totalwith{ code: 'STACK_SCHEMA_INVALID', status: 422 }atflows.0.variables.0.name;{{ row.name }}in a loop body);flowAssignmentTargetsover the three shapes, and an assignment inside a region body;@objectstack/spec,vitest run --project local --maxWorkers=2: 642 files, 19286 passed, 1 todo.pnpm --filter @objectstack/spec typecheckexits 0, withcheck:test-typecheck: OK.@objectstack/service-automation, the whole suite against the rebuilt spec: 185 files, 2368 passed. Its typecheck exits 0.@objectstack/lint, the whole suite: 135 files, 6321 passed. Its typecheck exits 0.The CLI guidance test,
vitest run --project integration test/migrate-meta-engine-guidance.test.ts, after the CLI closure build (turbo run build --filter=@objectstack/cli^...): 3 passed. It holds the new entry's printed guidance.pnpm --filter @objectstack/spec check:generated: all 15 generated artifacts are up to date, after a rebuild at the final source.Ablation
Each ablation ran against the committed fix, with the mutation and restore going through
scripts/ablation-replace.mjsin wrap mode. The spec tests importsrc, so no dist leg applies.MapConfigSchema.indexVariable. The anchor went from 1 to 0, and the blob fromcbe18e1969b2toca9af2dd1eb3.indexVariablecontract, flow-door and engine-name pins, and the discovery pin withMapConfigSchema.indexVariable publishes no pattern.cbe18e1969b2), andgit diff HEADis empty.[^$\s]back to[^$]). The blob went fromb7564636d07etoae5b2c3c1277.b7564636d07e), andgit diff HEADis empty.Gates (at
af752544fb)node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack(no paths, merge basef59a73c39) derived 114 commands. 112 were run, all exit 0.check:skill-examplesfirst exited 3 (PREREQUISITE NOT MET:client-reactwas not built). After buildingclientandclient-react, its re-run read262 prose examples type-check.--ranreads114 derived, 112 run, 0 NOT-MEASURED, 2 UNRUN.check:dual-build-cjs-loads. Reason: it needs a whole-workspace build, which the dispatch rules out.check:type-check-debt. Reason: its script is--re-measure, which the dispatch rules out.check:api-surfaceis green), because aregexcheck changes no TypeScript type.$namespace at every binding door: loop and mapiteratorVariable/indexVariable, a screen'sidVariable, a declared flow variable'snameand anassignmenttarget still bind a$name a text slot refuses to read #22572.Acceptance notes
service-automationexecutorconfigSchemadescriptors, and theengine.tsbuildSubflowResumeSignalcomment. This PR touches noservice-automationfile.loop. Its values are not judged at the flow door. This is the contract map'sparsedWhen, unchanged since build: ascriptnode's undeclared config key passesobjectstack validate,compileandregisterFlow, then fails every run — the key half of #21898's class (subflowby reading) #21982. ItsiteratorVariableis never read; the executor sets$loopItemsand$loopIndexitself.map.inputandsubflow.inputkeys name the callee's variables, so they are not bindings in this flow. A callee can no longer declare a$variable, so a$key there binds nothing.api/automation-api.zod.tsScreenSpec.idVariableis a response shape the engine produces, not an authoring door. It carries no rule.content/docs/automation/flows.mdx(hand-written) still names onlyoutputVariablebesideerrorVariablein its paragraph on where the$names belong. The paragraph is still true, and it is outside the claim's surface. It is not widened here.Generated by Claude Code