feat(cua-driver): remove LaunchAgent auto-updater; add version hint and launch_app improvements - #1388
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe pull request refactors the cua-driver update mechanism to fetch version information from GitHub instead of relying on a local auto-update script, and extends the app launcher to support isolated browser sessions through configurable command-line arguments, environment variables, and instance creation flags. Changes
Sequence Diagram(s)sequenceDiagram
actor User as User
participant CLI as UpdateCommand
participant VC as VersionCheck
participant GitHub as GitHub API
participant Script as install.sh
User->>CLI: Run with/without --apply
CLI->>CLI: Output current version
CLI->>VC: fetchLatest()
VC->>GitHub: GET releases (cua-driver-v*)
GitHub-->>VC: Latest release tag
VC-->>CLI: Version string
CLI->>VC: isNewer(latest, current)
VC-->>CLI: Boolean result
alt Already up to date
CLI-->>User: "Already up to date"
else Update available
alt --apply flag provided
CLI->>Script: bash install.sh (execute)
Script-->>CLI: Exit code
CLI-->>User: Install complete or error
else Manual update guidance
CLI-->>User: Show manual curl | bash command
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (6)
libs/cua-driver/Sources/CuaDriverCLI/CuaDriverCommand.swift (3)
345-352: Diagnostics go to stdout instead of stderr.
"Could not reach GitHub …"and"Installation failed …"(line 375) are user-facing errors butcua-driver updateinto another tool can't distinguish success from failure by stream alone. Consider routing the error lines throughFileHandle.standardError.write(...)like the rest of the CLI does.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/cua-driver/Sources/CuaDriverCLI/CuaDriverCommand.swift` around lines 345 - 352, The guard that uses VersionCheck.fetchLatest() prints errors to stdout with print(...) — change those error messages (the "Could not reach GitHub — check your connection and try again." and the later "Installation failed …") to write to stderr instead by sending their UTF-8 Data to FileHandle.standardError (e.g. replace print(...) with FileHandle.standardError.write((message + "\n").data(using: .utf8)!)) in the CuaDriverCommand flow where VersionCheck.fetchLatest() and the installation failure path are handled so callers can distinguish errors via stderr.
372-377:Process.run()not error-handled; orphanedautoUpdateEnabledconfig field.Two small follow-ups on the install path:
try proc.run()will throw straight out of the function if/bin/bashis somehow unavailable or the working-directory state rejects spawning. The user sees a rawNSErrorrather than a friendly hint. Wrapping indo { try proc.run() } catch { … throw ExitCode(1) }mirrors the diagnostic style used inServeCommand.relaunchViaOpen.
CuaDriverConfig.autoUpdateEnabledandConfigStore.setAutoUpdateEnabledSync(referenced in the relevant snippets) are still wired intoConfigCommand, butUpdateCommandno longer reads them. A user togglingcua-driver config updates disablewill silently have no effect on this passive-update model. Either remove the field + subcommand or repurpose it to silenceVersionCheck.warnIfOutdated()on serve startup so the toggle actually means something.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/cua-driver/Sources/CuaDriverCLI/CuaDriverCommand.swift` around lines 372 - 377, Wrap the call to proc.run() in a do/catch and handle errors similarly to ServeCommand.relaunchViaOpen: attempt try proc.run() inside do, catch the error, print a friendly diagnostic message and throw ExitCode(1) so users see a clear hint instead of an unhandled NSError; additionally, reconcile the orphaned auto-update config by either removing CuaDriverConfig.autoUpdateEnabled and ConfigStore.setAutoUpdateEnabledSync (and the related ConfigCommand subcommand) or by repurposing them so UpdateCommand/serve startup consults the flag—specifically wire the flag into VersionCheck.warnIfOutdated() (or guard that call during serve startup) so toggling via ConfigCommand actually suppresses warnings as intended.
376-376: RedundantInt32cast onterminationStatus.
Process.terminationStatusis alreadyInt32, soInt32(proc.terminationStatus)is a no-op.- throw ExitCode(Int32(proc.terminationStatus)) + throw ExitCode(proc.terminationStatus)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/cua-driver/Sources/CuaDriverCLI/CuaDriverCommand.swift` at line 376, The throw line uses a redundant Int32 cast — replace the unnecessary Int32(proc.terminationStatus) with proc.terminationStatus directly when constructing ExitCode (i.e., change the throw in CuaDriverCommand where proc is used to pass proc.terminationStatus into ExitCode), removing the no-op cast to simplify the code.libs/cua-driver/Sources/CuaDriverCLI/VersionCheck.swift (2)
11-32: Lookup is fragile if 40 non-driver releases land first.
fetchLatestwalks the first page (per_page=40) and returns the first non-draft / non-prerelease release whose tag starts withcua-driver-v. If thetrycua/cuamonorepo ever ships ≥40 newer releases for sibling components (cua-cli-v*,cua-server-v*, …) before the next cua-driver release, the loop falls off the end andfetchLatest()silently returnsnil, suppressing the upgrade hint until the next driver release rolls onto page 1.Consider either filtering the URL by tag prefix on the server (e.g. via the
git/refs/tags/cua-driver-vAPI) or following pagination until a match is found. Low-priority for now since cua-driver is currently the dominant release line, but worth noting.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/cua-driver/Sources/CuaDriverCLI/VersionCheck.swift` around lines 11 - 32, fetchLatest currently only checks a single page (per_page=40) which can miss the newest cua-driver tag if ≥40 non-driver tags precede it; update fetchLatest to either (a) query the GitHub tags ref API for the driver prefix (e.g. use the git/refs/tags endpoint filtered by tagPrefix) or (b) implement pagination: add a page parameter to the request URL, loop requests using the Link header (or increment page until no results) and inspect each page’s releases for the first non-draft/non-prerelease tag starting with tagPrefix; update references to repo, tagPrefix and the URLRequest/URLSession logic in fetchLatest to continue paging until a match is found or no more pages remain.
46-57: Detached background task may be torn down before it can warn.
warnIfOutdated()firesTask.detachedand returns immediately. For short-lived CLI invocations (e.g.cua-driver list_appsif it ever calls this) the parent process can exit before the 4-second HTTP fetch resolves, leaving the user with no warning. Today onlyservecalls this and the daemon outlives the request, so it's fine — but if you ever wire it into other entry points, you'll want either to await the task or to drop the warning entirely on short-lived paths.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/cua-driver/Sources/CuaDriverCLI/VersionCheck.swift` around lines 46 - 57, warnIfOutdated currently spawns a Task.detached that may be torn down if the process exits; change warnIfOutdated to return a Task<Void, Never> created with Task { ... } (not Task.detached) so callers can await it on short‑lived CLI paths, or modify callers to await the returned Task; update references to warnIfOutdated, fetchLatest, isNewer, and CuaDriverCore.version so the new Task handle is awaited where appropriate (e.g., short-lived commands) while long‑running commands like serve can ignore the handle.libs/cua-driver/Sources/CuaDriverServer/Tools/LaunchAppTool.swift (1)
148-149: Consider validating port ranges forelectron_debugging_portandwebkit_inspector_port.
intValueaccepts any integer — negative values, 0, or values above 65535 will be passed through verbatim into--remote-debugging-port=<N>orWEBKIT_INSPECTOR_SERVER=127.0.0.1:<N>. Chrome will silently fail to start CDP on an invalid port, leaving the caller to discover the misconfiguration via tool failure later. A quick(1...65535).contains(port)check at the boundary would surface a clear error.Also applies to: 207-219
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/cua-driver/Sources/CuaDriverServer/Tools/LaunchAppTool.swift` around lines 148 - 149, Validate parsed port integers for electronDebuggingPort and webkitInspectorPort by ensuring any non-nil port falls within the valid TCP port range 1...65535 before using them; if a port is out of range, return or throw a clear, descriptive error rather than passing the invalid value into "--remote-debugging-port" or "WEBKIT_INSPECTOR_SERVER". Apply the same boundary check to the other port-parsing sites in this file where similar variables are created/used (the other electron/webkit port handling blocks) so all uses of electronDebuggingPort and webkitInspectorPort (and their siblings) perform the (1...65535).contains(port) check and produce a user-visible error when invalid.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@libs/cua-driver/Sources/CuaDriverCLI/CuaDriverCommand.swift`:
- Around line 337-338: Add a hidden deprecated alias for the removed --silent
flag and fold it into the existing apply logic: introduce a new `@Flag` annotated
variable (e.g., `@Flag`(name: .long, help: .hidden) var silent = false) alongside
the existing var apply, then at the top of run() compute a single boolean (e.g.,
let shouldApply = apply || silent) and use shouldApply everywhere instead of
apply; reference the existing apply var and the run() method when making the
change.
In `@libs/cua-driver/Sources/CuaDriverCLI/VersionCheck.swift`:
- Around line 35-42: The isNewer(_ candidate:than:) function currently treats
versions with extra trailing zero components as newer (e.g., "1.2.0" > "1.2");
update isNewer to normalize both versions before comparing by either stripping
trailing zeros from the arrays returned by parts(_:) or by extending the shorter
array with zeros so that semantic equivalents compare equal. Locate isNewer and
its use of parts(candidate)/parts(current) and adjust normalization (remove
trailing zero entries or pad with zeros) so zipped comparisons correctly treat
"1.2" and "1.2.0" as equal and only return true for truly larger versions.
In `@libs/cua-driver/Sources/CuaDriverCore/Apps/AppLauncher.swift`:
- Around line 67-69: The code assigns additionalEnvironment directly to
config.environment which replaces the inherited process environment and drops
critical vars; change the logic in AppLauncher (the block that checks
additionalEnvironment) to merge additionalEnvironment into
ProcessInfo.processInfo.environment (e.g., start from
ProcessInfo.processInfo.environment, overlay additionalEnvironment) and then
assign that merged dictionary to config.environment so inherited vars like
PATH/HOME/LANG are preserved.
In `@libs/cua-driver/Sources/CuaDriverServer/Tools/LaunchAppTool.swift`:
- Line 206: The code silently drops malformed entries from rawExtraArgs when
building additionalArguments (var additionalArguments: [String] =
rawExtraArgs.compactMap { $0.stringValue }), causing inconsistent validation vs.
rawUrls and potential security footguns; change this to validate each
rawExtraArgs element the same way rawUrls does: iterate rawExtraArgs, require
each has a non-empty stringValue, and on any invalid/missing entry return the
same errorResult used for rawUrls (include context about additional_arguments)
instead of silently compact-mapping; update the function that processes
additionalArguments to use this explicit validation and the same error handling
path as rawUrls.
---
Nitpick comments:
In `@libs/cua-driver/Sources/CuaDriverCLI/CuaDriverCommand.swift`:
- Around line 345-352: The guard that uses VersionCheck.fetchLatest() prints
errors to stdout with print(...) — change those error messages (the "Could not
reach GitHub — check your connection and try again." and the later "Installation
failed …") to write to stderr instead by sending their UTF-8 Data to
FileHandle.standardError (e.g. replace print(...) with
FileHandle.standardError.write((message + "\n").data(using: .utf8)!)) in the
CuaDriverCommand flow where VersionCheck.fetchLatest() and the installation
failure path are handled so callers can distinguish errors via stderr.
- Around line 372-377: Wrap the call to proc.run() in a do/catch and handle
errors similarly to ServeCommand.relaunchViaOpen: attempt try proc.run() inside
do, catch the error, print a friendly diagnostic message and throw ExitCode(1)
so users see a clear hint instead of an unhandled NSError; additionally,
reconcile the orphaned auto-update config by either removing
CuaDriverConfig.autoUpdateEnabled and ConfigStore.setAutoUpdateEnabledSync (and
the related ConfigCommand subcommand) or by repurposing them so
UpdateCommand/serve startup consults the flag—specifically wire the flag into
VersionCheck.warnIfOutdated() (or guard that call during serve startup) so
toggling via ConfigCommand actually suppresses warnings as intended.
- Line 376: The throw line uses a redundant Int32 cast — replace the unnecessary
Int32(proc.terminationStatus) with proc.terminationStatus directly when
constructing ExitCode (i.e., change the throw in CuaDriverCommand where proc is
used to pass proc.terminationStatus into ExitCode), removing the no-op cast to
simplify the code.
In `@libs/cua-driver/Sources/CuaDriverCLI/VersionCheck.swift`:
- Around line 11-32: fetchLatest currently only checks a single page
(per_page=40) which can miss the newest cua-driver tag if ≥40 non-driver tags
precede it; update fetchLatest to either (a) query the GitHub tags ref API for
the driver prefix (e.g. use the git/refs/tags endpoint filtered by tagPrefix) or
(b) implement pagination: add a page parameter to the request URL, loop requests
using the Link header (or increment page until no results) and inspect each
page’s releases for the first non-draft/non-prerelease tag starting with
tagPrefix; update references to repo, tagPrefix and the URLRequest/URLSession
logic in fetchLatest to continue paging until a match is found or no more pages
remain.
- Around line 46-57: warnIfOutdated currently spawns a Task.detached that may be
torn down if the process exits; change warnIfOutdated to return a Task<Void,
Never> created with Task { ... } (not Task.detached) so callers can await it on
short‑lived CLI paths, or modify callers to await the returned Task; update
references to warnIfOutdated, fetchLatest, isNewer, and CuaDriverCore.version so
the new Task handle is awaited where appropriate (e.g., short-lived commands)
while long‑running commands like serve can ignore the handle.
In `@libs/cua-driver/Sources/CuaDriverServer/Tools/LaunchAppTool.swift`:
- Around line 148-149: Validate parsed port integers for electronDebuggingPort
and webkitInspectorPort by ensuring any non-nil port falls within the valid TCP
port range 1...65535 before using them; if a port is out of range, return or
throw a clear, descriptive error rather than passing the invalid value into
"--remote-debugging-port" or "WEBKIT_INSPECTOR_SERVER". Apply the same boundary
check to the other port-parsing sites in this file where similar variables are
created/used (the other electron/webkit port handling blocks) so all uses of
electronDebuggingPort and webkitInspectorPort (and their siblings) perform the
(1...65535).contains(port) check and produce a user-visible error when invalid.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: abdccfcd-c410-4b34-8936-bf20d3ac7d2d
📒 Files selected for processing (7)
libs/cua-driver/Sources/CuaDriverCLI/CuaDriverCommand.swiftlibs/cua-driver/Sources/CuaDriverCLI/ServeCommand.swiftlibs/cua-driver/Sources/CuaDriverCLI/VersionCheck.swiftlibs/cua-driver/Sources/CuaDriverCore/Apps/AppLauncher.swiftlibs/cua-driver/Sources/CuaDriverServer/Tools/LaunchAppTool.swiftlibs/cua-driver/scripts/install.shlibs/cua-driver/scripts/uninstall.sh
| @Flag(name: .long, help: "Download and apply the update without prompting.") | ||
| var apply = false |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm whether the prior --silent flag ever shipped in a tagged release.
git log --all --oneline -- libs/cua-driver/Sources/CuaDriverCLI/CuaDriverCommand.swift | head -20
git tag --list 'cua-driver-v*' | sort -V | tail -5Repository: trycua/cua
Length of output: 217
🏁 Script executed:
#!/bin/bash
# Check what the file looked like at v0.1 release
git show 9bb952c6:libs/cua-driver/Sources/CuaDriverCLI/CuaDriverCommand.swift 2>/dev/null | grep -A 2 -B 2 "silent\|apply" || echo "File not found at that commit or grep found nothing"
# List all tags more carefully
echo "=== All cua-driver tags ==="
git tag -l | grep -i "cua-driver"
# Check git log for when --silent was removed
echo "=== Commits mentioning silent ==="
git log --all --oneline --grep="silent" -- libs/cua-driver/Sources/CuaDriverCLI/CuaDriverCommand.swift 2>/dev/null || echo "No commits found with 'silent' in message"
# Check git log for when --apply was added
echo "=== Commits mentioning apply ==="
git log --all --oneline --grep="apply" -- libs/cua-driver/Sources/CuaDriverCLI/CuaDriverCommand.swift 2>/dev/null || echo "No commits found with 'apply' in message"
# Try to find when silent flag was removed from the file
echo "=== Checking for silent in file history ==="
git log -p --all -- libs/cua-driver/Sources/CuaDriverCLI/CuaDriverCommand.swift 2>/dev/null | grep -B 5 -A 5 "silent" | head -40 || echo "No silent flag found in history"Repository: trycua/cua
Length of output: 1269
🏁 Script executed:
#!/bin/bash
# Check the change in commit 25700779
echo "=== Full diff for commit 25700779 ==="
git show 25700779 -- libs/cua-driver/Sources/CuaDriverCLI/CuaDriverCommand.swift | grep -A 10 -B 10 "silent\|apply" | head -60
# Check current HEAD
echo ""
echo "=== Current HEAD for the file (lines 330-345) ==="
git show HEAD:libs/cua-driver/Sources/CuaDriverCLI/CuaDriverCommand.swift 2>/dev/null | sed -n '330,345p'
# Check if 25700779 is the current HEAD
echo ""
echo "=== Current HEAD commit ==="
git rev-parse HEADRepository: trycua/cua
Length of output: 3200
Breaking CLI change: --silent → --apply with no alias.
The --silent flag shipped in the v0.1 public release and is now removed entirely. Any user scripts or documentation calling cua-driver update --silent will start failing argument parsing. Keep --silent as a deprecated hidden alias for one release cycle:
`@Flag`(name: .long, help: "Download and apply the update without prompting.")
var apply = false
+
+ // Deprecated alias for one release; remove after 0.0.7.
+ `@Flag`(name: [.customLong("silent")], help: .hidden)
+ var silentDeprecated = falseThen OR the two together at the top of run().
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @Flag(name: .long, help: "Download and apply the update without prompting.") | |
| var apply = false | |
| `@Flag`(name: .long, help: "Download and apply the update without prompting.") | |
| var apply = false | |
| // Deprecated alias for one release; remove after 0.0.7. | |
| `@Flag`(name: [.customLong("silent")], help: .hidden) | |
| var silentDeprecated = false |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@libs/cua-driver/Sources/CuaDriverCLI/CuaDriverCommand.swift` around lines 337
- 338, Add a hidden deprecated alias for the removed --silent flag and fold it
into the existing apply logic: introduce a new `@Flag` annotated variable (e.g.,
`@Flag`(name: .long, help: .hidden) var silent = false) alongside the existing var
apply, then at the top of run() compute a single boolean (e.g., let shouldApply
= apply || silent) and use shouldApply everywhere instead of apply; reference
the existing apply var and the run() method when making the change.
| static func isNewer(_ candidate: String, than current: String) -> Bool { | ||
| let lhs = parts(candidate) | ||
| let rhs = parts(current) | ||
| for (a, b) in zip(lhs, rhs) { | ||
| if a != b { return a > b } | ||
| } | ||
| return lhs.count > rhs.count | ||
| } |
There was a problem hiding this comment.
isNewer returns true for equal versions written with trailing-zero padding.
When lhs and rhs agree on every zipped component but lhs has trailing extra parts, the function returns true. Concretely:
isNewer("1.2.0", than: "1.2")→lhs=[1,2,0],rhs=[1,2], zip is all equal, thenlhs.count > rhs.count→ returnstrue, even though the versions are semantically equivalent.
In practice the cua-driver tag format is fixed three-part X.Y.Z so this isn't currently exploitable, but if a future tag is normalized differently (e.g. 0.1 vs 0.1.0), startup warnIfOutdated() would noisily warn forever. Strip trailing zeros before comparing, or treat missing parts as 0 on both sides.
♻️ Possible fix
static func isNewer(_ candidate: String, than current: String) -> Bool {
let lhs = parts(candidate)
let rhs = parts(current)
- for (a, b) in zip(lhs, rhs) {
- if a != b { return a > b }
+ let count = max(lhs.count, rhs.count)
+ for i in 0..<count {
+ let a = i < lhs.count ? lhs[i] : 0
+ let b = i < rhs.count ? rhs[i] : 0
+ if a != b { return a > b }
}
- return lhs.count > rhs.count
+ return false
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| static func isNewer(_ candidate: String, than current: String) -> Bool { | |
| let lhs = parts(candidate) | |
| let rhs = parts(current) | |
| for (a, b) in zip(lhs, rhs) { | |
| if a != b { return a > b } | |
| } | |
| return lhs.count > rhs.count | |
| } | |
| static func isNewer(_ candidate: String, than current: String) -> Bool { | |
| let lhs = parts(candidate) | |
| let rhs = parts(current) | |
| let count = max(lhs.count, rhs.count) | |
| for i in 0..<count { | |
| let a = i < lhs.count ? lhs[i] : 0 | |
| let b = i < rhs.count ? rhs[i] : 0 | |
| if a != b { return a > b } | |
| } | |
| return false | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@libs/cua-driver/Sources/CuaDriverCLI/VersionCheck.swift` around lines 35 -
42, The isNewer(_ candidate:than:) function currently treats versions with extra
trailing zero components as newer (e.g., "1.2.0" > "1.2"); update isNewer to
normalize both versions before comparing by either stripping trailing zeros from
the arrays returned by parts(_:) or by extending the shorter array with zeros so
that semantic equivalents compare equal. Locate isNewer and its use of
parts(candidate)/parts(current) and adjust normalization (remove trailing zero
entries or pad with zeros) so zipped comparisons correctly treat "1.2" and
"1.2.0" as equal and only return true for truly larger versions.
| if !additionalEnvironment.isEmpty { | ||
| config.environment = additionalEnvironment | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
NSWorkspace.OpenConfiguration environment property replace or merge with inherited environment macOS
💡 Result:
The environment property of NSWorkspace.OpenConfiguration sets the environment variables for the newly launched app instance. Apple documentation describes it as "The set of environment variables to set in a new app instance." This indicates that the dictionary provided completely replaces the default environment (inherited from launchd/system defaults) with exactly the key-value pairs specified, rather than merging or augmenting the inherited environment. No official documentation explicitly states "replace" vs "merge," but the phrasing "to set" combined with standard macOS launch behaviors (where launchd provides a minimal/default environment, and tools like NSTask allow full replacement via dictionary) supports replacement semantics. Sandboxed callers have restrictions on modifying arguments and environment, but the property is available. Example usage: let config = NSWorkspace.OpenConfiguration config.environment = ["MY_VAR": "value", "PATH": "/custom/path:$PATH"] NSWorkspace.shared.openApplication(at: appURL, configuration: config) { app, error in ... } Note: To preserve standard vars like PATH, explicitly include them in the dictionary.
Citations:
- 1: https://developer.apple.com/documentation/appkit/nsworkspace/openconfiguration
- 2: https://developer.apple.com/documentation/appkit/nsworkspace/openconfiguration/environment
- 3: https://developer.apple.com/documentation/appkit/nsworkspace/launchconfigurationkey
- 4: https://stackoverflow.com/questions/62897936/nsworkspace-openconfiguration-ignores-arguments-despite-not-being-sandboxed
Environment replacement will strip critical inherited variables; merge with ProcessInfo.processInfo.environment before assigning.
Setting config.environment to a small dictionary like ["WEBKIT_INSPECTOR_SERVER": ..., "TAURI_WEBVIEW_AUTOMATION": "1"] completely replaces the inherited environment. This removes critical variables (PATH, HOME, LANG, etc.) that the launched app depends on, breaking Tauri/WKWebView launches when only webkit_inspector_port is set.
Merge additionalEnvironment onto ProcessInfo.processInfo.environment before assignment, or if intentional, document this behavior loudly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@libs/cua-driver/Sources/CuaDriverCore/Apps/AppLauncher.swift` around lines 67
- 69, The code assigns additionalEnvironment directly to config.environment
which replaces the inherited process environment and drops critical vars; change
the logic in AppLauncher (the block that checks additionalEnvironment) to merge
additionalEnvironment into ProcessInfo.processInfo.environment (e.g., start from
ProcessInfo.processInfo.environment, overlay additionalEnvironment) and then
assign that merged dictionary to config.environment so inherited vars like
PATH/HOME/LANG are preserved.
| ) | ||
| } | ||
|
|
||
| var additionalArguments: [String] = rawExtraArgs.compactMap { $0.stringValue } |
There was a problem hiding this comment.
Inconsistent input validation: silent drop of malformed additional_arguments entries.
rawUrls rejects non-string / empty entries with an errorResult (lines 163-171), but rawExtraArgs uses compactMap { $0.stringValue } and silently drops anything that isn't a string. A caller passing additional_arguments: ["--user-data-dir=...", 42] would get a partial argv with no warning, which is exactly the kind of "isolated session is suddenly using the user's real profile" footgun #1377 wants to avoid.
🛡️ Suggested validation parity with `urls`
- var additionalArguments: [String] = rawExtraArgs.compactMap { $0.stringValue }
+ var additionalArguments: [String] = []
+ for raw in rawExtraArgs {
+ guard let str = raw.stringValue else {
+ return errorResult("additional_arguments entries must be strings.")
+ }
+ additionalArguments.append(str)
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| var additionalArguments: [String] = rawExtraArgs.compactMap { $0.stringValue } | |
| var additionalArguments: [String] = [] | |
| for raw in rawExtraArgs { | |
| guard let str = raw.stringValue else { | |
| return errorResult("additional_arguments entries must be strings.") | |
| } | |
| additionalArguments.append(str) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@libs/cua-driver/Sources/CuaDriverServer/Tools/LaunchAppTool.swift` at line
206, The code silently drops malformed entries from rawExtraArgs when building
additionalArguments (var additionalArguments: [String] = rawExtraArgs.compactMap
{ $0.stringValue }), causing inconsistent validation vs. rawUrls and potential
security footguns; change this to validate each rawExtraArgs element the same
way rawUrls does: iterate rawExtraArgs, require each has a non-empty
stringValue, and on any invalid/missing entry return the same errorResult used
for rawUrls (include context about additional_arguments) instead of silently
compact-mapping; update the function that processes additionalArguments to use
this explicit validation and the same error handling path as rawUrls.
…nd launch_app improvements Auto-updater (LaunchAgent): - Remove com.trycua.cua_driver_updater LaunchAgent from install.sh — weekly silent binary replacement of a security-sensitive tool is disruptive and raises MDM/security concerns - uninstall.sh gracefully removes the LaunchAgent from existing installs - `cua-driver update` simplified: checks GitHub, prints instructions, applies with --apply flag by running install.sh - VersionCheck: lightweight GitHub releases check on server startup; prints one-liner to stderr if a newer version exists (fire-and-forget, 4s timeout, silent if offline) launch_app improvements (closes #1377): - `creates_new_application_instance`: force a new process even if the app is already running — enables isolated Chrome sessions per agent - `additional_arguments`: raw argv passthrough for any launch flag (e.g. --user-data-dir=/tmp/session-a for isolated Chrome profiles) - AppLauncher: wire both through NSWorkspace.OpenConfiguration Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ent in AppLauncher Setting config.environment to only additionalEnvironment replaced the entire inherited environment, stripping PATH, HOME, LANG and similar variables that launched apps expect. Merge additionalEnvironment on top of ProcessInfo.processInfo.environment instead. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1743075 to
d366d31
Compare
1 participant
Summary
Auto-updater removal
The weekly LaunchAgent (
com.trycua.cua_driver_updater) silently replaced a security-sensitive binary (accessibility + Apple Events entitlements) on a schedule. This is disruptive and raises concerns for users on managed machines.New model: passive version hint on startup + explicit
cua-driver update [--apply]command.cua-driver updateshows what's available;cua-driver update --applyrunsinstall.shuninstall.shcleans up the legacy LaunchAgent from existing installslaunch_appimprovements (closes #1377)Two new params enable isolated browser sessions for multi-agent testing:
{ "bundle_id": "com.google.Chrome", "urls": ["http://localhost:3000"], "creates_new_application_instance": true, "additional_arguments": [ "--user-data-dir=/tmp/cua-session-a", "--no-first-run", "--no-default-browser-check" ] }Each session gets a distinct pid, isolated profile (no shared cookies/localStorage/extensions), and can be controlled independently. The user's real Chrome profile is untouched.
Test plan
swift buildpassescua-driver updateshows version info without applyingcua-driver update --applytriggers install.shlaunch_appwithcreates_new_application_instance=true+additional_argumentslaunches isolated ChromeCloses #1377
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Changes
--applyflag for automatic installation; without it, users receive manual update guidance.