Make the sandbox name pattern per-supervisor, not a mutable global - #288
Merged
Conversation
Supervisor.spawn declared `global NAME_PATTERN`, read it, and reset it to DEFAULT_NAME_PATTERN on every successful spawn. Two problems: * A caller that installed a custom pattern silently lost it after one sandbox, and two threads spawning concurrently raced over the value -- one thread's reset could reject the other's legitimate name. * The reset existed only so tests/test_metrics.py could rebind the global without restoring it. Production code was carrying a test affordance. Add a name_pattern argument to Supervisor so the rule belongs to the instance that enforces it, exposed as a .name_pattern property. When it is not given the supervisor falls back to the module-level NAME_PATTERN at spawn time, so the documented global override still works for the process-wide supervisor -- but spawn no longer writes to it. Restoration moves to where it belongs: the metrics test uses monkeypatch.setattr, which undoes itself. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ebvMQ3vLxdK3joymz6Feg
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.
The problem
Supervisor.spawndeclaredglobal NAME_PATTERN, read it, and reset it toDEFAULT_NAME_PATTERNon every successful spawn:Two problems:
tests/test_metrics.pyrebound the module global and never restored it, relying onspawnto clean up. Production code was carrying a test affordance.The change
Supervisor(name_pattern=...)— the rule belongs to the instance that enforces it, exposed as a.name_patternproperty. When not given, the supervisor falls back to the module-levelNAME_PATTERNat spawn time, so the documented global override still works for the process-wide supervisor.spawnno longer writes to it.Restoration moves to where it belongs: the metrics test uses
monkeypatch.setattr, which undoes itself.Tests
Three new in
tests/test_supervisor.py: a custom pattern is honored and rejects non-matching names; two supervisors keep independent patterns; and the regression itself — a custom global survives a spawn and is still honored on the next one.Full suite 508 passed / 6 skipped (was 505). flake8 clean.
Note
I let
blacktouch only the files this change edits. A newer black than the repo's pinned 23.9.1 wanted to reformatnogil.py,test_ebpf_contract.py, andtest_thread_extra.py— unrelated, so I reverted those.Generated by Claude Code