Skip to content

fix(mirage-sandbox): don't re-home imported functions onto the sandbox namespace - #94

Merged
YoanSallami merged 1 commit into
mainfrom
fix/sandbox-rehome-stdlib-globals
Aug 16, 2026
Merged

fix(mirage-sandbox): don't re-home imported functions onto the sandbox namespace#94
YoanSallami merged 1 commit into
mainfrom
fix/sandbox-rehome-stdlib-globals

Conversation

@YoanSallami

Copy link
Copy Markdown
Contributor

The bug

MirageSandbox persists its namespace between run calls with dill, then re-homes restored functions onto the live ns so they share one global namespace (#92). The predicate matched any function whose __globals__ was not ns — including functions imported from a module:

sandbox.run("from collections import Counter")   # run 1
sandbox.run("Counter('aa')")                     # run 2
# NameError: name '_collections_abc' is not defined

An imported function closes over its own module's globals and reaches names in them at call time. Counter.update reads _collections_abc and _count_elements from collections' globals; os.path.join reads sep from posixpath's. Rebuilding them on ns strips exactly those.

Because classes are re-homed in place (setattr on the class object), the damage outlived the name that triggered it: after any run imported Counter, a later plain import collections inherited the broken class.

Found in a long agent run, where it accounted for 264 sandbox failures across four tasks — _collections_abc (122), sep (62), _itemgetter (48), _count_elements (32). It only bites from the second run onward, which is what makes it confusing in the wild: the import turn works, and the code that uses the import fails.

The fix

Re-home only functions defined in the sandbox. Their ghost globals are a copy of ns, which carries __name__ == "__main__"; an imported function carries its own module name. One extra clause on the existing predicate.

Tests

Two colocated tests, both failing on main and passing here:

  • test_imported_function_keeps_its_own_module_globalsos.path.join still works a run after it was imported.
  • test_imported_class_methods_keep_their_module_globalsCounter works a run after import, and a subsequent plain import collections is undamaged.

The cross-run cases #92 added (test_function_sees_names_defined_in_later_runs, test_method_sees_names_defined_in_later_runs, test_rehomed_function_keeps_closure_and_kwdefaults) still pass, so this narrows the predicate without giving back what #92 bought.

Full sandbox suite: 121 passed, 9 skipped.

🤖 Generated with Claude Code

…x namespace

Re-homing restored functions onto the live namespace (#92) matched *any*
function whose globals were not `ns`, including ones imported from a module.
An imported function closes over its module's globals and reaches names in
them at call time, so rebuilding it on `ns` strips those names:

    from collections import Counter   # run 1
    Counter("aa")                     # run 2
    NameError: name '_collections_abc' is not defined

`Counter.update` reads `_collections_abc` and `_count_elements` from
`collections`' own globals; `os.path.join` reads `sep` from `posixpath`'s.
Because classes are re-homed in place via `setattr`, the damage outlived the
name that triggered it — a later plain `import collections` inherited the
broken `Counter`.

Re-home only functions defined in the sandbox. Their (ghost) globals are a
copy of `ns`, which carries `__name__ == "__main__"`; an imported function
carries its own module name. The cross-run cases #92 fixed are unaffected and
still covered.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@YoanSallami
YoanSallami merged commit fcdec8a into main Aug 16, 2026
6 checks passed
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