Repository navigation
fix(spec/automation)!: refuse a $-named outputVariable, and a $-named errorVariable other than $error, at authoring - #22569
Conversation
…rrorVariable other than $error at authoring (WIP) Claude-Session: https://claude.ai/code/session_01KNKBCRDJCu5tGy3TEbvtrF Co-authored-by: Claude <[email protected]>
…ding-key $-name refusal (WIP) Claude-Session: https://claude.ai/code/session_01KNKBCRDJCu5tGy3TEbvtrF Co-authored-by: Claude <[email protected]>
…ey descriptions; read MIGRATIONS_BY_MAJOR as a record in the pin (WIP) Claude-Session: https://claude.ai/code/session_01KNKBCRDJCu5tGy3TEbvtrF Co-authored-by: Claude <[email protected]>
…WIP) Claude-Session: https://claude.ai/code/session_01KNKBCRDJCu5tGy3TEbvtrF Co-authored-by: Claude <[email protected]>
…orVariable (WIP) Claude-Session: https://claude.ai/code/session_01KNKBCRDJCu5tGy3TEbvtrF Co-authored-by: Claude <[email protected]>
…llar-variable-names
…e merged tree Claude-Session: https://claude.ai/code/session_01KNKBCRDJCu5tGy3TEbvtrF Co-authored-by: Claude <[email protected]>
📓 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:
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 5c9d1a66fd61d53618bf18085580df842bac695c && git checkout 5c9d1a66fd61d53618bf18085580df842bac695c
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 96e4be482987d0acd55e7b9c3c957580fe7c21d8 dfe2e07ef34ae1a76873479c2910da2a7e296618 && git checkout -B drift-repro 96e4be482987d0acd55e7b9c3c957580fe7c21d8 && git merge --no-ff dfe2e07ef34ae1a76873479c2910da2a7e296618
node scripts/docs-audit/affected-docs.mjs --json 96e4be482987d0acd55e7b9c3c957580fe7c21d8
|
Contract reviewServed-tier:
① Derived judgmentsEvery accept-set and public-surface change the diff implies, each judged against the triage direction (
② Semver level
③ Boundary flags
Implemented-by: VERDICT: PASS Generated by Claude Code |
…llar-variable-names # Conflicts: # packages/spec/src/automation/builtin-node-config.zod.ts
…ollows) The reference page builtin-node-config.mdx is regenerated here because the merge commit's deferral named it; spec-changes.json and the upgrade guide are main's side, regenerated in the next commit. Claude-Session: https://claude.ai/code/session_01KNKBCRDJCu5tGy3TEbvtrF Co-authored-by: Claude <[email protected]>
…e merged tree Claude-Session: https://claude.ai/code/session_01KNKBCRDJCu5tGy3TEbvtrF Co-authored-by: Claude <[email protected]>
Contract reviewServed-tier:
Why this round exists, and what it adds. The head moved from ① Derived judgmentsEvery accept-set and public-surface change the net diff implies, each judged against the triage direction (
② Semver level
③ Boundary flags
Implemented-by: VERDICT: PASS Generated by Claude Code |
Landing pre-checks at
|
Fixes #22502
Clause-②: no (narrowing)
What this does
The
$names are the flow engine's at the binding keys too, so a flow can no longer bind a$name that a text slot then refuses to read.outputVariable(get_record,create_record,map,script,subflow) refuses a name that starts with$, the engine's own names included (outputVariable: '$record'would overwrite the trigger record).try_catchnode'serrorVariablerefuses every$name except$error, its default.$, read as{{ name }}. ForerrorVariable: '$caught'that iscaught, read as{{ caught.message }}, or deleting the key and reading the default{{ $error.message }}.One rule, composed into all six keys:
flowBoundVariableNameSchemain the new package-internal leafpackages/spec/src/automation/flow-bound-variable-name.ts(not inautomation/index.ts, so there is no new public export;check:api-surfaceis green).regexcheck, not a.refine().z.toJSONSchema()emits it aspattern, so the publishedjson-schema/**refuses exactly what the parse refuses, anddropped-refinements.baseline.jsongains no row.FlowSchema.parse,registerFlow,objectstack validate,defineStackand the run itself all refuse such a name throughflowNodeConfigRefusals, anchored atnodes.N.config.outputVariable/nodes.N.config.errorVariable, region bodies included.$prefix the engine's closed list implies, never a copy of the list.FLOW_ENGINE_VARIABLESstays package-internal inflow-text-slot-template.ts, untouched. A binding must not claim any name in the engine's namespace, whichever name that is.6084430227rules that option out).The PM's mechanism assumptions, measured at
b53b949a15try_catch.errorVariablewasz.string().default('$error')atcontrol-flow.zod.ts:329and accepted any string. Confirmed.outputVariableis declared at five sites, each its ownz.string().optional(). They share no schema:builtin-node-config.zod.ts:445(get_record),:471(create_record),:991(map);schemaless-node-config.zod.ts:305(script),:384(subflow).flow.zod.tsandflow-function.zod.tsonly mention it in comments. Measured withgit grep -nE '^\s+[a-zA-Z]*(Variable|Var)s?\s*:\s*z\.' -- packages/spec/src.FLOW_ENGINE_VARIABLESinpackages/spec/src/automation/flow-text-slot-template.ts. It is unexported on purpose, and the module isexport *-ed from the barrel. So this card reads the prefix rule it implies rather than publishing the list.$-namederrorVariable/outputVariableother than$error, measured withgit grep -nP 'Variable\W{0,8}\$[a-zA-Z_]' -- .over the whole tree (13 hits, 10 of them$error). All three non-$errorhits are test fixtures, and all three are renamed here:packages/spec/src/automation/region-normalization.test.ts:127:'$err'to'err';packages/spec/src/automation/flow-builtin-node-config-keys.test.ts:344:'$err'to'err';packages/services/service-automation/src/throw-arm-error-refresh.test.ts:77:'$caught'to'caught'.errorVariableinexamples/app-showcase(2),content/docs(2) and theservice-automationREADME (1) is$error, which stays legal.$-namedoutputVariable.47b1f0bb71) authorserrorVariable: '$error'only (apps/console/src/preview-samples.ts:244).try-catch-node.ts:105(cfg.errorVariable || '$error') still gets$error.outputVariableverbatim withvariables.set(crud-nodes.ts,map-node.ts,screen-nodes.ts,subflow-node.ts).$-named binding.packages/services/**edit is the one fixture above. It is not an engine change: under the new contractregisterFlowrefused that flow (reverse check below).ADR-0087 disposition
packages/spec/src/migrations/entries/semantic/18.flow-binding-variable-dollar-name-refused.ts(protocol 18), plus its step-18 rationale fragment (order: 92).gen:migration-registry,gen:spec-changes,gen:upgrade-guide.test/migrate-meta-engine-guidance.test.tsis green (3/3)..changeset/22502-flow-binding-variable-dollar-name-refused.md,@objectstack/specmajorin pre mode, withClause-②: no (narrowing)and theregistered flow-binding-variable-dollar-name-refusedmarker.check-adr-0087-registrationreads[major+BREAKING+clause-②-narrowing] registered flow-binding-variable-dollar-name-refused (new here).Tests (at
ff2832a7bb, after the oneorigin/mainmerge throughos-regen-merge.shand its regeneration commit)packages/spec/src/automation/flow-bound-variable-name.test.ts, 26 cases.$xis refused with the remedy on each of the fiveoutputVariablecontracts, and every$name with it ($record,$error,$,$$x).x,a$band an absent key are accepted. A non-string keeps its type refusal.errorVariable: '$caught'is refused with the remedy, and so are$record,$errors,$error.xand$. Controls: the default$error, an explicit$errorandcaughtare accepted.FlowSchemarefuses atnodes.1.config.errorVariable, and inside a region atnodes.1.config.try.nodes.0.config.outputVariable.defineStackrefuses with{ code: 'STACK_SCHEMA_INVALID', status: 422 }, and the save door at the key.textSlotTemplateRefusal('Failed: {{ caught.message }}')passes, and a flow bindingcaughtwith that text in its catch region parses clean.patternon all six keys that accepts and refuses the same names.pnpm --filter @objectstack/spec exec vitest run --maxWorkers=2 src/automation src/migrations: 39 files, 1416 tests passed.pnpm --filter @objectstack/service-automation exec vitest run --maxWorkers=2(whole suite): 180 files, 2259 tests passed.pnpm --filter @objectstack/cli exec vitest run --project integration test/migrate-meta-engine-guidance.test.ts(after its closure build): 3 passed. The file lives in theintegrationproject. A first--project unitcall selected nothing; that call is not counted as a measurement.pnpm --filter @objectstack/spec typecheckandpnpm --filter @objectstack/service-automation typecheckboth exit 0,check:test-typecheck: OK.pnpm --filter @objectstack/spec check:generated: all 15 generated artifacts up to date.Ablation (committed fix, mutation and restore through
scripts/ablation-replace.mjs, wrap mode). The spec tests importsrcdirectly, so no dist leg applies.5211426e99fbtod0cb32780409.patternpin. Green: the controls, the remedy-reads pin, the non-string case and the ledger pins.5211426e99fb, andgit diff HEADis empty.Reverse check of the services fixture:
'caught'was put back to'$caught', thenthrow-arm-error-refresh.test.tswas run against the rebuilt spec.ZodErroratnodes.3.config.errorVariablethrown fromAutomationEngine.canonicalizeStoredFlowinsideregisterFlow. So the fixture rename was owed.b4f38da11225.Gates
node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack(no paths, merge base6a3f82efa) derived 116 commands, reconciled with--ran.pnpm check:dual-build-cjs-loadsis NOT MEASURED (reason: it needs a whole-workspace build, which this dispatch rules out). It is declared to CI.Acceptance notes
The same class, on sibling binding keys, is not in this PR.
FlowSchema.parseat this head still accepts a$name on:loop/mapiteratorVariableandindexVariable;screenidVariable;name;assignmenttarget key.NotifyConfigSchemarefuses a read of one ({{ $row.name }}). The triage ruling scoped this card toerrorVariableandoutputVariable, so the siblings are reported for the family close-out rather than widened here.service-automation's executor descriptors (configSchemaoncrud-nodes.ts,map-node.ts,try-catch-node.ts) still describe these keys as plain strings. The spec contract is the judge at every door, and the descriptor walk stands aside for builtins. Noted, not changed.engine.ts'sbuildSubflowResumeSignalcomment calls the reserved-name check a false positive "on an oddly-named"outputVariable. After this narrowing such a name cannot be authored. WhetherengineBuiltis still needed there for another reason was not measured. Noted, not changed.The hand-written
content/docs/automation/flows.mdxgains one paragraph beside thetry_catchexample saying where the$names belong.Generated by Claude Code