Repository navigation
fix(macos): accept mixed-case bundle ids in open - #3867
Conversation
|
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 |
|
Done in e2e4ad7: one shared |
|
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 |
|
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 |
Agent: claude
…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
e2e4ad7 to
0e536c7
Compare
|
Rebased onto main and moved the lowercase-first rule into resolveMacOsApp; the shared predicate is the permissive pattern helper and lease used on main.
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 Live check on 0e536c7 (macOS, native backend, isolated state and claims dirs; JSON output trimmed): |
|
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, Not blocking, take it or leave it: |
Summary
MACOS_BUNDLE_ID_PATTERNinpackages/platform-apple/src/os/macos/apps.tsonly matched lowercase, so a canonical id such ascom.apple.TextEditwas treated as a display name.resolveMacOsAppthen searched app names, found none and threwAPP_NOT_INSTALLEDbefore native dispatch. A dotted id with a lowercase first segment and mixed-case later segments (such ascom.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(inkernel/device) is one permissive shared check ([A-Za-z0-9_-]+segments) used byapps.tsandmacos-app-lease.ts, soparseMacOsAppLeaseKeyaccepts exactly what it did before; the lowercase-first-segment rule, which tells a display name likeBooking.comfrom a bundle id, lives only inresolveMacOsApp.helper.tsis untouched and keeps its own identical pattern, since importing the shared one grows themacos-facadeeager closure.I did not touch
rollbackNewSessionClaimorabandonDeviceClaim. Fixes #3860.Validation
tsc -b tsconfig.typecheck.json examples/sdk/tsconfig.jsonandoxlint . --deny-warnings: passed on top of current main.com.apple.TextEdit,com.x.My_Appkept as typed,Booking.comresolved by name),isMacOsBundleIdtable, a mixed-case lease key, the helper acceptingFoo.Bar.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.