fix: remove async lock polling in Duplicate and Arpeggio flows (Fix #6882)

[gcharpe1604]

Description

This PR removes the asynchronous lock-polling mechanism from the DuplicateBlock and ArpeggioBlock execution paths.

The affected connection-store critical sections are fully synchronous. Polling with setTimeout() and awaiting lock acquisition introduced unnecessary event-loop boundaries while the lock remained held, and the fallback eventually force-acquired the same boolean lock.

The listeners and main flow now use the existing synchronous critical-section pattern, with try/finally ensuring that logo.connectionStoreLock is always released.

Related Issue

Fixes #6882.

PR Category

• Bug Fix — Fixes a bug or incorrect behavior
• Feature — Adds new functionality
• Performance — Improves performance (load time, memory, rendering, etc.)
• Tests — Adds or updates test coverage
• Documentation — Updates to docs, comments, or README

Changes Made

• Removed __acquireLock() polling from DuplicateBlock.
• Removed __acquireLock() polling from ArpeggioBlock.
• Converted both end listeners from asynchronous to synchronous callbacks.
• Removed timeout retries, forced acquisition, and warning guards.
• Preserved the existing multi-turtle connection copy and restoration behaviour.
• Kept lock release inside finally blocks.
• Added focused tests for synchronous execution and lock cleanup after success and errors.

Safety Validation

The affected critical sections were checked for asynchronous yield points.

The following operations are synchronous:

• setTurtleListener
• findBottomBlock
• __lookForOtherTurtles
• queue construction
• connection-array mutation and restoration

There are no promises, awaited calls, timers, asynchronous functions, or callback registration inside the critical sections.

This safety assumption should be revisited if asynchronous work is added to these paths in the future.

Testing Performed

• FlowBlocks
• IntervalsBlocks
• RhythmBlocks
• ExtrasBlocks

Result: 187 tests passed

Additional checks:

• Scoped ESLint passed
• Prettier passed
• git diff --check passed
• Codecov reports all modified and coverable lines covered

Checklist

• Tested locally
• Added focused tests
• Followed project coding style
• Ran lint and formatting checks
• Addressed previous reviewer feedback
• Enabled maintainer edits

[github-actions]

✅ All Jest tests passed! This PR is ready to merge.

Coverage: Statements: 44.81% | Branches: 36.32% | Functions: 49.85% | Lines: 45.25%

[Ashutoshx7]

i stronly think your reasoning is right given javascript is single threaded so that settimeout pollin was nevera cutally doing anything usefull
small fixs the warn+ force- acquire in_listner is a but confusing since the lock should not ever be held there in normal flow them main flow path below it just does logo.connectionstrorelock=true
i'll suggest match that style here and drop the warn

[github-actions]

❌ Some Jest tests failed. Please check the logs and fix the issues before merging.

Coverage: Statements: 45.15% | Branches: 36.5% | Functions: 50.5% | Lines: 45.58%

Failed Tests:

planetInterface.test.js

[github-actions]

❌ Some Jest tests failed. Please check the logs and fix the issues before merging.

Coverage: Statements: 45.14% | Branches: 36.48% | Functions: 50.5% | Lines: 45.58%

Failed Tests:

planetInterface.test.js

[github-actions]

✅ All Jest tests passed! This PR is ready to merge.

Coverage: Statements: 45.14% | Branches: 36.48% | Functions: 50.5% | Lines: 45.58%

[gcharpe1604]

@Ashutoshx7 Thanks for the review!
Addressed your suggestion in 4290bcb removed the if (connectionStoreLock) warn guard from the _listener and replaced it with the same direct assignment pattern used in the main flow below.

Also pushed 97c8b64 to fix the planetInterface.test.js CI failure, unrelated to the core change, but stage.update(event) on line 223 of planetInterface.js was passing undefined as an argument (stray event reference with no DOM context). Cleaned that up so all 29 tests pass.

All checks are green now. Ready for final review.

[github-actions]

This PR has merge conflicts with master.

Please rebase your branch:

# Add upstream remote (one-time setup)
git remote add upstream https://github.com/sugarlabs/musicblocks.git

# Fetch latest master and rebase
git fetch upstream
git rebase upstream/master

# Resolve any conflicts, then:
git push --force-with-lease origin YOUR_BRANCH

Tip: Enable "Allow edits from maintainers" on this PR so we can auto-rebase for you next time. This only grants access to your PR branch. Your fork's other branches are not affected.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 59.27%. Comparing base (a76b8bb) to head (c8f0a9e).

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #6883      +/-   ##
==========================================
- Coverage   59.28%   59.27%   -0.01%     
==========================================
  Files         176      176              
  Lines       57967    57935      -32     
==========================================
- Hits        34365    34341      -24     
+ Misses      23602    23594       -8     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[gcharpe1604]

Rebased this PR onto the latest master and added focused tests for the synchronous lock cleanup.

Current status:

• merge conflicts resolved
• only the intended Duplicate/Arpeggio production files and their tests are changed
• 187 relevant tests passed
• lint, Prettier, and git diff --check passed
• Codecov reports all modified and coverable lines covered

The earlier reviewer suggestion to remove the warning/forced-acquisition guard and use the synchronous direct-assignment pattern remains preserved.

Ready for final review.