Split the tongs launcher core into one module per concern - #38
Merged
Conversation
`swarmforge/tongs.py` had reached 1800 lines and eight concerns kept apart only by comment banners. Those banners were already a table of contents, so each becomes a module under `swarmforge/tongs/`: `model`, `discovery`, `validate`, `secrets`, `mounts`, `mcp`, `network`, `approvals`, `argv`, and the diagnostic `cli`. Nothing crosses a seam it did not already sit behind. Every top-level name keeps its body byte for byte; the new text is the module headers, the package docstring, and one comment on `_is_int`. Four helpers follow their subject rather than their old banner. `_is_int` and the readiness resolvers join the schema vocabulary in `model`, the leaf module every other one imports. `_has_socket_mount` joins the mount words it reads. `_is_network_facing` joins the interface kinds it dispatches on, which is what keeps `argv` from importing `network` for a one-line predicate. The environment-variable naming folds into `mcp`, beside the interface wiring that is its only caller. `__init__.py` re-exports the whole public surface, so `swarmforge.anvil`, `bin/tongs`, and the tests import exactly the names they did before, and `__main__.py` keeps `python3 -m swarmforge.tongs` working. The CLI prints byte-identical output for `validate`, `discover`, and its usage error.
Splitting the core into modules gave `SECRET_FIFO_TARGET` two bindings: the one `secret_inject_argv` reads, and the copy the package re-exports. This test redirects that path to a file it knows is missing and asserts the wrapper aborts instead of exec'ing the tong's real entrypoint with no secret environment. Redirecting the re-export left the real `/run/swarmforge/ secret-env` baked into the script, so the test passed only because that path happens not to exist on a test host -- and would have hung had it existed as a FIFO with no writer. Point the redirect at `swarmforge.tongs.secrets` instead. With the path made readable the wrapper now execs the target again, which is what makes the failure case worth asserting.
The explicit package list is what makes imports resolve for editors, type checkers and linters; a subpackage missing from it is invisible to them.
The test asserted only that the wrapper exited non-zero with no output. Both hold when the redirect never reaches the script and the wrapper reads the real `/run/swarmforge/secret-env`, which is absent on a test host -- the exact failure mode redirecting the package re-export produces, and the one the previous commit fixed. So the test could not detect a reintroduction of the bug it exists to prevent. Assert the temp path actually appears in the generated script. Reverting the redirect to `tongs.SECRET_FIFO_TARGET` now fails the test instead of passing it, which is what pins the fail-closed guarantee that a tong never execs its target without its secret environment.
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.
swarmforge/tongs.pyhad reached 1800 lines and roughly eighty top-level functions spanning eight concerns, kept apart only by comment banners. Those banners were already a table of contents, so each becomes its own module underswarmforge/tongs/:modeldiscoveryvalidatesecretsmountsmounts:magic words and their docker bind specsmcpinterface:contributes to the anvilnetworkapprovalsargvclivalidate/discoverdiagnostic commands__init__.pyre-exports the whole public surface, soswarmforge.anvil,bin/tongs, and the tests import exactly the names they did before — no call site churns in this PR.__main__.pykeepspython3 -m swarmforge.tongsworking.This is a move, not a change
Every top-level name keeps its body byte for byte. Verified by parsing the old module and the ten new ones and comparing each definition's source segment: 112 of 112 identical, none missing, none added, none duplicated across modules. The new text is the module headers, the package docstring, and one comment on
_is_int.Four helpers follow their subject rather than their old banner:
_is_intand the readiness resolvers join the schema vocabulary inmodel(the leaf every other module imports),_has_socket_mountjoins the mount words it reads, and_is_network_facingjoins the interface kinds it dispatches on — which is what keepsargvfrom importingnetworkfor a one-line predicate. The import graph is acyclic withmodelat the leaf.Also checked: the CLI prints byte-identical stdout, stderr, and exit codes for
validate,discover, and its usage error; importing the package has no observable effect beyond defining names; no dependency was added, and the modules import nothing outside the standard library andswarmforge.yamlite.The one thing the split broke, and the fix
Splitting a module means
from .secrets import SECRET_FIFO_TARGETcopies the binding, so a constant that had one home now has several.tests/test_tongs.pyredirects that path at a file it knows is missing and asserts the secret wrapper aborts rather than exec'ing the tong's real entrypoint with no secret environment. Redirecting the re-exported copy left the real/run/swarmforge/secret-envbaked into the script, so the test kept passing for the wrong reason — and would have hung had that path existed as a FIFO with no writer.The redirect now lands on
swarmforge.tongs.secrets. With the path made readable the wrapper execs the target again, which is what makes the failure case worth asserting. The package docstring records the rule so the next test does not repeat it.Rounding it out:
pyproject.tomllists subpackages explicitly and would have silently omitted the new one.Testing
python3 -m unittest discover -s tests -p 'test_*.py'— 495 tests, OK, unchanged from before the split. The passthrough tests (tests/test_run_anvil.py, which spawn the real launcher and assert byte-identical forwarding when no tongs are discovered) and the launch-path tests (tests/test_run_agent_container.py, which drive the real make recipes withdockerstubbed) are green and untouched.