Skip to content

Use encrypted Modal sandbox tunnels#237

Open
abhinav-bellapu wants to merge 1 commit into
allenai:mainfrom
abhinav-bellapu:fix-modal-tunnel-deprecation
Open

Use encrypted Modal sandbox tunnels#237
abhinav-bellapu wants to merge 1 commit into
allenai:mainfrom
abhinav-bellapu:fix-modal-tunnel-deprecation

Conversation

@abhinav-bellapu

Copy link
Copy Markdown

Description

Fixes the Modal sandbox deprecation warning described in #141 by exposing the SWE-ReX runtime port through encrypted_ports instead of unencrypted_ports.

SWE-ReX currently hard-codes unencrypted_ports=[self._port] when creating its Modal sandbox. That means olmo-eval cannot fix the warning by passing modal_sandbox_kwargs: providing encrypted_ports there would still leave the deprecated unencrypted runtime port in place, and providing either port key can conflict with the hard-coded argument.

This change keeps the existing SWE-ReX Modal startup flow, but wraps the deployment class that olmo-eval constructs so the runtime port is requested as an encrypted tunnel. User-provided encrypted_ports are preserved, the runtime port is appended if needed, and deprecated unencrypted_ports entries are dropped from forwarded sandbox kwargs. Docker and local sandbox modes are unchanged.

Reviewer notes:

  • The wrapper is scoped to olmo-eval's Modal deployment creation path and can be removed once the pinned SWE-ReX dependency exposes encrypted sandbox ports directly.
  • The regression test uses fake modal and SWE-ReX modules, so it verifies the port kwargs without requiring Modal credentials or network access.

Closes #141

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactoring (no functional changes)

Testing

  • Unit tests pass locally (pytest tests/ --ignore=tests/integration/)
  • Integration tests pass (if applicable)
  • New tests added for new functionality

Local checks run:

  • uv run --no-group vllm pytest tests/core/harness/test_sandbox_executor.py
  • uv run --no-group vllm pytest tests/core/harness
  • uv run --no-group vllm ruff check src/ tests/
  • uv run --no-group vllm ruff format --check src/ tests/
  • uv run --no-group vllm ty check src/ alembic/ (exits 0; reports existing unused type: ignore warnings in optional SWE-ReX paths)
  • uv run --no-group vllm pytest tests/ --ignore=tests/integration/ (1711 passed, 6 skipped)

Checklist

  • My code follows the project's style guidelines
  • I have performed a self-review of my code
  • I have added/updated documentation as needed
  • My changes generate no new warnings
  • Any dependent changes have been merged and published

@abhinav-bellapu abhinav-bellapu marked this pull request as ready for review July 1, 2026 04:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix Deprecated Modal tunnel implementation in swe-rex

1 participant