Skip to content

fix(spec/automation)!: refuse a $-named outputVariable, and a $-named errorVariable other than $error, at authoring - #22569

Merged
objectstack-fleet[bot] merged 11 commits into
mainfrom
claude/issue-22502-dollar-variable-names
Oct 10, 2026
Merged

objectstack-fleet[bot] merged 11 commits into
mainfrom
claude/issue-22502-dollar-variable-names

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

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.

  • A node's 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).
  • A try_catch node's errorVariable refuses every $ name except $error, its default.
  • The refusal names the remedy: the same name without the $, read as {{ name }}. For errorVariable: '$caught' that is caught, read as {{ caught.message }}, or deleting the key and reading the default {{ $error.message }}.

One rule, composed into all six keys: flowBoundVariableNameSchema in the new package-internal leaf packages/spec/src/automation/flow-bound-variable-name.ts (not in automation/index.ts, so there is no new public export; check:api-surface is green).

  • It is a regex check, not a .refine(). z.toJSONSchema() emits it as pattern, so the published json-schema/** refuses exactly what the parse refuses, and dropped-refinements.baseline.json gains no row.
  • Each executor parses its config against the same contract. So FlowSchema.parse, registerFlow, objectstack validate, defineStack and the run itself all refuse such a name through flowNodeConfigRefusals, anchored at nodes.N.config.outputVariable / nodes.N.config.errorVariable, region bodies included.
  • It reads the $ prefix the engine's closed list implies, never a copy of the list. FLOW_ENGINE_VARIABLES stays package-internal in flow-text-slot-template.ts, untouched. A binding must not claim any name in the engine's namespace, whichever name that is.
  • ⛔ The text-slot judge is not widened (triage 6084430227 rules that option out).

The PM's mechanism assumptions, measured at b53b949a15

  1. try_catch.errorVariable was z.string().default('$error') at control-flow.zod.ts:329 and accepted any string. Confirmed.
  2. outputVariable is declared at five sites, each its own z.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.ts and flow-function.zod.ts only mention it in comments. Measured with git grep -nE '^\s+[a-zA-Z]*(Variable|Var)s?\s*:\s*z\.' -- packages/spec/src.
  3. The closed list is FLOW_ENGINE_VARIABLES in packages/spec/src/automation/flow-text-slot-template.ts. It is unexported on purpose, and the module is export *-ed from the barrel. So this card reads the prefix rule it implies rather than publishing the list.
  4. In-repo authors of a $-named errorVariable / outputVariable other than $error, measured with git grep -nP 'Variable\W{0,8}\$[a-zA-Z_]' -- . over the whole tree (13 hits, 10 of them $error). All three non-$error hits 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'.
    • Every authored errorVariable in examples/app-showcase (2), content/docs (2) and the service-automation README (1) is $error, which stays legal.
    • No example or doc binds a $-named outputVariable.
    • The pinned objectui (47b1f0bb71) authors errorVariable: '$error' only (apps/console/src/preview-samples.ts:244).
  5. Engine readers. No engine reader breaks.
    • try-catch-node.ts:105 (cfg.errorVariable || '$error') still gets $error.
    • The executors write the author's outputVariable verbatim with variables.set (crud-nodes.ts, map-node.ts, screen-nodes.ts, subflow-node.ts).
    • No engine code authors a $-named binding.
    • The only packages/services/** edit is the one fixture above. It is not an engine change: under the new contract registerFlow refused that flow (reverse check below).

ADR-0087 disposition

  • D3 entry: packages/spec/src/migrations/entries/semantic/18.flow-binding-variable-dollar-name-refused.ts (protocol 18), plus its step-18 rationale fragment (order: 92).
  • Projections regenerated: gen:migration-registry, gen:spec-changes, gen:upgrade-guide.
  • No D2 conversion: the bare name may already be bound in the flow, and the reads of the old name sit in every dialect a flow string speaks.
  • Guidance: the printed guidance carries no tracker number. test/migrate-meta-engine-guidance.test.ts is green (3/3).
  • Changeset: .changeset/22502-flow-binding-variable-dollar-name-refused.md, @objectstack/spec major in pre mode, with Clause-②: no (narrowing) and the registered flow-binding-variable-dollar-name-refused marker. check-adr-0087-registration reads [major+BREAKING+clause-②-narrowing] registered flow-binding-variable-dollar-name-refused (new here).

Tests (at ff2832a7bb, after the one origin/main merge through os-regen-merge.sh and its regeneration commit)

  • New: packages/spec/src/automation/flow-bound-variable-name.test.ts, 26 cases.
    • $x is refused with the remedy on each of the five outputVariable contracts, and every $ name with it ($record, $error, $, $$x).
    • Controls: x, a$b and 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.x and $. Controls: the default $error, an explicit $error and caught are accepted.
    • FlowSchema refuses at nodes.1.config.errorVariable, and inside a region at nodes.1.config.try.nodes.0.config.outputVariable.
    • defineStack refuses with { code: 'STACK_SCHEMA_INVALID', status: 422 }, and the save door at the key.
    • textSlotTemplateRefusal('Failed: {{ caught.message }}') passes, and a flow binding caught with that text in its catch region parses clean.
    • The published JSON Schema carries a pattern on all six keys that accepts and refuses the same names.
    • The D3 entry and its rationale fragment are registered.
  • 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 the integration project. A first --project unit call selected nothing; that call is not counted as a measurement.
  • pnpm --filter @objectstack/spec typecheck and pnpm --filter @objectstack/service-automation typecheck both 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 import src directly, so no dist leg applies.

  • The rule's regex was replaced with one that accepts everything. The anchor fell from 1 to 0, and the blob moved from 5211426e99fb to d0cb32780409.
  • Result: 16 failed, 10 passed of 26. Red: every refusal, every door and the pattern pin. Green: the controls, the remedy-reads pin, the non-string case and the ledger pins.
  • Restore was proven: blob equals HEAD 5211426e99fb, and git diff HEAD is empty.

Reverse check of the services fixture:

  • 'caught' was put back to '$caught', then throw-arm-error-refresh.test.ts was run against the rebuilt spec.
  • Result: 1 failed, a ZodError at nodes.3.config.errorVariable thrown from AutomationEngine.canonicalizeStoredFlow inside registerFlow. So the fixture rename was owed.
  • Restore was proven: blob equals HEAD b4f38da11225.

Gates

Acceptance notes

  • The same class, on sibling binding keys, is not in this PR. FlowSchema.parse at this head still accepts a $ name on:

    • loop / map iteratorVariable and indexVariable;
    • a screen idVariable;
    • a declared flow variable's name;
    • an assignment target key.

    NotifyConfigSchema refuses a read of one ({{ $row.name }}). The triage ruling scoped this card to errorVariable and outputVariable, so the siblings are reported for the family close-out rather than widened here.

  • service-automation's executor descriptors (configSchema on crud-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's buildSubflowResumeSignal comment calls the reserved-name check a false positive "on an oddly-named" outputVariable. After this narrowing such a name cannot be authored. Whether engineBuilt is still needed there for another reason was not measured. Noted, not changed.

  • The hand-written content/docs/automation/flows.mdx gains one paragraph beside the try_catch example saying where the $ names belong.


Generated by Claude Code

…rrorVariable other than $error at authoring (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]>
@github-actions github-actions Bot added size/l documentation Improvements or additions to documentation tests tooling labels Oct 10, 2026
@github-actions

github-actions Bot commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/spec, touching 15 documentable anchor(s). ⚠️ 1 changed file(s) yielded no anchor (packages/spec/spec-changes.json), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

2 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/automation/flows.mdx (via errorVariable (literal, a string literal in FlowBindingKey; a string literal in TryCatchConfigSchema; a string literal in boundVariableNameRefusal; a string literal in flowBoundVariableNameSchema), outputVariable (literal, a string literal in CreateRecordConfigSchema; a string literal in FlowBindingKey; a string literal in GetRecordConfigSchema; a string literal in MapConfigSchema; a string literal in ScriptConfigSchema; a string literal in SubflowConfigSchema))
  • content/docs/kernel/runtime-services/examples.mdx (via outputVariable (literal, a string literal in CreateRecordConfigSchema; a string literal in FlowBindingKey; a string literal in GetRecordConfigSchema; a string literal in MapConfigSchema; a string literal in ScriptConfigSchema; a string literal in SubflowConfigSchema))
What this run could not see
  • 1 changed file(s) yielded no anchor (packages/spec/spec-changes.json) — pages documenting those are invisible to this run
  • 6 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 54 of 206 client-bound route-ledger rows — the other 152 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 152: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 97 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 139 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 96e4be482987d0acd55e7b9c3c957580fe7c21d8 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 5c9d1a66fd61d53618bf18085580df842bac695c — the merge of head dfe2e07ef34ae1a76873479c2910da2a7e296618 into base 96e4be482987d0acd55e7b9c3c957580fe7c21d8, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# 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

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 96e4be482987d0acd55e7b9c3c957580fe7c21d8 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: ff2832a7bb9768a311d803e72bc21b8d19637954
Local-runs: none

domain:spec seat 3 (#18883) · reviewer session session_01KNKBCRDJCu5tGy3TEbvtrF · 2026-10-10T02:00Z · PR #22569 (Fixes #22502). Inputs: the card body and its five comments (triage 6084430227, unlock 6091327685, claim 6091691863, report 6092326634, accept 6092376209), the PR body, its 17-file list, the net diff against merge-base 6a3f82efa (+535 / −29, 564 changed lines, no governed path, head repo = base repo), and the head's check-runs. Nothing built, run or re-run locally. Thread-read: 6092376209.

① Derived judgments

Every accept-set and public-surface change the diff implies, each judged against the triage direction (6084430227: one rule, the $ namespace is the engine's at every door; the text-slot judge is not widened):

  1. outputVariable on GetRecordConfigSchema, CreateRecordConfigSchema, MapConfigSchema, ScriptConfigSchema, SubflowConfigSchema — from any string to the pattern ^(?:[^$][\s\S]*)?$: a name whose first character is $ is refused, the engine's own names ($record, $error) included; the empty string, a$b, and an absent key stay accepted; a non-string keeps its type refusal. Right. The triage directs refusing a $-named outputVariable outright, and a binding over an engine name would shadow the trigger record for the rest of the run, so the prefix is the correct reading for a binding door.
  2. errorVariable on TryCatchConfigSchema — the pattern ^(?:\$error|[^$][\s\S]*)?$ with .default('$error') kept: $caught, $errors, $error.x and a bare $ are refused; $error explicit, the default, and caught stay accepted. Right — exactly the ruling's carve-out, and the default the engine binds anyway.
  3. The mechanism is a regex, not a .refine(), so the published JSON Schema (json-schema/, in the spec tarball's files) gains a pattern on the six keys and refuses what the parse refuses; dropped-refinements.baseline.json gains no row; the authorable-surface/automation.json ledger lists keys, not patterns, so it is unchanged and TryCatchConfig:errorVariable = "$error" stays in authorable-defaults. check:authorable-surface and check:docs are green (Type Check · source gates). Right — declared = enforced on the published schema.
  4. Refusal anchoring — the contract issue is invalid_format; flowNodeConfigRefusals re-emits it as custom at nodes.N.config.outputVariable / nodes.N.config.errorVariable, a region body included (nodes.1.config.try.nodes.0.config.outputVariable), and the pins cover the FlowSchema door, defineStack (STACK_SCHEMA_INVALID, 422) and the save door (getMetadataTypeSchema('flow')). The message names the remedy (caught, read as {{ caught.message }}, or delete the key and read {{ $error.message }}). Right — the same judge the service-automation: a built-in node's config value its own contract refuses still registers, then fails every run — the built-in half of #21848's class #21898 / build: a script node's undeclared config key passes objectstack validate, compile and registerFlow, then fails every run — the key half of #21898's class (subflow by reading) #21982 arms use, no second dialect.
  5. No new public export — flow-bound-variable-name.ts is imported by the three contract files only and is absent from automation/index.ts; check:api-surface is green (Type Check · consumer gates). Right. Shape note, not a finding: the spec tarball ships src/**/*.zod.ts, and this leaf is not a .zod.ts, so the shipped source tree does not carry it — the same already holds for flow-text-slot-template.ts, region-slots.ts and the shared/ helpers these files import; a pre-existing property of that surface, unchanged here.
  6. .describe() text on the six keys changed, so three reference pages (builtin-node-config.mdx, control-flow.mdx, schemaless-node-config.mdx) regenerate; check:docs green. Right.
  7. The text-slot judge and FLOW_ENGINE_VARIABLES are untouched (flow-text-slot-template.ts is not in the diff; the list stays unexported). The claim's "shared with spec(automation): a {{ $User.Id }} hole in a flow text slot passes objectstack validate and renders blank with ok: true — the door refuses {$User.Id} loudly but admits its {{ }} spelling silently #22477's closed list, not a second copy" is met by reading the prefix the list implies and copying nothing — stricter than the list for a binding, which is what the triage asks. Right; ruling 6084430227 honoured.
  8. ADR-0087 disposition — D3 semantic entry flow-binding-variable-dollar-name-refused at protocol 18 with no D2 conversion. Right: the bare name may already be bound in the flow and the reads sit in three dialects (a text-slot hole, a CEL expression, a single-brace token), so no rewrite could be proven. The entry's surface carries no backticks or pipes; the generated os-generated semantic:18 region of registry.ts holds the entry literal equal to the entry file field for field (surface 517, replacement 393, reason 1488, acceptanceCriteria 584 characters); the hand-written step-18 rationale fragment at order: 92 is unique on this head; spec-changes.json (both projections) and docs/protocol-upgrade-guide.md carry the entry and its re-joined step-18 paragraph. check:spec-changes and check:upgrade-guide (Type Check · source gates) and check:migration-registry (Lint & Repo Gates) are green on this head, so the post-merge regeneration commit is judged consistent.
  9. No engine change — try-catch-node.ts:105 still falls back to $error; the executors write the author's name verbatim through variables.set; a stored flow carrying a refused name is refused at registerFlow through canonicalizeStoredFlow's FlowSchema.parse and skipped with a warn, as the D3 entry states. Right.
  10. In-repo authors, re-measured on the head — outside test files every errorVariable is $error (flows.mdx ×2, app-showcase ×2, the service-automation README) and no $-named outputVariable exists; the three fixtures are renamed. Docs Drift Check's second page (kernel/runtime-services/examples.mdx) authors outputVariable: 'lines' / 'totals', unaffected. The Console Pin Gate is path-skipped; the dev's reading of the pinned objectui (errorVariable: '$error' only) is consistent with that. Right.
  11. The sibling binding keys (loop / map iteratorVariable and indexVariable, a screen idVariable, a declared variable's name, an assignment target) are not narrowed. Right — the triage scoped this card to two keys; escalated under ③.

② Semver level

  • Packages the diff publishes in: @objectstack/spec only. @objectstack/service-automation changes src/throw-arm-error-refresh.test.ts alone, and its files are dist, README.md, CHANGELOG.md — nothing published, no entry owed. content/docs/** does not publish. docs/protocol-upgrade-guide.md is spec's own projection (in spec's files) and is covered by spec's entry. Nothing new is published by any other package.
  • Spec's entry: .changeset/22502-flow-binding-variable-dollar-name-refused.md, '@objectstack/spec': major. What the diff publishes is an accept-set narrowing on six authorable keys of a published authoring surface, a pattern in the published JSON Schema, and a new D3 ledger entry — BREAKING. major is the strict-semver grade; .changeset/pre.json is mode: pre, tag: next, so check-changeset-no-major's RC exemption admits it (Check Changeset green), and four other spec major entries already sit on this head (22130-protocol-18 among them, with PROTOCOL_VERSION at 18.0.0 against spec 17.7.0, the lockstep shape that gate evidences), so this entry neither opens nor alone carries the major. The sibling 22477 entry graded the read-side narrowing minor under the launch-window convention; both grades are legal in pre mode, where the level is not the breaking carrier — the **BREAKING** sentence, the Clause-② line and the registered marker are, and this entry carries all three. Matches; no reconciliation owed in this PR.
  • The migration a breaking changeset must carry: the FROM → TO table, the one-line fix (drop the $, rename every read), why no D2 conversion, and "Who is affected, measured" naming the last published 17.7.0 and the three renamed fixtures. Present.
  • Clause-②: line — the PR body's second line and the changeset both read Clause-②: no (narrowing). The diff adds no key to a published payload and narrows an accept set, so no (narrowing) is the right arm; the ADR-0087 marker reads registered flow-binding-variable-dollar-name-refused as an HTML comment in the changeset body, the form check-adr-0087-registration reads ([major+BREAKING+clause-②-narrowing]). Right. skip-changeset is correctly absent.

③ Boundary flags

  • open_questions: none in report 6092326634.
  • Breach 1 — packages/services/service-automation/src/throw-arm-error-refresh.test.ts: the fixture hands registerFlow a flow with errorVariable: '$caught'; registerFlow parses through FlowSchema, so the new contract refuses that flow and the service-automation suite goes red without the rename. Owed. The edit is the fixture value and its comment only, no engine code, and the test's intent (a binding deliberately not $error, so the catch region reads the engine's run-wide $error) survives as caught. Accepted.
  • Breach 2 — content/docs/automation/flows.mdx: one paragraph beside the try_catch example (which authors errorVariable: '$error') saying the $ names are the engine's and a $-named errorVariable or outputVariable is refused at objectstack validate. Owed — the hand-written guide is where an author learns the rule, Docs Drift Check lists this page as naming both keys, and leaving it silent would advertise an accept set the runtime refuses. Accurate as written; "on any node" reads as "on any node that declares outputVariable" (an http node refuses the key itself) — a wording note, not a defect. Accepted.
  • Dev deviations, each answered: regex rather than refine (invalid_format at the contract, custom at the flow door) — fine, pinned both ways; prefix rather than list — fine (① 7); major beside the sibling's minor — fine (②); attribution in the AGENTS.md form — fine; one read-only npx tsx probe from the scratchpad — no repo effect; the os-regen-merge.sh landing as merge + regeneration commits — the generated files on this head are judged consistent (① 8); the worktree removed after push — fine.
  • out_of_scope_findings: item 1 (the sibling binding keys) is filed as spec(automation): the $ namespace at every binding door: loop and map iteratorVariable / indexVariable, a screen's idVariable, a declared flow variable's name and an assignment target still bind a $ name a text slot refuses to read #22572, open with finding, the family close-out card; items 2 and 3 (the executor descriptors' plain-string configSchema; the buildSubflowResumeSignal comment) are carried in its body. Escalation complete.
  • Landing note, not a verdict item: GitHub reports the PR mergeable: false (dirty) against origin/main at d6c37919c7. Four commits landed after the branch's one merge of main (feat(spec)!: a type: 'chart' list view whose effective binding names no dataset is refused at every list-view door #22528, feat(verify): the handle fronts the automation engine's condition evaluator #22553, feat(spec,lint,cli): a screen field's option labels and a flow's terminal toasts have keys in the flows translation face #22555, fix(cli): a refused Console mount answers 503 naming the remedy, not a bare 404 #22562) and touch five of this PR's files — registry.ts, spec-changes.json, docs/protocol-upgrade-guide.md, builtin-node-config.zod.ts and the builtin-node-config.mdx reference. A second os-regen-merge.sh merge plus its regeneration commit is owed before enqueue. This record names head ff2832a7bb only; whether the post-merge head needs a fresh record is the owning seat's call under the record-recognisers' pure-regeneration test.
  • Check-runs at this reading: every required context green — Lint & Repo Gates, the four Type Check lanes, Build Core, Dogfood Regression Gate, Temporal Conformance, Governed Surface Queue Guard — except Test Core shards 2/6 and 4/6, still in_progress (1/6, 3/6, 5/6 and 6/6 green); Console Pin Gate and Packed-tarball smoke are skipped by their path and opt-in filters. Enqueue waits on those two shards; the verdict below is the contract verdict.

Implemented-by: claude/issue-22502-dollar-variable-names
Reviewed-by: session_01KNKBCRDJCu5tGy3TEbvtrF

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]>
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: dfe2e07ef34ae1a76873479c2910da2a7e296618
Local-runs: none

domain:spec seat 3 (#18883) · reviewer session session_01KNKBCRDJCu5tGy3TEbvtrF · 2026-10-10T03:17Z · PR #22569 (Fixes #22502), round 2 on the post-sync head. Inputs: the card body and its eight comments (triage 6084430227, unlock 6091327685, claim 6091691863, report 6092326634, accept 6092376209, seat order 6092496540, round-2 report 6093134621, seat note 6093148207), the PR body, its two comments (Docs Drift Check 6092280868, the round-1 record 6092477252 at ff2832a7bb), its 17-file list, the net diff against merge-base d6c37919c7 (+535 / −29, 564 changed lines, no governed path, head repo = base repo), and the 35 check-runs on this head. Nothing built, run or re-run locally; every comparison below is a git diff over fetched refs and a read of the fetched files. Thread-read: 6093148207.

Why this round exists, and what it adds. The head moved from ff2832a7bb by one os-regen-merge.sh landing of main at d6c37919c7: merge eedc23f313, step-3 commit f3a9339aea, regeneration commit dfe2e07ef3. The seat's pure-regeneration test names two paths it cannot explain, builtin-node-config.zod.ts and registry.ts. Both are answered in ① 11 and ③. The PR's own +/− line set is byte-identical to round 1 on all 17 files (each file's +/− lines of 6a3f82efa7..ff2832a7bb hashed equal to those of d6c37919c7..dfe2e07ef3), so every judgment of 6092477252 is re-read here against the new base rather than carried on trust.

① Derived judgments

Every accept-set and public-surface change the net diff implies, each judged against the triage direction (6084430227: one rule, the $ namespace is the engine's at every door; the text-slot judge is not widened):

  1. outputVariable on GetRecordConfigSchema, CreateRecordConfigSchema, MapConfigSchema, ScriptConfigSchema, SubflowConfigSchema — from any string to flowBoundVariableNameSchema('outputVariable').optional(), the pattern ^(?:[^$][\s\S]*)?$: a name whose first character is $ is refused, the engine's own names included; the empty string, a$b and an absent key stay accepted; a non-string keeps its type refusal. Nothing is widened. Right — the triage refuses a $-named outputVariable outright, and a binding over $record would shadow the trigger record for the rest of the run, so the prefix is the correct reading at a binding door.
  2. errorVariable on TryCatchConfigSchema — the pattern ^(?:\$error|[^$][\s\S]*)?$ with .default(ENGINE_ERROR_VARIABLE) ('$error') kept, so authorable-defaults is unchanged and control-flow.mdx still renders default: "$error": $caught, $errors, $error.x and a bare $ are refused; the default, an explicit $error and caught stay accepted. Right — exactly the ruling's carve-out, and the default the engine binds anyway.
  3. The mechanism is a regex, not a .refine(). packages/spec/json-schema/ is gitignored and written by gen:schema at build, then shipped in spec's files; the pin reads z.toJSONSchema on all six contracts and holds a pattern that accepts and refuses the same names as the parse. dropped-refinements.baseline.json gains no row; authorable-surface/, api-surface/, liveness/ and export-origins/ are untouched by the diff (zero files), which is right for a key whose name and optionality did not change. check:authorable-surface, check:docs and check:api-surface are green on this head. Right — declared = enforced on the published schema.
  4. Refusal anchoring — the contract issue is invalid_format; flowNodeConfigRefusals re-emits it as custom at nodes.N.config.outputVariable / nodes.N.config.errorVariable, a region body included (nodes.1.config.try.nodes.0.config.outputVariable), and the pins cover FlowSchema, defineStack (STACK_SCHEMA_INVALID, 422) and the save door (getMetadataTypeSchema('flow')). The message names the remedy (caught, read as {{ caught.message }}, or delete the key and read {{ $error.message }}). Right — the same judge the sibling arms use, no second dialect.
  5. No new public export — flow-bound-variable-name.ts is imported by builtin-node-config.zod.ts, control-flow.zod.ts and schemaless-node-config.zod.ts only, and automation/index.ts does not name it; check:api-surface (Type Check · consumer gates) is green. Right. The round-1 shape note stands: the spec tarball ships src/**/*.zod.ts, and this leaf is not a .zod.ts, the same pre-existing property flow-text-slot-template.ts and region-slots.ts already have.
  6. .describe() text on the six keys changed, so three reference pages regenerate (builtin-node-config.mdx, three outputVariable rows; control-flow.mdx, the errorVariable row; schemaless-node-config.mdx, two rows); check:docs green. Right.
  7. The text-slot judge is untouched — flow-text-slot-template.ts is not in the file list; FLOW_ENGINE_VARIABLES on this head is the same closed list of seven ($record, $runId, $flowName, $flowLabel, $error, $loopItems, $loopIndex), still unexported. The rule reads the prefix that list implies and copies nothing, stricter than the list for a binding, which is what the triage asks. Right; ruling 6084430227 honoured.
  8. ADR-0087 disposition — D3 semantic entry flow-binding-variable-dollar-name-refused at protocol 18, no D2 conversion (the bare name may already be bound, and the reads sit in three dialects, so no rewrite is provable). Read field for field on this head: the entry file and the os-generated semantic:18 region of registry.ts carry the same literal (surface 517, replacement 388, reason 1482, acceptanceCriteria 584 characters once the escaped apostrophes are unescaped); both records in spec-changes.json equal the entry on surface, replacement and rationale with toMajor: 18; the upgrade guide's step-18 D3 bullet carries the surface, the reason as "Why not automatic" and the acceptance criteria as "Done when", and the hand-written rationale fragment (order: 92) is re-joined into the step-18 paragraph. The fragment sits where its id sorts, between flow-approval-… and flow-builtin-…, which is the invariant the registry's own test holds (ids sorted and unique; order places the sentence and duplicates are tolerated, and none exists for 92 on main anyway). check:migration-registry (Lint & Repo Gates) and check:spec-changes / check:upgrade-guide (Type Check · source gates) are green. One correction to how round 1 phrased the last two gates: under ADR-0087 D4 ruling B′, built at #22533 and so already in force at this PR's base, those two gates generate in memory and read no committed copy; the committed spec-changes.json and docs/protocol-upgrade-guide.md lines are the no-flag generators' output, kept until spec(changes): delete the committed spec-changes per-major projection and the upgrade guide copy, with their two merge=os-regen routes, once generation at publish has landed (#22449 B′) #22485 deletes them, and the publish lane regenerates both. They are consistent with the entry (above), and nothing lands or ships from them. Right.
  9. No engine change — try-catch-node.ts:105 still reads cfg.errorVariable || '$error'; crud-nodes.ts, map-node.ts, screen-nodes.ts and subflow-node.ts write the author's name verbatim through variables.set; a stored flow carrying a refused name is refused at registerFlow through canonicalizeStoredFlow, as the D3 entry states. Right.
  10. In-repo authors, re-measured on this head — git grep -nP 'Variable\W{0,8}\$[a-zA-Z_]' over the whole tree: 30 hits; outside $error, every hit is this PR's own test cases, the changeset's FROM → TO table, the rule's module comment and the D3 entry text. No example, doc, skill or app binds a $-named outputVariable or a non-$error errorVariable. The three fixtures are renamed. Right.
  11. The merge hop, on the two unexplained paths. builtin-node-config.zod.ts: this PR's +/− set is 23 lines and identical between rounds; the hop ff2832a7bb..dfe2e07ef3 on that file is 71 lines and identical to main's own 6a3f82efa7..d6c37919c7 change on it, so the hand-resolved conflict kept both import blocks (flowBoundVariableNameSchema from this PR, flowScreenFieldOptionKey from feat(spec,lint,cli): a screen field's option labels and a flow's terminal toasts have keys in the flows translation face #22555) and nothing else. registry.ts: this PR's set is 62 lines and identical between rounds; the hop is 53 lines and identical to main's. Neither path carries anything beyond this PR's own lines plus main's. The same identity holds for the other 15 files. Right — the hop is main plus this PR, and the regeneration commit restored exactly this card's projection lines.
  12. The sibling binding keys (loop / map iteratorVariable and indexVariable, a screen idVariable, a declared variable's name, an assignment target) are not narrowed. Right — the triage scoped this card to two keys; spec(automation): the $ namespace at every binding door: loop and map iteratorVariable / indexVariable, a screen's idVariable, a declared flow variable's name and an assignment target still bind a $ name a text slot refuses to read #22572 carries them.

② Semver level

  • Packages the diff publishes in: @objectstack/spec only. @objectstack/service-automation changes src/throw-arm-error-refresh.test.ts alone, and its files are dist, README.md, CHANGELOG.md — nothing published, no entry owed. content/docs/** does not publish. docs/protocol-upgrade-guide.md at the repo root is the committed copy; what ships is packages/spec/protocol-upgrade-guide.md, generated at publish from the registry this PR extends, so spec's entry covers it. Nothing new is published by any other package.
  • Spec's entry: .changeset/22502-flow-binding-variable-dollar-name-refused.md, '@objectstack/spec': major. What the diff publishes is an accept-set narrowing on six authorable keys, a pattern in the published JSON Schema and a new D3 ledger entry — BREAKING, and major is the strict-semver grade. .changeset/pre.json on this head is mode: pre, tag: next, so the RC exemption admits it (Check Changeset green), and five other spec major entries sit beside it (19939-…, 22110-…, 22130-protocol-18, 22158-…, 22491-…), so this entry neither opens nor alone carries the major. main has since graded the same family's next narrowing (feat(spec,service-automation)!: refuse {$User.*} in flow value slots, and bind current_user in the flow CEL scope (#19939 pass 2) #22563, {$User.*} refused) major too, so the round-1 note about the minor sibling (spec(automation): a {{ $User.Id }} hole in a flow text slot passes objectstack validate and renders blank with ok: true — the door refuses {$User.Id} loudly but admits its {{ }} spelling silently #22477) no longer leaves this entry as the odd one out. The breaking carriers are all present: the **BREAKING** sentence, the Clause-②: no (narrowing) line, and the adr-0087: registered flow-binding-variable-dollar-name-refused marker in the HTML-comment form the gate reads (check-adr-0087-registration is inside the green Lint & Repo Gates). Matches.
  • The migration a breaking changeset must carry: the FROM → TO table, the one-line fix (drop the $ and rename every read), why no D2 conversion, and "Who is affected, measured" naming the last published 17.7.0 and the three renamed fixtures. Present.
  • Clause-②: line — the PR body's second line and the changeset both read Clause-②: no (narrowing). The diff adds no key to a published payload and narrows an accept set, so no (narrowing) is the right arm. Right. skip-changeset is correctly absent.

③ Boundary flags

Implemented-by: claude/issue-22502-dollar-variable-names
Reviewed-by: session_01KNKBCRDJCu5tGy3TEbvtrF

VERDICT: PASS


Generated by Claude Code

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

Landing pre-checks at dfe2e07ef3, by the owning seat: queued under the maintainer's rule-A relaxation

domain:spec seat 3 (#18883) · zhuangjianguo · session session_01KNKBCRDJCu5tGy3TEbvtrF · 2026-10-10T03:19Z · holder of claim 6091691863.

  • The review: contract review PASS 6093227778, round 2, names this head. Nothing it records blocks. Round 1 (6092477252) passed at ff2832a7bb. The sync hop was not a pure regeneration, which is why round 2 ran. Round 2 finds that the hop's lines in builtin-node-config.zod.ts and registry.ts are exactly this PR's lines plus main's.
  • CI: 33 success, 2 skipped. check-expected-skips --pr 22569 reads "OK — 2 skipped check-run(s), every one in the roster". Governed Surface Guard is green.
  • Governed: check-governed-merges --pr objectstack-ai/objectstack#22569 reads 0 of 17 paths on the register, NOT governed; 564 changed lines, under 3,000.
  • Closing keywords: the body carries Fixes #22502 alone, and no commit message carries one.
  • main drift since the sync base d6c37919c7: GitHub reports mergeable: true, mergeable_state: clean. Round 2 read the later main commits against this PR's four shared files and found the hunks disjoint.
  • Why no further sync: the maintainer's ruling recorded at 6091886885 on spec(changes): delete the committed spec-changes per-major projection and the upgrade guide copy, with their two merge=os-regen routes, once generation at publish has landed (#22449 B′) #22485 (correction 6091996837). A D3-entry PR that GitHub reports clean goes to the queue, and the merge group judges the merged tree. An ejection brings it back for a sync.

needs:contract-review comes off; pr_ready and automerge_enable follow.


Generated by Claude Code

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review October 10, 2026 03:21
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge October 10, 2026 03:21
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Oct 10, 2026
Merged via the queue into main with commit 3e72f93 Oct 10, 2026
40 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/l tests tooling

Projects

None yet

2 participants