Repository navigation
fix(app-shell): a flow node rename carries the expressions that read its outputs, and Problems names a reference to a missing node (objectui#11838) - #11850
Merged
objectstack-fleet[bot] merged 4 commits intoOct 8, 2026
Conversation
…its outputs, and Problems names a reference to a missing node (objectui#11838) A rename now rewrites every expression reference whose root is the old id (CEL member roots, template holes in both spellings), read through parseCelToAst and the engine's template hole grammar, in the same patch as the edges and boundary host. A reference that does not parse, or a root that is also a variable name, refuses the rename and names it. flow-node-refs.ts holds the one list of node-id positions (edge source and target, boundary host, expression root); missingNodeRefDiagnostics in flow-sim-validate.ts adds the Problems error rows for an expression root and a boundary host naming a missing node. The enumeration pin runs every designer write against the list. Claude-Session: https://claude.ai/code/session_01DrKzdPdyLLBW3qpZ4vtk7z Co-authored-by: Claude <noreply@anthropic.com>
…node error row (objectui#11838) `ghost.ok` in an autolaunched flow reads a node the flow does not have, so beside the scope warning the Problems panel lists the new engine.flowValidate.exprRefNodeMissing error; the control asserts both. Claude-Session: https://claude.ai/code/session_01DrKzdPdyLLBW3qpZ4vtk7z Co-authored-by: Claude <noreply@anthropic.com>
…s alone precisely (objectui#11838) "A loop variable" read like a flow loop node's iterator, which a rename refuses as ambiguous rather than leaves alone; the untouched case is a variable the expression declares itself, as in rows.exists(x, x.ok). Claude-Session: https://claude.ai/code/session_01DrKzdPdyLLBW3qpZ4vtk7z Co-authored-by: Claude <noreply@anthropic.com>
Contributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
This was referenced Oct 8, 2026
… or an expression still names the node (objectui#11838) The removal carries a node's edges but has no new id for a boundary event's host or an expression root to follow, so it left them naming a node that no longer exists. nodeRemovalRefusal (flow-problems.ts) is the one rule both removal gestures apply: the inspector's Remove node shows the refusal under the button, and the canvas Delete key at the top of the canvas's inline alert stack, each naming every site in the rename refusal's form; nothing is written and the node stays selected. The enumeration pin's removal of the node every position names flips from reported to refused: after every designer write, no position names a missing node. Claude-Session: https://claude.ai/code/session_01DrKzdPdyLLBW3qpZ4vtk7z Co-authored-by: Claude <noreply@anthropic.com>
Contributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
objectstack-fleet
Bot
deleted the
claude/issue-11838-rename-expression-refs
branch
October 8, 2026 02:19
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #11838
Clause-②: no
What was wrong
Renaming a node in the flow designer's ID field carried its edges and a boundary event's host (objectui#11827), but not the expressions that read the node's outputs. The engine writes every node output under the node's id, so a later node reads it by that id:
x.decision == 'approve'on a decision branch and its edge guard, or{x.field}in a record field value. After a rename fromxtorenamed, all of these still namedx. The saved draft read a node that no longer existed, and the Problems panel showed only the scope warning.Measured on this branch through the real
FlowNodeInspector: withFlowNodeInspector.tsxreverted to the base commit, the card's pin receives"expression": "x.decision == 'approve'"where"renamed.decision == 'approve'"is expected. The reverse checks are below.The design
A rename rewrites every expression reference whose root is the old id, through the expression parsers. This is
expressionRefsAfterNodeRenamein the new modulepreviews/flow-node-refs.ts.FlowNodeInspectorcommits it in the same patch asedgesAfterNodeRenameandboundaryRefsAfterNodeRenamefrom objectui#11827. One builder (renamePatch) feeds both the field's refusal and the commit, so the field refuses exactly what the commit could not carry.parseCelToAstfrom@objectstack/formula. A reference is anidnode of that AST that no comprehension macro (all/exists/exists_one/map/filter) orcel.bindbinds. Only the root identifier's own characters are replaced, at the positions the AST reports, so the rest of the author's text keeps its exact bytes.cond ? value : nullwith itsdynwrap. For these, the AST's identifiers are placed back on the author's text by order, and only when the names match one for one; otherwise the source counts as unparsed.interpolateString. That pattern takes the inner{x.field}of a{{x.field}}, so both spellings read the same reference. A hole's content is read with the engine's dotted-path grammar fromresolveToken; when the content is arithmetic, it is read as CEL. The| formatterof a double-brace hole is not part of the reference. No spelling is inserted (objectui#11824 is not touched).xy,ax), a member name (y.x), a string literal ("x.decision"), a macro-boundx, and a script body. A script body is a code slot and no parser here reads it.What is refused. In each case the field shows the message and the stored id again, and nothing is written. The refused expressions are listed in a locale-free form: the node or edge, then the path, then the expression.
engine.inspector.flowNode.idRefsUnparsed: an expression, or a template hole, reads the old id as a root but does not parse. No parser can say where the reference is, so the rename refuses and names it.engine.inspector.flowNode.idRefsAmbiguous: the old or new id is also a declared variable or a root that every run binds (record,previous,vars, or a name starting with$). The engine nests both the variable and the node's outputs under the same name, so the reference could mean either one. This also covers a new id that a comprehension macro in that expression already binds, which would capture the reference.A removal is refused while a boundary event's host or an expression root still names the node (revision 1, the review's ruling B). A removal carries the node's edges (
edgesAfterNodeRemoval, objectui#11772) but has no new id for the other two kinds of position to follow, so it would leave them naming a node that does not exist.nodeRemovalRefusalinflow-problems.ts, besideedgesAfterNodeRemoval, is the one rule both removal gestures apply:FlowNodeInspector'sremove("Remove node") andFlowCanvas'sdeleteNode(the Delete and Backspace keys). It readsnodeIdPositionson the draft before the removal and returns each boundary event whose host is the node and each expression with a reference rooted at it, ornull.decision), and a boundary event inside the removed node. The edge in that a splice reconnects keeps its guard, so a reference there is counted. A duplicate id (another node still carries it) refuses nothing, the rule the edge half already applies. A source that does not parse names no position, so it does not refuse; the expression checks already report it as malformed.engine.inspector.flowNode.removeRefused(EN and ZH) names each site in the rename refusal's locale-free form, for examplebe › boundaryConfig.attachedToNodeId: `x`; d › config.conditions[0].expression: `x.decision == 'approve'`. Nothing is written, and the node stays selected.role="alert". The canvas Delete key had no refusal surface. Measured:FlowCanvashas one alert surface, its inline banner stack at the top left where structural errors already show (role="alert"rows);FlowPreviewhas none; toasts are used by page-level metadata-admin components (ResourceEditPage,PackagesPageand others), never by the canvas, and would be transient. The smallest honest surface is therefore arole="alert"row at the top of that existing stack: no new dependency, and it stays while the node is selected. A silent no-op was not an option.The list of node-id positions is
nodeIdPositionsinflow-node-refs.ts. It holds edgesourceandtarget, a boundary event'sboundaryConfig.attachedToNodeId, and an expression reference's root, at every depth, including the regions of loop, parallel and try/catch containers. An expression root counts as a position in two cases:x.decision), and no node, declared variable or runtime root answers to it. The start node's own expressions are excluded from this case, because its entry condition reads the trigger record's fields bare.The list lives in the new module, not in
flow-problems.ts: the Problems rows inflow-sim-validate.tsread the same list, andflow-problems.tsalready imports that file, so putting the list there would create an import cycle. The new exports are module-internal: the package'sexportsmap names only.and./styles.css, andsrc/index.tsre-exports none of these modules."An error, not a warning" is met in the problems check.
missingNodeRefDiagnosticsinflow-sim-validate.tsadds two rows besideedgeSourceMissing:engine.flowValidate.exprRefNodeMissing: an expression root that names a missing node, shown on the top-level node or edge that holds it;engine.flowValidate.boundaryHostMissing.buildFlowProblemslists both as errors. The scope warning inflow-ref-check.tsis not changed.Placement, stated as a deviation from the letter of the ruling. The rows sit beside
validateFlowDraftin the same file, not inside it.validateFlowDraftis also the debugger's Run preflight, which refuses whatregisterFlowrefuses. The platform registers a flow that has either of these, and a missing root faults only when its node runs, so the debugger's Run is unchanged.The boundary-host row is a bounded in-place fix in a file already on the claim: the same defect class (a position names a missing node and no row reports it), a mechanical row shaped like
edgeSourceMissing, the same gate family, and the enumeration pin needs it, because without it the pin has nothing to find for the boundary-host position.The enumeration pin:
FlowPreview.nodeIdPositions-11838.test.tsxThe pin runs a fixture flow through the real canvas and inspector. The fixture holds every kind of position, each naming the approval
x. Each designer write is eitherapplied(the draft changed) orrefused(the draft is the very same object, and the surface that refused shows the refusal naming each site). After every write, no position names a missing node (missingNodePositionsis empty), and the Problems check names none of the fixture's positions as missing. A new kind of position fails the fixture control (nodeIdPositionsof the fixture must hold every kind inNODE_ID_POSITION_KINDS) until the fixture holds one, and every write then runs against it.Designer write paths (H4):
addNode: a node card's "Add connected node", and the toolbar's Add node throughaddAfter;insertOnEdge: "Insert node here";addReviseLoop: "Add revision loop".FlowNodeInspector's "Remove node", andFlowCanvas's Delete and Backspace keys (the pin drives Delete). Both useedgesAfterNodeRemoval.FlowCanvas,FlowPreview,flow-canvas-parts,FlowInspectorandFlowNodeInspectorfor duplicate, clone, copy and paste finds no write that copies a node.Every add, every rename, and every removal of a node that no position names is
appliedand leaves nothing behind. Removing the node that every position names isrefusedby both gestures, naming the boundary event and the three expressions that readx; it flipped fromreportedin revision 1. A further test pins that neither refusal is left standing once another node is selected.Expression positions read (H2), derived from the descriptors and the spec's ledger
Positions are read in this order (
fieldsForNodeTypefield kinds andFLOW_NODE_EXPRESSION_PATHS, both read-only):refMode: 'expression': the script body.kind: 'expression'that is not a template: the start node'sconditionandcriteria, the decision node'scondition, andlegacy_action'srecordId;expressioncolumn of anobjectListfield: a decision branch'sexpression, a screen field'svisibleWhen;predicateslot inFLOW_NODE_EXPRESSION_PATHS(the same two columns).schedule.expressionis one, so a cron string is never read as CEL);expressionfield markedrefMode: 'template': the loop and mapcollection;flow-templateslot in the ledger (the same two).conditionorexpressionkey, following the platform linter'sCEL_KEYSrule, including keys no descriptor describes.configand inconnectorConfig.input. The engine interpolates{token}holes into node config strings wholesale.An edge's
conditionis CEL. A{ dialect, source }envelope anywhere is read by its own dialect, for example thevalueslots ofassignments,create_recordfields andupdate_recordfields.Positions whose kind cannot be derived, or that this rename does not carry (listed for the record, not changed here):
expressionstores its CEL in theapprovers[].valuecolumn, of kindreference. Its roots are closed (current,trigger,vars), so it names a node as a member,vars.x.key, not as a root.vars.x.keyhas a root that is not the node id. The ruling covers roots only, so the rename does not carry these.waitEventConfig, and strings inboundaryConfigother than the host, are not walked.conditionandexpressionkeys read as CEL, and its otherconfigstrings as templates.targetcolumn is virtual. It is stored on the edges.Tests and gates (all on
c7b3699unless marked)flow-node-refs.renameExprRefs-11838.test.ts: 20 tests;FlowNodeInspector.renameExprRefs-11838.test.tsx: 5 tests;FlowPreview.nodeIdPositions-11838.test.tsx: 10 tests (the fixture control, the 8 writes, and the refusal-not-left-standing test);flow-problems.removalRefusal-11838.test.ts: 8 tests (the sites named, the message in EN and ZH, an edge-only node, a dropped guarded out-edge, a spliced edge in, a container's own regions, a duplicate id, an unparsed source);FlowNodeInspector.removeRefusal-11838.test.tsx: 4 tests (the refusal in EN and ZH with nothing written and the node still selected, the refusal clearing once nothing names the node and the removal then going through, and an edge-only node removed at once).FlowPreview.connectorOutputProblems-11085.test.tsx("a root no node writes is still reported"). In that autolaunched flow,ghost.okreads a node the flow does not have, so the panel now lists the new error beside the scope warning, and the test asserts both rows.pnpm exec vitest run --maxWorkers=2over 78 files: everyflow-problems*,FlowNodeInspector*,FlowCanvas*,FlowPreview*, simulator andflow-expr-problemstest, every test that imports a touched module (includingFlowCanvasandFlowInspectorimporters, among them the Studio design-surface suites), and the metadata-admin i18n string tests. Result:Test Files 78 passed (78),Tests 1209 passed (1209), lock verdictcommand-exit 0.pnpm --filter @object-ui/app-shell type-check:command-exit 0. Its chainedtsconfig.test.jsonincludessrc/**/*.test.tsandsrc/**/*.test.tsx, so it compiles all six touched test files.pnpm --workspace-concurrency=2 --filter '@object-ui/app-shell^...' build(29 of 47 workspace projects):command-exit 0.check:*gates, each exit 0:control-bytes,new-line-citations,changeset-claims,pending-changeset-literals,i18n-designer-parity;i18n-keys,i18n-drift,i18n-dead-keys,spec-symbols,phantom-deps,unused-deps,self-import,esm-specifiers,test-path-roots,vi-mock-specifiers,vi-mock-inherit,vi-mock-override-shape,unreferenced-sources,handler-key-reads,metadata-write-doors,designer-field-key-parity,comment-mask-corpus;scripts/check-changeset-presence.mjsandscripts/check-changeset-no-major.mjs.check:eager-closure, reason: it reads a built console bundle (apps/console/dist/eager-closure.json), which was not built locally; it printed PREREQUISITE NOT MET. Left to CI.eslint --no-inline-config --format jsonover the 12 changed.tsand.tsxfiles. The json lists 12 files, 0 errors and 7 warnings. All 7 are onFlowNodeInspectorandFlowCanvaslines outside this diff's hunks, and the same rules fire on the earlier copies of those files (four onFlowNodeInspectorat the base commit, threeexhaustive-depsonFlowCanvasat252b045), at lines shifted by the inserted code. Type-aware linting is not enabled (the**/*.{ts,tsx}block ofeslint.config.jssets noparserOptions.projectorprojectService), so this diff cannot change the result for an untouched file. The customobject-ui/*rules were not audited for cross-file reads. The repo-widepnpm lintis left to CI.Reverse checks. Each one reverts committed files, runs the pins under the lock, and restores with
git checkout HEADinside a trap. The restore is proven by an emptygit diff HEADand a blob hash equal to HEAD's.252b045,FlowNodeInspector.tsxreverted to the base: 5 failed and 11 passed."expression": "x.decision == 'approve'"where"renamed.decision == 'approve'"is expected.expected [ Array(1) ] to deeply equal []).expression-root: none: expected true to be false.252b045,flow-problems.tsreverted to the base: 4 failed and 27 passed. "it reaches the Problems panel as errors, not only the scope warning" fails withexpected [] to deeply equal [ 'error', 'error' ], and both removal legs and the panel test find no error row.flow-problems.tschanged after this run only by additions (git diff --stat 252b045 c7b3699on it: 90 insertions, 0 deletions), so the Problems wiring this check proved is unchanged.c7b3699,FlowNodeInspector.tsxreverted to the base again: 7 failed and 8 passed. The card's pin again receives"expression": "x.decision == 'approve'", and the inspector's refused removal leg now fails too; the canvas's refused leg stays green, becauseFlowCanvas.tsxwas not reverted.c7b3699:FlowNodeInspector.tsxandFlowCanvas.tsxboth reverted to252b045(markernodeRemovalRefusal4 and 3 before, 0 and 0 after; both blobs equal to252b045's), with the helper left in place: 6 failed and 16 passed. Both refused legs of the enumeration pin fail witha refused write changes nothing, the inspector refusal tests find a removal that landed (expected [ Array(1) ] to deeply equal [],expected null not to be null), and the not-left-standing test finds the Problems banner rows of an applied removal instead of the refusal. All 8 helper tests and every control stay green.False positives measured on real flows. Over the 35 flows of the objectstack example apps (showcase, CRM and todo, objectstack
bafb58bb), which hold 435 expression sites, the parser reads 161 references (111 of them member reads; the most frequent roots arerecordandprevious). None of them is an expression-root position, and the new rows fire 0 times. Positive control: injecting a decision that readsloop_tasks.decisionintoshowcase_batch_remindersand removingloop_tasksmakes the list report it.Acceptance notes
validateFlowDraftreads only top-level edges. The designer has no nested structural editing, so only hand-written JSON can produce one. Noted, not filed.field.keythat reads an object-valued trigger field on a non-start node would be reported as a missing node. The designer's picker writesrecord.fieldthere, the start node is exempt, and the 35 example flows report none.RUNTIME_ROOTSinflow-node-refs.tsmirrors the module-privateRUNTIME_GLOBALSofflow-ref-check.ts, which this claim may import but not edit. Exporting that set fromflow-ref-check.tswould remove the copy; the natural carrier is objectui#11789, which owns that file.Fence
Every changed file is on the claim's surface:
flow-node-refs.ts, the new module;FlowNodeInspector.tsx, and (revision 1) itsremovepath and the refusal under the Remove node button;FlowCanvas.tsx'sdeleteNodeand the refusal row at the top of the canvas's inline alert stack;flow-sim-validate.ts;buildFlowProblemswiring inflow-problems.ts, and (revision 1) the removal helpernodeRemovalRefusalwith its message builderdescribeNodeRemovalRefusal;engine.flowValidate.*andengine.inspector.flowNode.*rows ini18n.ts, en and zh (revision 1 addsengine.inspector.flowNode.removeRefused);FlowPreviewcontrol;.changeset/11838-rename-expression-refs.md(@object-ui/app-shellpatch). Revision 1 also edits its text, outside the revision's listed surface, because its example "x.decisionafterxis removed" became untrue: it now states the removal rule.No package export, prop,
@object-ui/typesmember orpackages/i18nkey is added.flow-scope.ts(nodeOutputRefs) andflow-node-config.ts(fieldsForNodeType) are imported read-only;flow-ref-check.tsis neither imported nor edited. No file of objectui#11783 or objectui#11789 is edited.Written by the os-dev agent of session
https://claude.ai/code/session_01DrKzdPdyLLBW3qpZ4vtk7z(revision 1 in the same session).Generated by Claude Code