-
-
Notifications
You must be signed in to change notification settings - Fork 313
refactor(ios): make unscoped simctl argv a type error at the Apple tool port #2898
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 |
|---|---|---|
|
|
@@ -5,15 +5,16 @@ import { | |
| type ExecOptions, | ||
| type ExecResult, | ||
| } from '@agent-device/host-kit/command'; | ||
| import type { ScopedSimctlArgs } from '@agent-device/contracts/platform-runtime-host'; | ||
| import { createScopedProvider } from '@agent-device/kernel/scoped-provider'; | ||
| import { createLocalAppleMacOsHostProvider } from '../os/macos/host-provider.ts'; | ||
| import type { | ||
| AppleMacOsHelperProvider, | ||
| AppleMacOsHostProvider, | ||
| ApplePlistProvider, | ||
| AppleSimctlToolProvider, | ||
| AppleToolAvailabilityChecker, | ||
| AppleToolCommandExecutor, | ||
| AppleToolSubcommandExecutor, | ||
| AppleXcrunToolProvider, | ||
| } from './tool-provider-types.ts'; | ||
|
|
||
|
|
@@ -29,7 +30,7 @@ export type { | |
|
|
||
| export type AppleToolProvider = { | ||
| runCommand: AppleToolCommandExecutor; | ||
| simctl: AppleXcrunToolProvider; | ||
| simctl: AppleSimctlToolProvider; | ||
| devicectl: AppleXcrunToolProvider; | ||
| macosHelper?: AppleMacOsHelperProvider; | ||
| macosHost?: AppleMacOsHostProvider; | ||
|
|
@@ -118,7 +119,7 @@ export async function runXcrun(args: string[], options?: ExecOptions): Promise<E | |
| const provider = resolveAppleToolProvider(); | ||
| const [tool, ...toolArgs] = args; | ||
| if (tool === 'simctl') { | ||
| return await provider.simctl.run(toolArgs, options); | ||
| return await provider.simctl.run(toolArgs as unknown as ScopedSimctlArgs, options); | ||
|
Member
Author
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. This |
||
| } | ||
| if (tool === 'devicectl') { | ||
| return await provider.devicectl.run(toolArgs, options); | ||
|
|
@@ -149,7 +150,9 @@ function coerceRunCommand(run: AppleToolCommandExecutor): AppleToolCommandExecut | |
| return async (cmd, args, options) => coerceExecResult(await run(cmd, args, options)); | ||
| } | ||
|
|
||
| function coerceRun(run: AppleToolSubcommandExecutor): AppleToolSubcommandExecutor { | ||
| function coerceRun<Args>( | ||
| run: (args: Args, options?: ExecOptions) => Promise<ExecResult>, | ||
| ): (args: Args, options?: ExecOptions) => Promise<ExecResult> { | ||
| return async (args, options) => coerceExecResult(await run(args, options)); | ||
| } | ||
|
|
||
|
|
||
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.
scopeSimctlArgs*returnsScopedSimctlArgs(branded), but the adjacentbuildSimctlArgsForAddress/buildSimctlArgsForDevicereturn plainstring[]— an unexplained asymmetry between functions whose names differ by one word. The consequence is that the brand lands on the minorityhost.appleTools.runpath, while the dominantbuild*+runSimctlForDevice/runXcrunpath — and the cross-package façade@agent-device/platform-apple/simctl— hand out unbrandedstring[]. So R79, not the compiler, is what actually enforces the invariant on most call sites. If you brand thebuild*return (a brandedreadonly string[]is still assignable to thereadonly string[]/string[]sinks those ~20 call sites use, so they compile untouched), the guarantee spans both execution paths and theArrayExpression+'--set'clauses of R79 become deletable — its own kill criterion says exactly this. Even if you keep the current scope deliberately, this naming asymmetry should at least be explained in a comment.