fix(core): add macOS compatibility for provider loading and URL handling#382
Open
nofcfy-fanqi wants to merge 1 commit intoarchitects-toolkit:263-compatibility-with-macfrom
Open
Conversation
5 tasks
Member
|
Hi @nofcfy-fanqi , I just updated the title and description to not mention Linux as a potential compatible environment since Rhino is not available for Linux. This will avoid confusion. I'll test your changes soon, but they look good! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fix(core): add macOS compatibility for provider loading and URL handling
Description
SmartHopper currently does not work on macOS due to three platform-specific issues that cause crashes or incorrect behavior. This PR addresses all three issues to enable cross-platform compatibility.
Issue 1:
VerifySignaturecrashes on macOSProviderManager.VerifySignature()callsX509Certificate.CreateFromSignedFile(), which is a Windows-only API. On macOS, this throwsPlatformNotSupportedException, preventing any AI provider from loading.Fix: Wrap the Authenticode verification block in a
RuntimeInformation.IsOSPlatform(OSPlatform.Windows)guard. Strong-name verification (which is cross-platform) remains active on all platforms.Issue 2:
BuildFullUrlproducesfile://URLs on macOSAIProvider<T>.BuildFullUrl()usesUri.TryCreate(endpoint, UriKind.Absolute, out var abs)to detect whetherendpointis already an absolute URL. On Windows, relative paths like/chat/completionsreturnfalsefromTryCreatewithUriKind.Absolute. However, on macOS, the same call returnstrueand produces afile:///chat/completionsURI, causing the base URL concatenation logic to be skipped entirely. This results in HTTP requests being sent tofile://URLs, which fail silently.Fix: Add an additional scheme check:
abs.Scheme == Uri.UriSchemeHttp || abs.Scheme == Uri.UriSchemeHttps. This ensures only actual HTTP/HTTPS URLs are treated as absolute, and relative paths like/chat/completionsare correctly appended to the provider's base URL.Issue 3:
ComponentStateManagerdeadlock on macOSProcessTransitionQueue()fires state transition events (StateExited,StateEntered,TransitionCompleted) while holdingstateLock. On macOS, Grasshopper's threading model can cause event handlers to re-enter methods that also acquirestateLock, leading to a deadlock. This manifests as the UI freezing when components attempt state transitions.Fix: Refactor the transition logic to collect events inside the lock but fire them outside:
ExecuteTransitioninto a newExecuteTransitionCorethat returns transition info without firing events.Monitor.Exit, fire all collected events, then re-acquire viaMonitor.Enter.FireTransitionEventshelper method for clarity.Issue 4 (NOT FIXED):
WebChatDialogcrashes when opening Chat UI on macOSWebChatDialog.LoadInitialHtmlIntoWebView()callsthis._webView.LoadHtml(html, new Uri("https://smarthopper.local/")). On macOS, the Eto.FormsWKWebViewHandler.LoadHtml()implementation callsWKWebView.LoadFileUrl(baseNSUrl, baseNSUrl)for any non-nullbaseUri, butWKWebView.LoadFileUrl()only acceptsfile://URLs. Passinghttps://smarthopper.local/triggers anNSInvalidArgumentException: https://smarthopper.local/ is not a file URL, crashing Rhino.This affects the CanvasButton chat window (the icon in the top-right corner of the Grasshopper canvas). The crash occurs when the WebView is first initialized.
Root cause: Eto.Forms macOS
WKWebViewHandler.LoadHtml()(line 323-330 in the Eto source):Possible fix: In
WebChatDialog.csline 970, passnullasbaseUrion non-Windows platforms (or pass afile://temp directory URI). This would causeLoadHtmlStringto useabout:blankas the origin, which should be sufficient since JS-to-C# communication usesWKUserContentController.AddScriptMessageHandler()(not origin-dependent). However, this fix has not been tested yet and is NOT included in this PR.Testing Done
BuildFullUrlfix correctly constructs URLs likehttps://api.deepseek.com/chat/completionsinstead offile:///chat/completions.WKWebView.LoadFileUrl()bug when passed anhttps://base URI. This is documented as Issue 4 above but is NOT fixed in this PR.Checklist