Skip to content

fix(editor): block rename applies to correct block when selection changes - #3129

Merged
waleedlatif1 merged 1 commit into
stagingfrom
fix/renaming
Feb 4, 2026
Merged

fix(editor): block rename applies to correct block when selection changes#3129
waleedlatif1 merged 1 commit into
stagingfrom
fix/renaming

Conversation

@waleedlatif1

Copy link
Copy Markdown
Collaborator

Summary

  • Fixed block rename applying to wrong block when user clicks another block mid-rename
  • Fixed cancelled executions showing as "running" in terminal logs
  • Refactored flag+effect anti-pattern to callback registration pattern
  • Added duration calculation for cancelled entries

Type of Change

  • Bug fix

Testing

Tested manually

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel

vercelBot commented Feb 4, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
ProjectDeploymentActionsUpdated (UTC)
docsSkippedSkippedFeb 4, 2026 3:58am

Request Review

@greptile-apps

Copy link
Copy Markdown
Contributor

Greptile Overview

Greptile Summary

This PR fixes a critical bug where block rename would apply to the wrong block if the user changed selection mid-rename, and ensures cancelled executions display correct status and duration in terminal logs.

Key Changes:

  • Refactored block rename from flag+effect anti-pattern to callback registration pattern, eliminating stale closure issues
  • Added renamingBlockIdRef to capture the block ID when rename starts, ensuring rename applies to the originally selected block even if selection changes
  • Enhanced execution cancellation to properly finalize block logs with endedAt timestamps and calculated durations
  • Updated terminal console to calculate and display accurate durations for cancelled entries
  • Added safeguard in formatDuration() to handle NaN/Infinity values by displaying em-dash

Architecture:
The rename flow now uses a more robust pattern: workflow.tsx calls triggerRename() → store invokes registered callback → editor.tsx reads fresh state from stores and captures block ID in ref → rename applies to captured ID regardless of subsequent selection changes.

Confidence Score: 5/5

  • This PR is safe to merge with only a minor style improvement recommended
  • The implementation is well-designed and solves both stated bugs correctly. The callback registration pattern is cleaner than the previous flag+effect approach, and the ref-based block ID capture elegantly handles race conditions. Execution cancellation changes are comprehensive and properly propagated through the stack. Only minor style issue: missing devtools middleware.
  • No files require special attention - all changes follow good patterns and solve the stated problems effectively

Important Files Changed

FilenameOverview
apps/sim/stores/panel/editor/store.tsRefactored from flag-based pattern to callback registration pattern for rename triggering
apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/editor.tsxAdded ref-based block ID capture to ensure rename applies to correct block when selection changes mid-rename
apps/sim/executor/execution/engine.tsAdded finalizeIncompleteLogs() to properly set endedAt and duration for blocks when execution is cancelled
apps/sim/stores/terminal/console/store.tsEnhanced cancelRunningEntries() to calculate and set duration for cancelled entries

Sequence Diagram

sequenceDiagram
participant User
participant Workflow
participant EditorStore
participant Editor
participant WorkflowStore
participant CollabAPI
Note over User,CollabAPI: Block Rename Flow (Fixed Race Condition)
User->>Workflow: Right-click → Rename
Workflow->>EditorStore: setCurrentBlockId(blockA)
Workflow->>EditorStore: triggerRename()
EditorStore->>Editor: invoke registered callback
Editor->>EditorStore: getState().currentBlockId
Editor->>WorkflowStore: getState().blocks[blockA]
Note over Editor: Capture blockA in renamingBlockIdRef
Editor->>Editor: setIsRenaming(true)
Editor->>Editor: setEditedName(blockA.name)
Note over User,Editor: User clicks different block mid-rename
User->>Workflow: Click blockB
Workflow->>EditorStore: setCurrentBlockId(blockB)
Note over Editor: renamingBlockIdRef still holds blockA
User->>Editor: Complete rename
Editor->>Editor: Read renamingBlockIdRef (blockA)
Editor->>WorkflowStore: getState().blocks[blockA]
Editor->>CollabAPI: updateBlockName(blockA, newName)
Note over Editor: ✅ Correct block renamed!
Note over User,CollabAPI: Execution Cancellation Flow
User->>Workflow: Cancel execution
Workflow->>Engine: cancel()
Engine->>Engine: finalizeIncompleteLogs()
Note over Engine: Set endedAt, calculate durationMs
Engine->>Hook: onExecutionCancelled()
Hook->>TerminalStore: cancelRunningEntries(workflowId)
Note over TerminalStore: Calculate duration, set isCanceled
TerminalStore->>UI: Update entries
UI->>Formatting: formatDuration(durationMs)
Note over Formatting: Return actual duration or "—" for NaN
Loading

@greptile-appsgreptile-appsBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

4 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

@greptile-apps

Copy link
Copy Markdown
Contributor
Additional Comments (1)

apps/sim/stores/panel/editor/store.ts
Missing devtools middleware wrapping the persist middleware

According to the Zustand store patterns guide, stores should use the devtools middleware. Other persisted stores in the codebase follow the pattern: devtools(persist(...), { name: 'store-name' })

Context Used: Context from dashboard - Zustand store patterns (source)

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/sim/stores/panel/editor/store.ts
Line: 37:38
Comment:
Missing `devtools` middleware wrapping the `persist` middleware
According to the Zustand store patterns guide, stores should use the `devtools` middleware. Other persisted stores in the codebase follow the pattern: `devtools(persist(...), { name: 'store-name' })`**Context Used:** Context from `dashboard` - Zustand store patterns ([source](https://app.greptile.com/review/custom-context?memory=b1b216e7-c04e-4351-9ea0-47c57790a11a))
<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>
How can I resolve this? If you propose a fix, please make it concise.

@waleedlatif1
waleedlatif1 merged commit 7977ac8 into stagingFeb 4, 2026
11 checks passed
@waleedlatif1
waleedlatif1 deleted the fix/renaming branch February 4, 2026 04:06
royceP2 pushed a commit to arenadeveloper02/p2-sim that referenced this pull request Mar 3, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@waleedlatif1