Fix Modal SDK minimum and simplify sandbox creation - #2586
Merged
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Contributor
ApprovabilityVerdict: Not approved Macroscope's review found this PR not approvable — This is a narrow Modal SDK compatibility cleanup, and restricted sandboxes retain the existing allowlist values. Because sandbox egress-policy provisioning is security-sensitive and the change removes a capability guard while altering unrestricted arguments, the network-boundary behavior warrants human validation. You can add or adjust custom eligibility rules. Learn more. |
snimu
approved these changes
Sep 11, 2026
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 the Modal SDK minimum and simplify sandbox creation. This is a targeted cleanup of the existing networking implementation.
Require Modal 1.5.4, the version already in the lockfile, so the dependency requirement covers the policy-update API used by the runtime. Remove the capability check and pass the network options directly to sandbox creation.
Note
Medium Risk
Touches Modal sandbox provisioning and egress policy wiring; mis-specified allowlists could affect network-restricted rollouts, though behavior is intended to match the prior dict-based create path.
Overview
Raises the
verifiers[modal]extra tomodal>=1.5.4so the declared minimum matches the SDK that supports outbound network policy at sandbox creation and the execution-time policy update used inprepare_execution.In
ModalRuntime, drops thestart()guard that failed if_experimental_set_outbound_network_policywas missing. Sandbox creation now passesoutbound_domain_allowlistandoutbound_cidr_allowlistas first-classSandbox.createarguments (Nonewhen not network-restricted) instead of building and unpacking a separatenetworkdict.Reviewed by Cursor Bugbot for commit b5629ec. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Raise
modalSDK minimum to 1.5.4 and simplifyModalRuntimesandbox creationmodaloptional-dependency minimum from 1.4.0 to 1.5.4 in pyproject.toml.ModalRuntime.startthat raisedSandboxErrorwhen_experimental_set_outbound_network_policywas missing.outbound_domain_allowlistandoutbound_cidr_allowlistdirectly tomodal.Sandbox.create.aioin verifiers/v1/runtimes/modal.py.Nonefor both allowlist parameters instead of omitting them.Macroscope summarized b5629ec.