Skip to content

fix(macos): accept mixed-case bundle ids in open - #3867

Merged
thymikee merged 2 commits into
callstack:mainfrom
tlq5l:fix/macos-mixed-case-bundle-id
Oct 22, 2026
Merged

thymikee merged 2 commits into
callstack:mainfrom
tlq5l:fix/macos-mixed-case-bundle-id

Conversation

@tlq5l

@tlq5l tlq5l commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Summary

MACOS_BUNDLE_ID_PATTERN in packages/platform-apple/src/os/macos/apps.ts only matched lowercase, so a canonical id such as com.apple.TextEdit was treated as a display name. resolveMacOsApp then searched app names, found none and threw APP_NOT_INSTALLED before native dispatch. A dotted id with a lowercase first segment and mixed-case later segments (such as com.apple.TextEdit) now passes through unchanged and the claim is keyed by it. Display-name opens and the typed missing-app error are unchanged.

isMacOsBundleId (in kernel/device) is one permissive shared check ([A-Za-z0-9_-]+ segments) used by apps.ts and macos-app-lease.ts, so parseMacOsAppLeaseKey accepts exactly what it did before; the lowercase-first-segment rule, which tells a display name like Booking.com from a bundle id, lives only in resolveMacOsApp. helper.ts is untouched and keeps its own identical pattern, since importing the shared one grows the macos-facade eager closure.

I did not touch rollbackNewSessionClaim or abandonDeviceClaim. Fixes #3860.

Validation

  • Full unit suite (1803 files, 15644 tests), tsc -b tsconfig.typecheck.json examples/sdk/tsconfig.json and oxlint . --deny-warnings: passed on top of current main.
  • New tests: resolver table (com.apple.TextEdit, com.x.My_App kept as typed, Booking.com resolved by name), isMacOsBundleId table, a mixed-case lease key, the helper accepting Foo.Bar.
  • Ran live at 0e536c7: AGENT_DEVICE_MACOS_APP_BACKEND=native agent-device open com.apple.TextEdit --platform macos --session one --json (isolated state and claims dirs) -> success, appBundleId: com.apple.TextEdit, TextEdit running, one claim file; close TextEdit --session one -> TextEdit quit, claims dir empty.

@thymikee

Copy link
Copy Markdown
Member

The PR is ready at 8f9ca58. The code looks correct to me, only one check has reported so far and none fail, and there are no conflicts. Not blocking, and you can take or leave these: the widened /i shape test in resolveMacOsApp (apps.ts:20) now also matches dotted mixed-case display names such as "Booking.com", so they return as literal ids instead of resolving by name, as dotted lowercase names like zoom.us already do, which a dotted mixed-case display-name test would cover or you could confirm that route is acceptable; and the macOS bundle-id shape now lives in three copies (apps.ts, helper.ts:86, macos-app-lease.ts:23), where apps.ts still rejects an underscore the other two accept, so one shared isMacOsBundleId would keep them aligned. I read the code and did not run the tests or the live native open/close, so I could not confirm the claim file is app-keyed. Nothing blocks merge.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Oct 21, 2026
@tlq5l

tlq5l commented Oct 21, 2026

Copy link
Copy Markdown
Contributor Author

Done in e2e4ad7: one shared isMacOsBundleId in contracts is now used by apps.ts, helper.ts and macos-app-lease.ts, and it accepts underscores everywhere. It requires a lowercase first segment, so dotted display names like "Booking.com" resolve by name again, with a test. The side effect is that helper and lease keys with an uppercase first segment are now rejected.

@thymikee

Copy link
Copy Markdown
Member

The earlier review at 8f9ca58 found no problems, and e2e4ad7 now accepts mixed-case bundle ids in open, such as com.apple.TextEdit. I read the diff only, and I did not run the tests or the live macOS open at e2e4ad7, so the live check on this head is still missing. Please re-run open with a mixed-case id such as com.apple.TextEdit on macOS and show that the app launches. CI is green (1 check, none failing), there are no conflicts, and nothing else stops the merge once that output is posted. Not blocking, and you can take or leave these: the lowercase-first-segment rule in session-surface.ts only exists to tell a display name such as Booking.com from a bundle id in the open resolver, but the shared validator now also feeds assertMacOsBundleId in helper.ts and parseMacOsAppLeaseKey in macos-app-lease.ts, which only validate. Those two used to accept [A-Za-z0-9_-]+ as the first segment, so an id like Foo.Bar is now refused in close or quit by display name, and a macos-app lease with that deviceKey is refused. I could not confirm that real installed apps use an uppercase or underscore first segment, so the impact is unknown. Would it be simpler to keep the shared predicate permissive and move the lowercase-first-segment check into resolveMacOsApp, so helper and lease behavior stay unchanged? If the narrowing is intended, please say so in the PR. Also, no test covers isMacOsBundleId directly, the lease parser with a mixed-case id, or the helper assertion, so a small table test next to session-surface (accept com.apple.TextEdit and com.x.My_App, reject Booking.com) plus one mixed-case parseMacOsAppLeaseKey case would pin the behavior.

@thymikee thymikee removed the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Oct 21, 2026
@thymikee

Copy link
Copy Markdown
Member

This branch now conflicts with main at e2e4ad7. Please rebase it onto main. The code verdict from the earlier review does not change. The live open check with a mixed-case bundle id is still needed after the rebase.

tlq5l added 2 commits October 22, 2026 05:44
…ay-name rule in open

isMacOsBundleId (kernel/device) accepts [A-Za-z0-9_-]+ segments, the shape
macos-app-lease.ts already used, so parseMacOsAppLeaseKey behaves as before.
helper.ts keeps its own identical pattern: importing the shared one would add
modules to the macos-facade eager closure (eager-closure-budgets).

Only resolveMacOsApp also requires a lowercase first segment, so a dotted
display name like Booking.com resolves by name instead of being passed on
as a bundle id.

Agent: claude
@tlq5l
tlq5l force-pushed the fix/macos-mixed-case-bundle-id branch from e2e4ad7 to 0e536c7 Compare October 21, 2026 22:56
@tlq5l

tlq5l commented Oct 21, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto main and moved the lowercase-first rule into resolveMacOsApp; the shared predicate is the permissive pattern helper and lease used on main.

isMacOsBundleId is [A-Za-z0-9_-]+ segments again, so parseMacOsAppLeaseKey accepts exactly what it did before (Foo.Bar works as a lease key). Only resolveMacOsApp also requires a lowercase first segment, so Booking.com still resolves by name. Two things differ from the earlier head: the predicate now lives in kernel/device rather than session-surface, and helper.ts keeps its own identical pattern (unchanged from main). Importing the shared predicate from contracts or kernel into helper.ts adds modules to the macos-facade eager closure, and eager-closure-budgets fails on it. Say if you would rather have helper.ts use the shared one and raise that budget.

The PR is now two commits on main (c8b944c); new head is 0e536c7. Full unit suite (1803 files, 15644 tests), typecheck and lint pass.

Tests: an isMacOsBundleId table, a resolveMacOsApp table (Booking.com resolves by name), mixed-case lease keys, and helper quitMacOsApp('Foo.Bar').

Live check on 0e536c7 (macOS, native backend, isolated state and claims dirs; JSON output trimmed):

$ pgrep -x TextEdit
(no output)
$ AGENT_DEVICE_MACOS_APP_BACKEND=native node bin/agent-device.mjs open com.apple.TextEdit --platform macos --session one --json
{ "success": true, "data": { "session": "one", "appName": "com.apple.TextEdit", "appBundleId": "com.apple.TextEdit",
  "platform": "macos", "target": "desktop", "id": "host-macos-local:app:com.apple.textedit",
  "message": "Opened: com.apple.TextEdit", ... } }
$ pgrep -x TextEdit
25340
$ ls $AGENT_DEVICE_CLAIMS_DIR
255e6e0d36afb0c479a9e71966c9cd99f62b1a647b964a9207096913ec2710e8.json
$ node bin/agent-device.mjs close TextEdit --platform macos --session one --json
{ "success": true, "data": { "session": "one", "message": "Closed: one" } }
$ pgrep -x TextEdit
(no output)
$ ls $AGENT_DEVICE_CLAIMS_DIR
(empty)

@thymikee

Copy link
Copy Markdown
Member

The PR is ready at 0e536c7. The conflict from the earlier review (e2e4ad7) is gone, and I know of no conflicts now. The one check on this head is green. I read the diff from the base to this commit, and it touches the shared bundle id check, apps.ts and the macOS app lease parsing, all covered by the added unit tests. I did not re-run the live macOS TextEdit open and close. That run comes from the PR body, so the maintainer should be happy with it before merge. Cubic and Copilot have not reviewed this head yet.

Not blocking, take it or leave it: helper.ts still keeps its own MACOS_BUNDLE_ID_PATTERN (https://github.com/callstack/agent-device/blob/0e536c7/packages/platform-apple/src/os/macos/helper.ts#L90), identical to the kernel isMacOsBundleId, so a later change to the shared grammar could make assertMacOsBundleId accept different ids than open and lease parsing. This copy was already on main. Could assertMacOsBundleId call isMacOsBundleId from @agent-device/kernel/device? If the closure budget rejects that, a test that pins the helper pattern to the kernel one would do. I did not run the budget test, so I do not know whether it allows the import.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Oct 22, 2026
@thymikee
thymikee merged commit 4966739 into callstack:main Oct 22, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-human Valid work that needs human implementation, judgment, or maintainer merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

macOS open rejects canonical mixed-case bundle IDs and leaves an abandoned claim

2 participants