Fix focus nudge launching Electron instead of restoring VS Code on macOS#783
Merged
dsarno merged 2 commits intoCoplayDev:betafrom Feb 19, 2026
Merged
Conversation
The focus restore used process name (e.g. "Electron") which caused macOS to launch a standalone Electron instance instead of returning to VS Code. Now captures bundle identifier and uses `tell application id` for precise activation, falling back to name-based activation if bundle ID fails. Fixes CoplayDev#699 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
Reviewer's GuideRefactors focus tracking to carry structured frontmost app info (including macOS bundle IDs) through the nudge pipeline and updates macOS focus-restore to prefer bundle-id-based activation, fixing the Electron/VS Code mis-focus while keeping Windows/Linux behavior intact. Sequence diagram for macOS Unity focus nudge with bundle_id-based restoresequenceDiagram
actor User
participant UnityEditor as UnityEditor
participant FocusNudge as nudge_unity_focus
participant FrontApp as _get_frontmost_app
participant MacFront as _get_frontmost_app_macos
participant FocusApp as _focus_app
participant FocusMac as _focus_app_macos
participant AppleScript as osascript
User->>UnityEditor: Perform action requiring focus nudge
UnityEditor->>FocusNudge: nudge_unity_focus(unity_project_path)
FocusNudge->>FrontApp: _get_frontmost_app()
FrontApp->>MacFront: _get_frontmost_app_macos()
MacFront->>AppleScript: osascript System Events query
AppleScript-->>MacFront: procName|bundleID
MacFront-->>FrontApp: _FrontmostAppInfo(name, bundle_id)
FrontApp-->>FocusNudge: original_app
FocusNudge->>FocusApp: _focus_app("Unity", unity_project_path)
FocusApp->>FocusMac: _focus_app_macos(app_name="Unity", unity_project_path, bundle_id=None)
FocusMac->>AppleScript: Unity PID based activation
AppleScript-->>FocusMac: success
FocusMac-->>FocusApp: True
FocusApp-->>FocusNudge: True
FocusNudge->>FrontApp: _get_frontmost_app()
FrontApp-->>FocusNudge: current_app _FrontmostAppInfo
FocusNudge-->>FocusNudge: wait focus_duration_s
FocusNudge->>FocusApp: _focus_app(original_app)
FocusApp-->>FocusNudge: platform route by system
FocusApp->>FocusMac: _focus_app_macos(app_name=original_app.name, unity_project_path=None, bundle_id=original_app.bundle_id)
alt bundle_id present
FocusMac->>AppleScript: tell application id bundle_id to activate
AppleScript-->>FocusMac: success
FocusMac-->>FocusApp: True
else bundle_id missing or failed
FocusMac->>AppleScript: tell application original_app.name to activate
AppleScript-->>FocusMac: success or failure
FocusMac-->>FocusApp: result
end
FocusApp-->>FocusNudge: True or False
FocusNudge-->>UnityEditor: nudge complete
Class diagram for focus_nudge frontmost app handling and macOS focus restoreclassDiagram
class _FrontmostAppInfo {
+str name
+str bundle_id
+__str__() str
}
class FocusNudgeModule {
+_parse_env_float(env_var, default) float
+_is_available() bool
+reset_nudge_backoff() void
+_get_frontmost_app_macos() _FrontmostAppInfo
+_get_frontmost_app_windows() _FrontmostAppInfo
+_get_frontmost_app_linux() _FrontmostAppInfo
+_get_frontmost_app() _FrontmostAppInfo
+_focus_app_macos(app_name, unity_project_path, bundle_id) bool
+_focus_app_windows(window_title) bool
+_focus_app_linux(window_id) bool
+_focus_app(app_info, unity_project_path) bool
+_find_unity_pid_by_project_path(project_path) int
+_focus_any_unity_macos() bool
+nudge_unity_focus(unity_project_path, focus_duration_s) bool
}
_FrontmostAppInfo <.. FocusNudgeModule : used by
FocusNudgeModule ..> _FrontmostAppInfo : returns
FocusNudgeModule ..> _FrontmostAppInfo : wraps string into
class MacOSPath {
+_get_frontmost_app_macos() _FrontmostAppInfo
+_focus_app_macos(app_name, unity_project_path, bundle_id) bool
}
class WindowsPath {
+_get_frontmost_app_windows() _FrontmostAppInfo
+_focus_app_windows(window_title) bool
}
class LinuxPath {
+_get_frontmost_app_linux() _FrontmostAppInfo
+_focus_app_linux(window_id) bool
}
FocusNudgeModule ..> MacOSPath : delegates on Darwin
FocusNudgeModule ..> WindowsPath : delegates on Windows
FocusNudgeModule ..> LinuxPath : delegates on Linux
MacOSPath ..> _FrontmostAppInfo : constructs
WindowsPath ..> _FrontmostAppInfo : constructs
LinuxPath ..> _FrontmostAppInfo : constructs
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
📝 WalkthroughWalkthroughIntroduces Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- When parsing the
procName|bundleIDAppleScript output in_get_frontmost_app_macos, consider using a delimiter that cannot appear in app names (or escaping) to avoid edge cases where a process name might contain|and break the split logic. - In
_focus_app_macos, when bundle ID activation fails, it could be helpful to logresult.stdout/result.stderrat debug level along with the bundle ID to make diagnosing future activation failures easier.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- When parsing the `procName|bundleID` AppleScript output in `_get_frontmost_app_macos`, consider using a delimiter that cannot appear in app names (or escaping) to avoid edge cases where a process name might contain `|` and break the split logic.
- In `_focus_app_macos`, when bundle ID activation fails, it could be helpful to log `result.stdout`/`result.stderr` at debug level along with the bundle ID to make diagnosing future activation failures easier.
## Individual Comments
### Comment 1
<location> `Server/src/utils/focus_nudge.py:157-162` </location>
<code_context>
)
if result.returncode == 0:
- return result.stdout.strip()
+ output = result.stdout.strip()
+ parts = output.split("|", 1)
+ name = parts[0]
+ bundle_id = parts[1] if len(parts) > 1 and parts[1] else None
</code_context>
<issue_to_address>
**suggestion:** Handle 'missing value' bundle identifier explicitly to avoid using it as a real bundle_id
For some macOS processes, the front process bundle identifier can be the literal string `"missing value"`. In that case `parts[1]` is truthy, so `bundle_id` becomes `"missing value"` and you end up calling `tell application id "missing value"`, which will always fail. Consider normalizing this case (e.g., mapping `"missing value"` or similar placeholder values to `None`) so the code skips the bundle-id path and uses the name-based fallback instead.
```suggestion
if result.returncode == 0:
output = result.stdout.strip()
parts = output.split("|", 1)
name = parts[0]
bundle_id: str | None = None
if len(parts) > 1:
raw_bundle_id = parts[1].strip()
# Some processes report the literal string "missing value" as a bundle ID;
# treat this as an absent bundle identifier so we fall back to name-based handling.
if raw_bundle_id and raw_bundle_id.lower() != "missing value":
bundle_id = raw_bundle_id
return _FrontmostAppInfo(name=name, bundle_id=bundle_id)
```
</issue_to_address>
### Comment 2
<location> `Server/src/utils/focus_nudge.py:303-305` </location>
<code_context>
+ # For non-Unity apps, prefer bundle_id to avoid the Electron bug:
+ # VS Code's process name is "Electron", and `tell application "Electron"`
+ # can launch a standalone Electron instance instead of returning to VS Code.
+ if bundle_id:
+ escaped_bundle_id = bundle_id.replace('"', '\\"')
+ result = subprocess.run(
+ ["osascript", "-e", f'tell application id "{escaped_bundle_id}" to activate'],
+ capture_output=True,
+ text=True,
+ timeout=5,
+ )
+ if result.returncode == 0:
+ return True
+ logger.debug(f"Bundle ID activation failed for {bundle_id}, falling back to name")
</code_context>
<issue_to_address>
**suggestion:** Log AppleScript stderr when bundle_id activation fails to aid debugging
Since we already capture `stderr`, consider logging `result.stderr` at debug level when bundle-id activation fails. That would preserve current behavior while greatly improving diagnosability of AppleScript/permission issues.
```suggestion
if result.returncode == 0:
return True
logger.debug(f"Bundle ID activation failed for {bundle_id}, falling back to name")
if result.stderr:
logger.debug(
"AppleScript stderr for bundle ID %s activation: %s",
bundle_id,
result.stderr.strip(),
)
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
Server/src/utils/focus_nudge.py (1)
592-596: Redundant guard —original_appis always non-None and never"Unity"at this point.
original_appis guaranteed non-None (early return at lines 557–559).original_app.name == "Unity"is also impossible here because line 562 already returnsFalsewhen"Unity" in original_app.name. The entire condition is dead code.♻️ Proposed cleanup
- if original_app and original_app.name != "Unity": + if original_app.name != "Unity": # always True; kept for explicitness if _focus_app(original_app):Or simply remove the check entirely if desired:
- if original_app and original_app.name != "Unity": - if _focus_app(original_app): - logger.info(f"Returned focus to {original_app} after {focus_duration_s:.1f}s Unity focus") - else: - logger.warning(f"Failed to return focus to {original_app}") + if _focus_app(original_app): + logger.info(f"Returned focus to {original_app} after {focus_duration_s:.1f}s Unity focus") + else: + logger.warning(f"Failed to return focus to {original_app}")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Server/src/utils/focus_nudge.py` around lines 592 - 596, The condition checking "if original_app and original_app.name != 'Unity'" is redundant because original_app is already guaranteed non-None and not Unity; remove that guard and directly call _focus_app(original_app) and log using logger.info/logger.warning with focus_duration_s and original_app as before (i.e., replace the entire outer if with a direct call to _focus_app(original_app) and the subsequent logging branches referencing focus_duration_s, _focus_app, and logger).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Server/src/utils/focus_nudge.py`:
- Around line 146-162: The AppleScript can return `missing value` for `bundle
identifier` which causes osascript to error before Python sees output; update
the AppleScript string in _get_frontmost_app_macos so it guards/coerces bundle
identifier to text (e.g., check `if bundle identifier of frontProc is missing
value then set bundleID to "" else set bundleID to bundle identifier of
frontProc` or use a try/coerce) so osascript always returns a string
"procName|bundleID"; keep the Python parsing that converts empty bundleID to
None and return _FrontmostAppInfo(name=name, bundle_id=bundle_id).
- Around line 295-298: The AppleScript string escaping in focus_nudge.py is
incorrect and Ruff S603/S607 are false positives; change the Python-style escape
(\" ) to AppleScript escaping by replacing any internal double-quote with two
double-quotes ("" ) when building escaped_bundle_id and escaped_app_name, and
add a local linter suppression (e.g., # noqa: S603 S607) on the subprocess.run
calls that invoke osascript since they pass an argv list (not shell=True) and
bundle_id/app_name come from the OS; update the code paths that set
escaped_bundle_id and escaped_app_name and annotate the subprocess.run lines
that call osascript to suppress the warning.
---
Nitpick comments:
In `@Server/src/utils/focus_nudge.py`:
- Around line 592-596: The condition checking "if original_app and
original_app.name != 'Unity'" is redundant because original_app is already
guaranteed non-None and not Unity; remove that guard and directly call
_focus_app(original_app) and log using logger.info/logger.warning with
focus_duration_s and original_app as before (i.e., replace the entire outer if
with a direct call to _focus_app(original_app) and the subsequent logging
branches referencing focus_duration_s, _focus_app, and logger).
…g, improve logging - Guard AppleScript against `missing value` bundle identifier that would crash osascript and silently disable the entire nudge cycle (CodeRabbit) - Also filter "missing value" string on the Python side as belt-and-suspenders (Sourcery) - Fix AppleScript string escaping: use "" (AppleScript convention) not \" (Python convention) for both bundle_id and app_name (CodeRabbit) - Log AppleScript stderr when bundle_id activation fails (Sourcery) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
Server/src/utils/focus_nudge.py (2)
279-285: Pre-existing:bundle identifiernot guarded againstmissing valuein the Unity PID activation path — inconsistent with the defensive fix introduced elsewhere in this PR.Line 280:
set bundleID to bundle identifier of targetProcIf
bundle identifierreturnsmissing value(unlikely for Unity, but possible for a stripped or unsigned build),tell application id bundleID to activateon line 284 raises an AppleScript runtime error,subprocess.runreturns non-zero, and_focus_app_macosreturnsFalse— silently failing to nudge Unity.The same pattern was fixed in
_get_frontmost_app_macos(lines 150–153). Applying the same guard here for consistency:♻️ Proposed fix — guard bundleID against `missing value`
tell application "System Events" set targetProc to first process whose unix id is {pid} set frontmost of targetProc to true -- Get bundle identifier to activate the app properly - set bundleID to bundle identifier of targetProc + set bundleID to "" + try + set bID to bundle identifier of targetProc + if bID is not missing value then set bundleID to bID + end try end tell --- Activate using bundle identifier (ensures Unity wakes up and starts processing) -tell application id bundleID to activate +-- Activate: prefer bundle ID, fall back to name +if bundleID is not "" then + tell application id bundleID to activate +else + tell application "Unity" to activate +end if🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Server/src/utils/focus_nudge.py` around lines 279 - 285, In _focus_app_macos, guard the AppleScript result for bundleID (set bundleID to bundle identifier of targetProc) against returning 'missing value' just like _get_frontmost_app_macos does: after retrieving bundleID, check if it's missing/None/empty and handle by skipping the "tell application id bundleID to activate" step (and return False or fall back) so we don't call AppleScript with a missing bundle identifier; reference the variables bundleID and targetProc and the function _focus_app_macos when making this change.
605-609: Redundantif original_appguard.At this point
original_appis guaranteed non-None— the early return at lines 570–572 exits the function if it isNone. Theif original_appcondition is alwaysTruehere.🧹 Proposed cleanup
- if original_app and original_app.name != "Unity": + if original_app.name != "Unity":🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Server/src/utils/focus_nudge.py` around lines 605 - 609, The redundant guard "if original_app" is unnecessary because original_app is guaranteed non-None earlier; remove that condition and directly check the app name and call _focus_app. Update the block to: if original_app.name != "Unity": if _focus_app(original_app): logger.info(...) else: logger.warning(...), referencing original_app, _focus_app, and logger to locate and change the code.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@Server/src/utils/focus_nudge.py`:
- Around line 144-156: Add Ruff suppressions for the false-positive
shell-injection warnings on the osascript subprocess calls: annotate the
subprocess invocations that pass ["osascript", ...] (the call that builds and
runs the AppleScript returning procName & "|" & bundleID and the other osascript
calls referenced around the later occurrences) with "# noqa: S603,S607". Keep
the list-style invocation (shell=False) and do not change logic; the noqa should
be placed on the lines with the subprocess call expressions so Ruff ignores
S603/S607 for those specific osascript usages (bundleID/bundle_id is sourced
from System Events and _is_available() already checks osascript on PATH).
---
Nitpick comments:
In `@Server/src/utils/focus_nudge.py`:
- Around line 279-285: In _focus_app_macos, guard the AppleScript result for
bundleID (set bundleID to bundle identifier of targetProc) against returning
'missing value' just like _get_frontmost_app_macos does: after retrieving
bundleID, check if it's missing/None/empty and handle by skipping the "tell
application id bundleID to activate" step (and return False or fall back) so we
don't call AppleScript with a missing bundle identifier; reference the variables
bundleID and targetProc and the function _focus_app_macos when making this
change.
- Around line 605-609: The redundant guard "if original_app" is unnecessary
because original_app is guaranteed non-None earlier; remove that condition and
directly check the app name and call _focus_app. Update the block to: if
original_app.name != "Unity": if _focus_app(original_app): logger.info(...)
else: logger.warning(...), referencing original_app, _focus_app, and logger to
locate and change the code.
msanatan
pushed a commit
to msanatan/unity-mcp
that referenced
this pull request
Feb 25, 2026
…cOS (CoplayDev#783) * Fix focus nudge launching Electron instead of restoring VS Code on macOS The focus restore used process name (e.g. "Electron") which caused macOS to launch a standalone Electron instance instead of returning to VS Code. Now captures bundle identifier and uses `tell application id` for precise activation, falling back to name-based activation if bundle ID fails. Fixes CoplayDev#699 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Address review feedback: guard AppleScript missing value, fix escaping, improve logging - Guard AppleScript against `missing value` bundle identifier that would crash osascript and silently disable the entire nudge cycle (CodeRabbit) - Also filter "missing value" string on the Python side as belt-and-suspenders (Sourcery) - Fix AppleScript string escaping: use "" (AppleScript convention) not \" (Python convention) for both bundle_id and app_name (CodeRabbit) - Log AppleScript stderr when bundle_id activation fails (Sourcery) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
msanatan
pushed a commit
to msanatan/unity-mcp
that referenced
this pull request
Feb 25, 2026
…cOS (CoplayDev#783) * Fix focus nudge launching Electron instead of restoring VS Code on macOS The focus restore used process name (e.g. "Electron") which caused macOS to launch a standalone Electron instance instead of returning to VS Code. Now captures bundle identifier and uses `tell application id` for precise activation, falling back to name-based activation if bundle ID fails. Fixes CoplayDev#699 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Address review feedback: guard AppleScript missing value, fix escaping, improve logging - Guard AppleScript against `missing value` bundle identifier that would crash osascript and silently disable the entire nudge cycle (CodeRabbit) - Also filter "missing value" string on the Python side as belt-and-suspenders (Sourcery) - Fix AppleScript string escaping: use "" (AppleScript convention) not \" (Python convention) for both bundle_id and app_name (CodeRabbit) - Log AppleScript stderr when bundle_id activation fails (Sourcery) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
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.
Summary
tell application "Electron"launches a standalone Electron instance instead of returning to VS Code ([Bug] Get Test Job launching Electron #699)com.microsoft.VSCode) alongside process name when saving the frontmost apptell application id "<bundle_id>" to activatefor precise focus restore, falling back to name-based activation if bundle ID failsTest plan
tell application id "com.apple.Terminal" to activate)"Electron"+ correct bundle_id → bundle_id takes priorityFixes #699
🤖 Generated with Claude Code
Summary by Sourcery
Improve cross-platform focus restoration by tracking richer frontmost app metadata and preferring precise activation on macOS.
New Features:
Bug Fixes:
Enhancements:
Summary by CodeRabbit