Repository navigation
fix(app-shell): renaming a flow node carries its edges, and commits only a finished, unique id (objectui#11827) - #11837
Merged
objectstack-fleet[bot] merged 3 commits intoOct 7, 2026
Conversation
…nly a finished, unique id (objectui#11827)
The node inspector's ID field committed patchNode({ id }) on every
keystroke with only `nodes` in the patch: every edge naming the old id
was left dangling, and the selection kept naming the old id, so the
inspector lost the node after one character.
- The ID field holds a local draft and commits on blur or Enter; Escape
reverts; an IME-composition Enter is not a commit.
- A finished id is refused inline when it is empty, when another node in
the flow's one id space has it (regions included, walked with the
spec's collectFlowGraphs), or when an edge still names it.
- One patch carries the node's new id, every edge endpoint that named the
old id (edgesAfterNodeRename) and a boundary event's host
(boundaryRefsAfterNodeRename); the selection follows the node.
Claude-Session: https://claude.ai/code/session_01CGZy1BGCjdN5cXqL9cnvB8
Co-authored-by: Claude <noreply@anthropic.com>
…11827) Claude-Session: https://claude.ai/code/session_01CGZy1BGCjdN5cXqL9cnvB8 Co-authored-by: Claude <noreply@anthropic.com>
objectui#11779 landed first and edited flow-problems.ts: both imports are kept (createContext beside collectFlowGraphs), and the edit-writers section header keeps this branch's wording with main's "component-free". Claude-Session: https://claude.ai/code/session_01CGZy1BGCjdN5cXqL9cnvB8 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 7, 2026
objectstack-fleet
Bot
deleted the
claude/issue-11827-flow-node-rename-edges
branch
October 7, 2026 23:53
This was referenced Oct 7, 2026
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
1 participant
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 #11827
Clause-②: no
What was wrong, measured on
main(32124ac)FlowNodeInspector's ID field was the sharedInspectorTextFieldwithonCommit={(v) => patchNode({ id: v })}. That field commits fromonChange, so every keystroke was a rename of its own, and the patch carriednodesonly: every edge naming the old id kept naming it.This PR's pins were first run with the tests added and the source unchanged (
32124ac): 31 failed, 4 passed (35). There were two failure signatures:after typing r: expected [ { nodes: [ …(3) ] } ] to deeply equal []: the first character typed was already a rename.Unable to find a label with the text of: ID, right after the first change event. The selection still named the old id, solocateFlowNodefound nothing and the inspector dropped to its empty state after one character. Through the real canvas and inspector,s → x → erenamedx → renamedleft both edges dangling, and the Problems panel listed them.The 4 that passed on
mainare the controls and are meant to pass: a label edit, read-only, a nested node's read-only id, and a duplicate carrier keeping its edges.The design
The field: a draft that commits a finished id
A local
FlowNodeIdFieldinFlowNodeInspector.tsxholds what the author types and commits on blur or Enter. Escape puts the stored id back. An Enter that ends an IME composition is the composition's, so it does not commit. The sharedInspectorTextFieldis untouched: every other inspector field still commits on change.A refused id is never handed to the write. The field shows the stored id again, and the reason sits under it in the flow inspectors' existing refusal line (
role="alert", the destructive text styleFlowEdgeInspectorandFlowNodeConfigFielduse), witharia-invalidandaria-describedbyon the input. The reason stays until the author edits again.What is refused, and where each rule comes from
nodeRenameRefusalsits inflow-problems.tsbesidefreshNodeIdandedgesAfterNodeRemoval.FlowSchemarefuses a duplicate, and a flow has ONE node id space, the top-levelnodes[]plus every region body at every depth. The id space is walked with the spec's own exportedcollectFlowGraphs, not a hand-rolled walk. Measured on the installed@objectstack/spec17.7.0: a loop-body node reusing a top-level id is refused atnodes.1.config.body.nodes.0.id.FlowNodeSchema.idis a barez.string(). Measured:FlowSchema.safeParseaccepts the empty string, a single space anda b. The designer's own structural check (validateFlowDraft, theengine.flowValidate.nodeMissingIdrow) already reports a node with no id as an error, so the designer's own write never makes one. No format rule is invented beyond that.freshNodeIdcounts edge endpoints as taken. The node would gain a connection the author never drew, or a second copy of one it has (a repeated connection, which this same module flags). The alternative, allowing it as a "repair by rename" gesture, was rejected because it re-attaches silently. The message tells the author to remove or reconnect that edge first.What one patch carries
edgesAfterNodeRename, besideedgesAfterNodeRemovaland mirroring its signature. Onlysource/targetmove. Each edge keeps its id, condition, label,isDefault,typeand its position in the list (the order a decision's branches are tried in). An untouched edge is the same object. While another node still carries the old id (a duplicate, itself a Problems error), the edges are that node's too and stay, which is the removal's rule on the same draft.boundaryConfig.attachedToNodeId:boundaryRefsAfterNodeRename. It is the one key on the spec'sFlowNodeSchema, besides an edge endpoint, that names another node by id ("Host node ID this boundary event monitors"). A boundary event left attached to nothing would be named by no Problems row. Top-level nodes only, since a nested node cannot be renamed here.onSelectionChange, which both hosts (the Studio Automations pillar andResourceEditPage) pass assetSelection. SoFlowCanvas.tsxandFlowPreview.tsxare not touched.DecisionConditionSchemais strict and its guidance says atargetis never stored on the branch.What a rename does not carry, and why
NODEID.decision,NODEID.KEYin CEL text) are not rewritten. Measured after this fix with a throwaway probe through the real inspector andbuildFlowProblems: a downstream decision readingx.decision == 'approve'keeps it afterx → renamed, and the Problems panel shows only a warning:ok: `x` is not a reference in scope at this step.Rewriting CEL text is not a mechanical edit, so it is outside this card. It is reported to the seat as a finding.Pins
inspectors/FlowNodeInspector.renameNode-11827.test.tsx, through the real component under a host that merges shallowly and keeps a selection:s → x → epin: one patch, both edges follow, nothing dangles;r,re,renwrite nothing until blur;edges;previews/FlowPreview.renameNode-11827.test.tsx, through the real canvas and inspector side by side:node_1, Add connected node does not mintnode_1.previews/flow-problems.renameNode-11827.test.ts: the three pure helpers, including region depth (a parallel branch inside a loop body) and a mid-edit draft with holes.Ablation
The fix is committed first. Then the field's
onRename={rename}is put back toonRename={(v) => patchNode({ id: v })}, the old write alone, throughablation-replace.mjsin WRAP mode (it restores on EXIT, INT and TERM against an absolute path):634f4e3to96e6d3c;634f4e3equals HEAD, andgit diff HEADis empty. Re-checked independently withgit hash-objectagainstgit rev-parse HEAD:PATH.The pins import the component from source (relative specifiers), so no
dist/sits on the ablation's resolution path.Gates
Every reading is at
f8e702b, the head this PR opens on, unless marked otherwise. objectui#11779 landed during this work;main(16cda0b) was merged in, with no rebase. The one conflict was mechanical: both imports at the head offlow-problems.ts, and the edit-writers section header.pnpm --workspace-concurrency=2 --filter '@object-ui/app-shell^...' build, 29 packages, exit 0.714010b, before the merge:pnpm exec vitest run packages/app-shell/gave 1072 files passed, 1 skipped (1073); 10534 tests passed, 9 skipped; exit 0.f8e702b:pnpm --filter @object-ui/app-shell type-check(the script echoedtsc --noEmit && tsc -p tsconfig.test.json): exit 0.vitest runof 97 files: this PR's 3 pin files, every flow designer test underinspectors/andpreviews/, the Problems panel tests, every test inmain's delta, the Studio flow and automation tests, and the designer-table i18n tests. Result: 97 passed; 1481 tests passed, 1 skipped; exit 0.main's delta is app-shell only and passed its own CI, and CI runs the full farm on this merge.pnpm exec eslint --no-inline-configon the 6 changed source and test files: exit 0, 0 errors. The 4 warnings are all on lines this PR does not touch.check:*, exit 0 each:control-bytes,test-path-roots,changeset-claims,pending-changeset-literals;i18n-keys,i18n-drift,i18n-dead-keys(none of the three new rows is a candidate),i18n-designer-parity;vi-mock-specifiers,vi-mock-inherit,vi-mock-override-shape;new-line-citations(0 added by this branch);changeset-presence,changeset-no-major,changeset-overwrite,changeset-fixed;spec-symbols,designer-field-key-parity,unreferenced-sources,phantom-deps,self-import,handler-key-reads,metadata-write-doors,lint-coverage,type-check-coverage,shell-escape-residue.check:eager-closure(PREREQUISITE NOT MET; see Acceptance notes).check:readme-exports, since no export changed.Acceptance notes
check:eager-closureis NOT MEASURED locally (PREREQUISITE NOT MET: it reads a built console bundle). No new module enters any closure.flow-problems.tsalready reached@objectstack/spec/automationat runtime throughflow-expr-problems.tsandscreen-spec.ts(predicateSlotRefusal), andFlowNodeInspector.tsxalready reached@object-ui/componentsthrough_shared.tsx. Only the tree-shaken bytes ofcollectFlowGraphscan move. Left to CI.InspectorTextField's commit-on-change behaviour is not changed (⛔ by the claim). The ID field's draft is local to this panel.check:i18n-designer-parityis green.Session:
https://claude.ai/code/session_01CGZy1BGCjdN5cXqL9cnvB8Generated by Claude Code