-
Notifications
You must be signed in to change notification settings - Fork 88
feat: thread app_bundle_id through open_app tool #7434
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -684,6 +684,11 @@ export class ComputerUseSession { | |||||||||||||||||||
| isError: true, | ||||||||||||||||||||
| }; | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| // Inject targetAppBundleId when the LLM didn't provide one | ||||||||||||||||||||
| if (!input.app_bundle_id && this.targetAppBundleId) { | ||||||||||||||||||||
| input = { ...input, app_bundle_id: this.targetAppBundleId }; | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
Comment on lines
+688
to
+691
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 Target app bundle ID injected for non-target app in cross-app workflows When a user's task explicitly requests a cross-app workflow (e.g. "Copy from Chrome and paste into Safari"), the guard at lines 670-686 is bypassed because Root Cause and ImpactConsider this scenario:
The fix should only inject the bundle ID when the requested app actually matches the target app (i.e.,
Suggested change
Was this helpful? React with 👍 or 👎 to provide feedback. |
||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| if (toolName === 'computer_use_run_applescript') { | ||||||||||||||||||||
|
|
||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -235,7 +235,7 @@ final class ActionExecutor: ActionExecuting { | |
| try drag(from: CGPoint(x: fromX, y: fromY), to: CGPoint(x: endX, y: endY)) | ||
| case .openApp: | ||
| guard let appName = action.appName else { throw ExecutorError.appNotFound("(no name)") } | ||
| try await openApp(name: appName) | ||
| try await openApp(name: appName, bundleId: action.appBundleId) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Passing Useful? React with 👍 / 👎. |
||
| case .runAppleScript: | ||
| guard let source = action.script else { throw ExecutorError.appleScriptMissingScript } | ||
| return try await runAppleScript(source) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This defaulting logic runs whenever
app_bundle_idis missing, even whentaskExplicitlyRequestsCrossApp()has already allowed switching to a differentapp_name. In cross-app tasks, that means a step like “open Slack” can be rewritten with the target app’s bundle ID, and becauseopenAppresolves bundle IDs before names, the executor activates the wrong app and the workflow gets stuck. Restrict this injection to cases where the requested app is still the target app.Useful? React with 👍 / 👎.