Skip to content

fix(sandbox): reject blank or whitespace Modal region identifiers - #121

Open
ayush-or wants to merge 1 commit into
mainfrom
devin/1790437575-modal-region-whitespace
Open

ayush-or wants to merge 1 commit into
mainfrom
devin/1790437575-modal-region-whitespace

Conversation

@ayush-or

Copy link
Copy Markdown
Contributor

TL;DR

ModalSandboxOptionsSchema now rejects blank or whitespace-containing modalRegions entries at config parse time instead of forwarding them to Modal.

What changed?

  • src/sandbox/modal-schema.ts: region names must match /^\S+$/ (replaces .min(1)), with the message region must not be blank or contain whitespace.
  • src/sandbox/modal.test.ts: the blank-region test now also covers " ", "\t", "us east" and " us".

Why?

Devin Review on openrouter-web#44757 flagged that a whitespace-only region passed .min(1) and reached Modal as an invalid region identifier, failing at sandbox creation rather than at config validation. The schema is harness-owned and shared by every Modal-backed benchmark (and by agent_dx in openrouter-web via ModalSandboxOptionsSchema.shape), so the fix lives here rather than per consumer. Rejecting is preferred over .trim() so malformed input is refused rather than repaired.

How to test

bun test src/sandbox

Expected: rejects blank region identifiers passes for all five inputs, ["us"] and [] still parse.

Reviewer focus

  • /^\S+$/ is intentionally the narrowest constraint that addresses the finding. Modal region ids are lowercase alphanumerics with hyphens today, but I did not lock that in.

Checklist

  • Tests cover changed behavior
  • Public API or configuration changes are backward compatible, or the break is documented
  • Benchmark changes document dataset provenance and licensing
  • No credentials, private results, or restricted dataset contents are included
  • Documentation is updated where needed

Link to Devin session: https://openrouter.devinenterprise.com/sessions/4a11ad51b948470fbe1b893fd1f275f8
Open in Devin Desktop: https://openrouter.devinenterprise.com/desktop/session/4a11ad51b948470fbe1b893fd1f275f8?variant=devin
Requested by: @ayush-or

A whitespace-only modalRegions entry passed the min(1) check and reached Modal as an invalid region, failing at sandbox creation. Region names now must match /^\S+$/ so blank and whitespace-containing values are rejected at config parse time instead of repaired.
@ayush-or
ayush-or requested a review from a team as a code owner September 26, 2026 15:47
@devin-ai-integration

Copy link
Copy Markdown
Contributor

I'll fix CI failures and address comments from users with write access that start with 'Devin'.

  • Disable automatic comment, CI, and merge conflict monitoring

Original prompt from Ayush

SYSTEM:
<latest_message>
Ayush Patel (U0B8L6RNMA9) [ts=1789652370.558029]: &gt; ◦ provider region 403 from the modal sandbox, Fugu Ultra/Max terminal-bench blocked. needs sandbox region pinning or egress through OR. no PR. maybe we just support pinning sandbox region? modal accepts a param
@Devin can you make a pr to enable this and default to US in benchmark-harness / openrouter-web
read modal upstream docs if needed, see the price diff if we region pin
</latest_message>

=== BEGIN THREAD HISTORY (in #agents) ===
Ayush Patel (U0B8L6RNMA9) [ts=1789652370.558029]: &gt; ◦ provider region 403 from the modal sandbox, Fugu Ultra/Max terminal-bench blocked. needs sandbox region pinning or egress through OR. no PR. maybe we just support pinning sandbox region? modal accepts a param
@Devin can you make a pr to enable this and default to US in benchmark-harness / openrouter-web
read modal upstream docs if needed, see the price diff if we region pin
=== END THREAD HISTORY ===
Channel ID: C07UF9XLTFF
Thread URL: https://openrouter.slack.com/archives/C07UF9XLTFF/p1789652370558029?thread_ts=1789652370.558029&amp;cid=C07UF9XLTFF

The <latest_message> is the message that you should use to guide your goals + task for this session, and you should use the rest of the slack thread as context.
A [ts=...] marker on a Slack message is that message's timestamp. To act on a specific message with the slack tool (e.g. adding an emoji reaction via the reaction command), pass that value as timestamp along with the Channel ID — no extra lookup call is needed.

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.

1 participant