From 6d1591a2f2c62992d68eb0bcb8ee744a3d761728 Mon Sep 17 00:00:00 2001 From: Joseph Ibrahim Date: Fri, 29 May 2026 16:58:47 -0400 Subject: [PATCH 1/7] fix(inspector,ci): exec-scoping crash in stage extraction + missing CI test deps inspector: the /stage extraction script runs through the bridge's execute_python, which execs with split globals/locals -- so the module-level `import hou` / `import json` were invisible inside `_synapse_extract_flat_ast`, raising NameError that the top-level handler swallowed as the generic `extraction_script_crash`. Import locally inside the function. Verified live on 21.0.671: extraction now returns the full /stage AST (17 nodes, json.dumps OK) where it previously crashed; inspector unit tests stay green (85 passed). ci: the `dev` extra was missing test deps the Linux CI runner needs, so `pytest tests/` failed en masse on master: - pytest-asyncio -> test_solaris_ordering ("async def functions not natively supported") - numpy -> test_frame_validator ("No module named 'numpy'") - anthropic -> test_host_layer ("anthropic SDK is not installed"; the vendored SDK is Windows-only binaries, unusable on the Linux runner) Added all three to `dev`. Remaining CI reds are platform-specific (Windows-only toast test, mock-path asserts that diverge Linux-vs-Windows) -- tracked as a follow-up. Co-Authored-By: Claude Opus 4.8 (1M context) --- pyproject.toml | 8 ++++++++ python/synapse/inspector/tool_inspect_stage.py | 7 +++++++ 2 files changed, 15 insertions(+) diff --git a/pyproject.toml b/pyproject.toml index 698beff..c67fca4 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -33,6 +33,14 @@ dependencies = [ dev = [ "pytest>=7.0", "pytest-cov>=4.0", + # CI test deps (Linux runners). Without these, pytest tests/ fails: + # - pytest-asyncio: test_solaris_ordering async tests ("not natively supported") + # - numpy: test_frame_validator ("No module named 'numpy'") + # - anthropic: test_host_layer ("anthropic SDK is not installed") -- the + # vendored SDK is Windows-only binaries, unusable on the Linux CI runner. + "pytest-asyncio>=0.21", + "numpy>=1.24", + "anthropic>=0.40.0", ] websocket = [ "websockets>=11.0", diff --git a/python/synapse/inspector/tool_inspect_stage.py b/python/synapse/inspector/tool_inspect_stage.py index 3aa9387..42f748d 100644 --- a/python/synapse/inspector/tool_inspect_stage.py +++ b/python/synapse/inspector/tool_inspect_stage.py @@ -198,6 +198,13 @@ def _synapse_extract_node(n): def _synapse_extract_flat_ast(target_path): + # The bridge execs this script with split globals/locals, so module-level + # imports are NOT visible inside functions (NameError -> swallowed as + # extraction_script_crash). Import locally so the function is self-sufficient. + # Verified live on 21.0.671: extraction succeeds with local imports; with + # module-level imports it raised "name 'json'/'hou' is not defined". + import hou + import json parent = hou.node(target_path) if parent is None: return json.dumps({ From 912ccc18bac74ff2ca83a8ceaf7b1bca843c2fd8 Mon Sep 17 00:00:00 2001 From: Joseph Ibrahim Date: Fri, 29 May 2026 18:30:57 -0400 Subject: [PATCH 2/7] fix(ci): green test_design_system via repo-source fallback + guard toast CREATE_NO_WINDOW test_design_system pointed only at the DEPLOYED base (~/.synapse), which the CI runner doesn't have -> ~30 failures (ModuleNotFound tokens/synapse_styles, FileNotFound shelf/panel/installer, "0 SVGs"). Fall back to the REPO source (design/, houdini/, install.py) when nothing is deployed; identical layout, so deployed installs are still validated when present. send_toast used subprocess.CREATE_NO_WINDOW (a Windows-only attribute); on the Linux runner, with os.name patched to "nt", evaluating it raised AttributeError before subprocess.run, so the toast tests saw call_args=None. Guard with getattr(subprocess, "CREATE_NO_WINDOW", 0) -- harmless on Windows, correct on Linux. Co-Authored-By: Claude Opus 4.8 (1M context) --- python/synapse/server/render_notify.py | 2 +- tests/test_design_system.py | 11 ++++++++++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/python/synapse/server/render_notify.py b/python/synapse/server/render_notify.py index 4d606b4..13a2453 100644 --- a/python/synapse/server/render_notify.py +++ b/python/synapse/server/render_notify.py @@ -139,7 +139,7 @@ def send_toast(title: str, body: str) -> bool: capture_output=True, text=True, timeout=10, - creationflags=subprocess.CREATE_NO_WINDOW if os.name == "nt" else 0, + creationflags=(getattr(subprocess, "CREATE_NO_WINDOW", 0) if os.name == "nt" else 0), ) if result.returncode != 0: logger.debug("Toast PowerShell failed: %s", result.stderr[:200]) diff --git a/tests/test_design_system.py b/tests/test_design_system.py index e2b1ce4..5b59c31 100644 --- a/tests/test_design_system.py +++ b/tests/test_design_system.py @@ -20,7 +20,16 @@ # ── Setup ───────────────────────────────────────────────── -_SYNAPSE_HOME = os.path.join(os.path.expanduser("~"), ".synapse") +# Prefer the DEPLOYED copy (~/.synapse) so this also validates a real install; +# fall back to the REPO source when nothing is deployed (CI runners, fresh dev +# checkouts). The layout is identical either way: /{design,houdini,install.py}. +_DEPLOYED_HOME = os.path.join(os.path.expanduser("~"), ".synapse") +_REPO_ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) +_SYNAPSE_HOME = ( + _DEPLOYED_HOME + if os.path.isdir(os.path.join(_DEPLOYED_HOME, "design")) + else _REPO_ROOT +) _DESIGN_DIR = os.path.join(_SYNAPSE_HOME, "design") _HOUDINI_DIR = os.path.join(_SYNAPSE_HOME, "houdini") _SVG_DIR = os.path.join(_DESIGN_DIR, "icons", "svg") From 7571484c62565767b068bd4b3c985c960d124fd4 Mon Sep 17 00:00:00 2001 From: Joseph Ibrahim Date: Fri, 29 May 2026 18:41:10 -0400 Subject: [PATCH 3/7] fix(ci): green the final 6 Linux-only test failures (test-only) test_render (4x "an integer is required"): the helper globally patches pathlib.Path.stat to a bare MagicMock with no st_mode. handlers_render.py:288 runs render_dir_path.mkdir(parents=True, exist_ok=True); on Linux, when the temp render dir already exists, mkdir's internal is_dir() calls S_ISDIR(stat().st_mode) -- st_mode is a MagicMock -> "an integer is required". On Windows the dir didn't pre-exist so stat was never hit. Give the stat mock a real int st_mode (directory) so is_dir() works; st_size stays for the flush poll. The handler is correct. test_layout matlib (2x): handlers_node binds `hou` at import time from sys.modules["hou"]. In the full suite it gets bound to a different stub than test_layout's _mock_hou, so patch.object(_mock_hou, "node", ...) never reaches the handler -> hou.node() returns a default mock -> result["shader_path"] is a raw createNode().createNode() chain and uv.parm("signature") is never called. mock_env now rebinds handlers_node.hou to _mock_hou (mirrors the existing handler_helpers rebind at module load), making the test order-independent. Both are test-only; production code unchanged. Local: 62 passed. Co-Authored-By: Claude Opus 4.8 (1M context) --- tests/test_layout_and_matlib.py | 10 ++++++++++ tests/test_render.py | 2 +- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/tests/test_layout_and_matlib.py b/tests/test_layout_and_matlib.py index fb023c1..50e0f23 100644 --- a/tests/test_layout_and_matlib.py +++ b/tests/test_layout_and_matlib.py @@ -262,6 +262,16 @@ def mock_env(self): from synapse.server import main_thread main_thread._HOU_AVAILABLE = True main_thread._USE_DEFERRED = False + + # Rebind the `hou` the node handler actually uses to THIS module's stub. + # handlers_node binds `hou` at import time from sys.modules["hou"]; in the + # full suite it can be imported against a different stub, so patching + # _mock_hou.node would never reach the handler -> hou.node() returns a + # default mock -> raw createNode chain. Forcing alignment makes the test + # order-independent (and is a no-op when they already match). + from synapse.server import handlers_node + handlers_node.hou = _mock_hou + handlers_node.HOU_AVAILABLE = True return _mock_hou def _make_parent_node(self): diff --git a/tests/test_render.py b/tests/test_render.py index 07b2787..eaef302 100644 --- a/tests/test_render.py +++ b/tests/test_render.py @@ -202,7 +202,7 @@ def _parm(n): patch.object(_handlers_hou, "frame", return_value=frame, create=True), \ patch.object(_handlers_hou, "text", MagicMock(expandString=MagicMock(return_value="/tmp/houdini_temp")), create=True), \ patch("pathlib.Path.exists", return_value=True), \ - patch("pathlib.Path.stat", return_value=MagicMock(st_size=1024)): + patch("pathlib.Path.stat", return_value=MagicMock(st_size=1024, st_mode=0o040755)): return handler._handle_render(payload) def test_returns_image_path_and_engine(self, handler): From 6635ea9ab4ecc260249c25c2652cee6d05582bfb Mon Sep 17 00:00:00 2001 From: Joseph Ibrahim Date: Fri, 29 May 2026 18:47:05 -0400 Subject: [PATCH 4/7] fix(ci): scope handlers_node.hou rebind with monkeypatch to stop leak The previous commit rebned handlers_node.hou (+ HOU_AVAILABLE) via direct module-global assignment in the mock_env fixture. With no teardown it leaked into every later test that uses handlers_node -> 14 regressions in test_network_explain (empty results, 0 == 3) and test_mcp_roundtrip (hou.node() returned a truthy mock instead of None, so "missing parent" no longer raised -> True is False). Scope both rebinds with monkeypatch so they auto-restore after each test. Verified against the full local suite in CI collection order: test_render / test_layout / test_network_explain / test_mcp_roundtrip all green. (The only full-suite-local failures are test_agent_state / test_scene_memory, which assert the no-pxr path and only fail because this machine has Houdini's pxr installed; CI has no pxr so they pass there.) Co-Authored-By: Claude Opus 4.8 (1M context) --- tests/test_layout_and_matlib.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/tests/test_layout_and_matlib.py b/tests/test_layout_and_matlib.py index 50e0f23..ca8c0bd 100644 --- a/tests/test_layout_and_matlib.py +++ b/tests/test_layout_and_matlib.py @@ -249,7 +249,7 @@ class TestMaterialLibraryAutoPopulate: """Test that create_node('materiallibrary') auto-scaffolds MaterialX.""" @pytest.fixture - def mock_env(self): + def mock_env(self, monkeypatch): """Set up mock hou environment for handler testing.""" # Stub hdefereval for synchronous execution if "hdefereval" not in sys.modules: @@ -263,15 +263,15 @@ def mock_env(self): main_thread._HOU_AVAILABLE = True main_thread._USE_DEFERRED = False - # Rebind the `hou` the node handler actually uses to THIS module's stub. - # handlers_node binds `hou` at import time from sys.modules["hou"]; in the - # full suite it can be imported against a different stub, so patching - # _mock_hou.node would never reach the handler -> hou.node() returns a - # default mock -> raw createNode chain. Forcing alignment makes the test - # order-independent (and is a no-op when they already match). + # Rebind the `hou` the node handler actually uses to THIS module's stub, + # scoped via monkeypatch so it auto-restores after the test and never + # leaks to later suites. handlers_node binds `hou` at import time from + # sys.modules["hou"]; in the full suite it can be bound to a different + # stub, so patch.object(_mock_hou, "node", ...) would never reach the + # handler -> hou.node() returns a default mock -> raw createNode chain. from synapse.server import handlers_node - handlers_node.hou = _mock_hou - handlers_node.HOU_AVAILABLE = True + monkeypatch.setattr(handlers_node, "hou", _mock_hou, raising=False) + monkeypatch.setattr(handlers_node, "HOU_AVAILABLE", True, raising=False) return _mock_hou def _make_parent_node(self): From c5843561ed3dd5ff363d940057381d6a74277f8c Mon Sep 17 00:00:00 2001 From: Joseph Ibrahim Date: Fri, 29 May 2026 18:51:18 -0400 Subject: [PATCH 5/7] fix(ci): patch Path.mkdir in the two flipbook render tests test_flipbook_fallback_on_usdrender_no_output and test_no_flipbook_fallback_for_non_usdrender patched Path.stat but not Path.mkdir -- unlike their 3 sibling render tests. handlers_render.py:288 runs render_dir_path.mkdir(parents=True, exist_ok=True); on Linux when the temp dir already exists, mkdir's internal is_dir() calls S_ISDIR(stat().st_mode) on the MagicMock -> "an integer is required". Patch mkdir to a no-op, matching the sibling tests. Test-only; handler is correct. Local: 2 passed. Co-Authored-By: Claude Opus 4.8 (1M context) --- tests/test_render.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/test_render.py b/tests/test_render.py index eaef302..b2969f8 100644 --- a/tests/test_render.py +++ b/tests/test_render.py @@ -298,6 +298,7 @@ def _fake_exists(self_path): patch.object(_handlers_hou, "text", MagicMock(expandString=MagicMock(return_value="/tmp/houdini_temp")), create=True), \ patch("pathlib.Path.exists", _fake_exists), \ patch("pathlib.Path.stat", return_value=MagicMock(st_size=2048)), \ + patch("pathlib.Path.mkdir", return_value=None), \ patch("time.sleep"): result = handler._handle_render({"node": "/stage/usdrender_rop1"}) @@ -322,6 +323,7 @@ def test_no_flipbook_fallback_for_non_usdrender(self, handler): patch.object(_handlers_hou, "text", MagicMock(expandString=MagicMock(return_value="/tmp/houdini_temp")), create=True), \ patch("pathlib.Path.exists", return_value=False), \ patch("pathlib.Path.stat", return_value=MagicMock(st_size=0)), \ + patch("pathlib.Path.mkdir", return_value=None), \ patch("time.sleep"): with pytest.raises(RuntimeError, match="output wasn't created"): handler._handle_render({"node": "/out/mantra1"}) From 338e23383e07cb4e4ffdc96d2d390c7efc3fca86 Mon Sep 17 00:00:00 2001 From: Joseph Ibrahim Date: Fri, 29 May 2026 19:10:39 -0400 Subject: [PATCH 6/7] fix(inspector): complete the split-scope fix -- re-publish top-level names to globals PR #5 added local `import hou`/`import json` to _synapse_extract_flat_ast, but that was INCOMPLETE. The bridge execs the extraction script with exec(code, G, L) where G != L (handlers.py _run_compiled / api_adapter _run_in_namespace). Top-level defs, imports and constants bind into L, but the script's functions resolve names via their __globals__ (= G). So from INSIDE the functions: - _synapse_extract_node (a top-level def) is invisible -> _synapse_extract_flat_ast crashes at the call site on EVERY scene (NameError -> extraction_script_crash) - SCHEMA_VERSION / _MAX_ERR_LEN (top-level consts) are invisible; an errored or warned node double-faults on _MAX_ERR_LEN inside its own except handler The original "json/hou not defined" was merely the FIRST name hit; fixing it just moved the crash downstream. Confirmed live: synapse_inspect_stage("/stage") on a clean Cornell Box returned extraction_script_crash. Fix: after the function defs, re-publish the names the functions need into G (globals() at the exec'd module level IS G). Drop the now-redundant local imports and their (disproven) "verified live" comment. Add tests/test_inspect_mock.py::TestExtractionScriptSplitScope: execs the REAL template under split globals/locals (hou NOT injected) across clean/errored/ warned nodes. Red against the prior template (NameError _synapse_extract_node), green now. The transport-mocked tests never exec the real script, so they missed this entirely. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../synapse/inspector/tool_inspect_stage.py | 31 +++++-- tests/test_inspect_mock.py | 88 +++++++++++++++++++ 2 files changed, 112 insertions(+), 7 deletions(-) diff --git a/python/synapse/inspector/tool_inspect_stage.py b/python/synapse/inspector/tool_inspect_stage.py index 42f748d..77a34ea 100644 --- a/python/synapse/inspector/tool_inspect_stage.py +++ b/python/synapse/inspector/tool_inspect_stage.py @@ -198,13 +198,9 @@ def _synapse_extract_node(n): def _synapse_extract_flat_ast(target_path): - # The bridge execs this script with split globals/locals, so module-level - # imports are NOT visible inside functions (NameError -> swallowed as - # extraction_script_crash). Import locally so the function is self-sufficient. - # Verified live on 21.0.671: extraction succeeds with local imports; with - # module-level imports it raised "name 'json'/'hou' is not defined". - import hou - import json + # hou / json / SCHEMA_VERSION / _synapse_extract_node all resolve via the + # globals() re-publish before the top-level try (split globals/locals exec + # would otherwise hide them inside this function). See that note for why. parent = hou.node(target_path) if parent is None: return json.dumps({ @@ -229,6 +225,27 @@ def _synapse_extract_flat_ast(target_path): }, sort_keys=True) +# The bridge execs this script with split globals/locals -- exec(code, G, L) +# where G != L (see handlers.py _run_compiled / api_adapter _run_in_namespace). +# Top-level defs, imports and constants bind into L, but functions resolve +# names via their __globals__ (= G), so from INSIDE the functions the other +# top-level function (_synapse_extract_node), the module constants +# (SCHEMA_VERSION, _MAX_ERR_LEN) and the imports are all invisible -> NameError, +# swallowed as extraction_script_crash. `globals()` here (module level) IS G, +# so re-publish the names the functions need into G. Without this, a clean +# scene crashes at the _synapse_extract_node call and any errored/warned node +# double-faults on _MAX_ERR_LEN inside its own except handler. +globals().update({ + "hou": hou, + "json": json, + "traceback": traceback, + "SCHEMA_VERSION": SCHEMA_VERSION, + "_MAX_ERR_LEN": _MAX_ERR_LEN, + "_synapse_extract_node": _synapse_extract_node, + "_synapse_extract_flat_ast": _synapse_extract_flat_ast, +}) + + try: print(_synapse_extract_flat_ast(%(target_path_literal)s)) except Exception as ex: diff --git a/tests/test_inspect_mock.py b/tests/test_inspect_mock.py index f5decff..f832f45 100644 --- a/tests/test_inspect_mock.py +++ b/tests/test_inspect_mock.py @@ -646,3 +646,91 @@ def test_error_message_length_capped(self): error_state="error", error_message="x" * 501, ) + + +# ----------------------------------------------------------------------------- +# Split-scope exec — the extraction script must survive the bridge's +# exec(code, globals, locals) with globals != locals (handlers.py _run_compiled +# / api_adapter _run_in_namespace). Regression for the inspector crash where +# _synapse_extract_node / SCHEMA_VERSION / _MAX_ERR_LEN were defined at the +# script's top level (-> locals) but referenced from inside its functions +# (-> resolved via globals), yielding extraction_script_crash on EVERY scene +# (and a double-fault on errored nodes via _MAX_ERR_LEN in the except handler). +# The transport-mocked tests above never exec the real script, so they missed it. +# ----------------------------------------------------------------------------- + + +class TestExtractionScriptSplitScope: + @staticmethod + def _child(name, node_type="null", errs=(), warns=()): + from unittest.mock import MagicMock + c = MagicMock() + c.name.return_value = name + c.type.return_value.name.return_value = node_type + c.path.return_value = "/stage/" + name + c.errors.return_value = list(errs) + c.warnings.return_value = list(warns) + c.isDisplayFlagSet.return_value = False + c.isBypassed.return_value = False + c.lastModifiedPrims.return_value = [] + c.inputs.return_value = [] + c.outputs.return_value = [] + return c + + def _run_template(self, children): + """Exec the REAL extraction template the way the bridge does: split + globals/locals, with `hou` NOT injected into globals (the inspector + transport doesn't). Returns the parsed JSON the script prints.""" + import io + import sys + import types + from contextlib import redirect_stdout + + from synapse.inspector.tool_inspect_stage import _EXTRACTION_SCRIPT_TEMPLATE + + parent = type("P", (), {})() + parent.children = lambda: list(children) + mock_hou = types.ModuleType("hou") + mock_hou.node = lambda p: parent if p == "/stage" else None + + script = _EXTRACTION_SCRIPT_TEMPLATE % { + "schema_version": SCHEMA_VERSION, + "target_path_literal": repr("/stage"), + } + saved = sys.modules.get("hou") + sys.modules["hou"] = mock_hou + try: + g = {"__builtins__": __builtins__} # split scope; hou NOT in globals + l = {} + buf = io.StringIO() + with redirect_stdout(buf): + exec(compile(script, "", "exec"), g, l) + finally: + if saved is not None: + sys.modules["hou"] = saved + else: + sys.modules.pop("hou", None) + return json.loads(buf.getvalue().strip()) + + def test_clean_scene_does_not_crash(self): + data = self._run_template([self._child("a"), self._child("b")]) + assert data.get("synapse_error") != "extraction_script_crash", data + assert data["schema_version"] == SCHEMA_VERSION + assert len(data["nodes"]) == 2 + assert all(n["error_state"] == "clean" for n in data["nodes"]) + + def test_errored_and_warned_nodes_survive_split_scope(self): + # The error/warning paths AND the per-node except handler reference + # _MAX_ERR_LEN -- a top-level constant invisible inside the function + # under split scope. Pre-fix this double-faulted into a script crash. + data = self._run_template([ + self._child("clean"), + self._child("boom", errs=["kaboom detail"]), + self._child("warn", warns=["a warning"]), + ]) + assert data.get("synapse_error") != "extraction_script_crash", data + by_name = {n["node_name"]: n for n in data["nodes"]} + assert by_name["boom"]["error_state"] == "error" + assert "kaboom detail" in (by_name["boom"]["error_message"] or "") + assert by_name["warn"]["error_state"] == "warning" + assert by_name["clean"]["error_state"] == "clean" From b2b3b34b3df3aebf2258ad45f65e2934bd0b0651 Mon Sep 17 00:00:00 2001 From: Joseph Ibrahim Date: Fri, 29 May 2026 19:16:41 -0400 Subject: [PATCH 7/7] test(render): patch Path.mkdir in _mock_hdefereval_and_run (hermetic + version-robust) The render helper added st_mode to the stat mock but -- unlike its sibling helpers (the flipbook tests and _setup_render) -- never patched Path.mkdir. So the three tests using it (test_returns_image_path_and_engine, test_frame_defaults_to_current, test_resolution_override) ran a REAL mkdir(parents=True, exist_ok=True) against the CI /tmp, creating /tmp/houdini_temp/.synapse/renders -- a filesystem side effect from a supposedly pure mock test. On Python 3.14, Path.is_dir() routes through os.path.isdir (not stat), so the st_mode trick was inert and the tests passed only because /tmp happened to be writable; on 3.11 they relied on the FileExistsError->is_dir->S_ISDIR(st_mode) branch. Two different behaviors across the matrix. Patch Path.mkdir to a no-op (matching the siblings) and drop the now-inert st_mode. Hermetic and identical on both 3.11 and 3.14; no FS side effects. Surfaced by the adversarial pre-merge review (findings #5/#6/#9/#14, all CONFIRMED). Co-Authored-By: Claude Opus 4.8 (1M context) --- tests/test_render.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test_render.py b/tests/test_render.py index b2969f8..317890f 100644 --- a/tests/test_render.py +++ b/tests/test_render.py @@ -202,7 +202,8 @@ def _parm(n): patch.object(_handlers_hou, "frame", return_value=frame, create=True), \ patch.object(_handlers_hou, "text", MagicMock(expandString=MagicMock(return_value="/tmp/houdini_temp")), create=True), \ patch("pathlib.Path.exists", return_value=True), \ - patch("pathlib.Path.stat", return_value=MagicMock(st_size=1024, st_mode=0o040755)): + patch("pathlib.Path.stat", return_value=MagicMock(st_size=1024)), \ + patch("pathlib.Path.mkdir", return_value=None): return handler._handle_render(payload) def test_returns_image_path_and_engine(self, handler):