Guard all four PyCoder result/error handlers against a disposed node

graphlink_app/graphlink_window_actions.py

@@ -1120,6 +1120,13 @@ def stop_pycoder_node(self, pycoder_node):
         pycoder_node.set_ai_analysis("Execution manually stopped.")
 
     def _handle_code_execution_result(self, output, pycoder_node, parent_history):
+        # Manual-mode stage 1 (execution) finishing kicks off stage 2 (AI
+        # analysis) below as a SEPARATE, window-level thread (self.pycoder_agent_thread,
+        # not node-scoped) - dispose() has no way to reach or stop it. Guarding here
+        # stops a disposed node from being touched further AND stops a pointless
+        # second-stage thread from ever starting for a node that's already gone.
+        if not pycoder_node or getattr(pycoder_node, "is_disposed", False):
+            return
         pycoder_node.set_output(output)
         code = pycoder_node.get_code()
         user_msg = f"--- EXECUTED PYTHON CODE ---\n```python\n{code}\n```\n\n--- EXECUTION OUTPUT ---\n{output}"
@@ -1132,6 +1139,14 @@ def _handle_code_execution_result(self, output, pycoder_node, parent_history):
         self.pycoder_agent_thread.start()
 
     def _handle_pycoder_analysis_result(self, analysis, pycoder_node, parent_history, user_msg):
+        # Bug-scan finding: stage 2 (self.pycoder_agent_thread, a window-level
+        # PyCoderAgentWorker with no stop() of its own - an in-flight LLM call
+        # can't be cancelled early) used to run unconditionally. Deleting
+        # pycoder_node mid-analysis let this handler mutate/reselect/save a
+        # node no longer on the canvas. Matches the guard every sibling result
+        # handler already has (_handle_artifact_result, _handle_code_sandbox_result).
+        if not pycoder_node or getattr(pycoder_node, "is_disposed", False):
+            return
         # We explicitly bundle the code, the output, and the analysis so downstream nodes inherit them.
         assign_history(pycoder_node, append_history(parent_history, [
             {'role': 'user', 'content': user_msg},
@@ -1143,6 +1158,8 @@ def _handle_pycoder_analysis_result(self, analysis, pycoder_node, parent_history
         self.save_chat()
 
     def _handle_ai_pycoder_result(self, result_dict, pycoder_node, parent_history):
+        if not pycoder_node or getattr(pycoder_node, "is_disposed", False):
+            return
         analysis_text = result_dict.get('analysis', '')
         code = result_dict.get('code', '')
         output = result_dict.get('output', '')
@@ -1164,6 +1181,11 @@ def _handle_ai_pycoder_result(self, result_dict, pycoder_node, parent_history):
         self.save_chat()
 
     def _handle_pycoder_error(self, error_message, pycoder_node):
+        # Shared by every PyCoder worker stage (manual execution, AI-driven
+        # generation, and the manual-mode analysis follow-up) - all three can
+        # report an error after the node was disposed.
+        if not pycoder_node or getattr(pycoder_node, "is_disposed", False):
+            return
         pycoder_node.set_ai_analysis(f"An error occurred: {error_message}"); pycoder_node.set_running_state(False)
 
     def execute_code_sandbox_node(self, sandbox_node):

graphlink_app/tests/test_pycoder_dispose_guards.py

@@ -0,0 +1,148 @@
+"""Bug-scan finding (worklist item 4): PyCoderNode's result/error handlers
+were the one worker-result-handler family with no is_disposed guard at all -
+every sibling (_handle_artifact_result/_handle_artifact_error,
+_handle_code_sandbox_result/_handle_code_sandbox_error) already had one
+(audit finding A3 for the artifact pair).
+
+Concretely: manual-mode execution finishing kicks off a SECOND, window-level
+thread (self.pycoder_agent_thread, a PyCoderAgentWorker with no stop() of its
+own - an in-flight LLM call can't be cancelled early) to get an AI analysis of
+the output. If the node is deleted (dispose()'d) while that second-stage
+thread is still running, dispose() has no way to reach or stop it - it only
+knows about the node's own worker_thread, not the window's separate
+pycoder_agent_thread attribute. When the analysis result (or an error) later
+arrives, _handle_pycoder_analysis_result/_handle_pycoder_error used to mutate,
+reselect, and save-chat a node no longer on the canvas. The AI-driven mode's
+single-stage result handler (_handle_ai_pycoder_result) had the identical gap.
+
+All four handlers now guard on is_disposed, matching the established
+sibling pattern exactly.
+"""
+
+import sys
+from pathlib import Path
+from unittest.mock import MagicMock, patch
+
+sys.path.insert(0, str(Path(__file__).resolve().parents[1]))
+
+from PySide6.QtWidgets import QApplication
+
+_APP = QApplication.instance() or QApplication([])
+
+from graphlink_pycoder import PyCoderNode
+from graphlink_scene import ChatScene
+from graphlink_window_actions import WindowActionsMixin
+
+
+def _window():
+    class _FakeWindow(WindowActionsMixin):
+        pass
+
+    return _FakeWindow()
+
+
+def _disposed_node():
+    node = MagicMock()
+    node.is_disposed = True
+    return node
+
+
+class TestHandleCodeExecutionResultGuardsDisposedNodes:
+    def test_ignores_a_disposed_node_and_never_starts_the_analysis_thread(self):
+        window = _window()
+        node = _disposed_node()
+
+        with patch("graphlink_window_actions.PyCoderAgentWorker") as mock_worker_cls:
+            window._handle_code_execution_result("output", node, [])
+
+        node.set_output.assert_not_called()
+        mock_worker_cls.assert_not_called()
+
+    def test_ignores_none(self):
+        window = _window()
+
+        with patch("graphlink_window_actions.PyCoderAgentWorker") as mock_worker_cls:
+            window._handle_code_execution_result("output", None, [])  # must not raise
+
+        mock_worker_cls.assert_not_called()
+
+
+class TestHandlePycoderAnalysisResultGuardsDisposedNodes:
+    def test_ignores_a_disposed_node(self):
+        window = _window()
+        node = _disposed_node()
+
+        window._handle_pycoder_analysis_result("analysis", node, [], "user msg")
+
+        node.set_ai_analysis.assert_not_called()
+        node.set_running_state.assert_not_called()
+
+    def test_ignores_none(self):
+        window = _window()
+
+        window._handle_pycoder_analysis_result("analysis", None, [], "user msg")  # must not raise
+
+
+class TestHandleAiPycoderResultGuardsDisposedNodes:
+    def test_ignores_a_disposed_node(self):
+        window = _window()
+        node = _disposed_node()
+
+        window._handle_ai_pycoder_result({"analysis": "a", "code": "c", "output": "o"}, node, [])
+
+        node.set_code.assert_not_called()
+        node.set_output.assert_not_called()
+        node.set_ai_analysis.assert_not_called()
+
+    def test_ignores_none(self):
+        window = _window()
+
+        window._handle_ai_pycoder_result({"analysis": "a", "code": "c", "output": "o"}, None, [])  # must not raise
+
+
+class TestHandlePycoderErrorGuardsDisposedNodes:
+    """Shared by all three PyCoder worker stages (manual execution,
+    AI-driven generation, and the manual-mode analysis follow-up)."""
+
+    def test_ignores_a_disposed_node(self):
+        window = _window()
+        node = _disposed_node()
+
+        window._handle_pycoder_error("boom", node)
+
+        node.set_ai_analysis.assert_not_called()
+        node.set_running_state.assert_not_called()
+
+    def test_ignores_none(self):
+        window = _window()
+
+        window._handle_pycoder_error("boom", None)  # must not raise
+
+
+class TestDeleteMidManualExecutionEndToEnd:
+    """The full scenario: node deleted (dispose()'d) while stage 1 (manual
+    execution) is still running. When its finished signal arrives late, the
+    node must be left completely untouched and stage 2 must never start."""
+
+    def test_stage_one_finishing_after_dispose_does_not_touch_the_node_or_start_stage_two(self):
+        window = MagicMock(spec=WindowActionsMixin)
+        # Bind the real methods under test onto the MagicMock window so they
+        # run for real while everything else on window stays a stub.
+        window._handle_code_execution_result = WindowActionsMixin._handle_code_execution_result.__get__(window)
+
+        scene = ChatScene(window=window)
+        parent = scene.add_chat_node("parent", is_user=True)
+        node = PyCoderNode(parent)
+        scene.addItem(node)
+        scene.pycoder_nodes.append(node)
+        node.set_output("original output")
+        node.setSelected(True)
+
+        scene.deleteSelectedItems()
+        assert node.is_disposed is True
+
+        with patch("graphlink_window_actions.PyCoderAgentWorker") as mock_worker_cls:
+            window._handle_code_execution_result("late output", node, [])
+
+        assert node.get_output() == "original output"
+        mock_worker_cls.assert_not_called()