From d5c8ddcd82d00ddce708331d540143aa3d3cb40e Mon Sep 17 00:00:00 2001
From: StillKnotKnown <stillknotknown@users.noreply.github.com>
Date: Tue, 10 Feb 2026 14:16:40 +0200
Subject: [PATCH] fix: address follow-up PR review findings

- NEW-001: Add sanitization to GitLab notes in buildIssueContext
  Apply sanitizeText() to note.author.username and note.body before
  writing to TASK.md, consistent with other external data sanitization.

- NEW-003: Add try/finally protection to sys.modules manipulation
  Save original modules and sys.path before modifications, restore in
  finally block to prevent cascading test failures if exceptions occur.

- NEW-004: Remove dead async function definition in test
  Removed agent_fn async function that was immediately overwritten by
  SystemExit(0) side_effect assignment.
---
 .../main/ipc-handlers/gitlab/spec-utils.ts    |  4 +-
 tests/test_cli_build_commands.py              | 47 ++++++++++---------
 2 files changed, 28 insertions(+), 23 deletions(-)

diff --git a/apps/frontend/src/main/ipc-handlers/gitlab/spec-utils.ts b/apps/frontend/src/main/ipc-handlers/gitlab/spec-utils.ts
index 74e2cad1..4408ee73 100644
--- a/apps/frontend/src/main/ipc-handlers/gitlab/spec-utils.ts
+++ b/apps/frontend/src/main/ipc-handlers/gitlab/spec-utils.ts
@@ -249,7 +249,9 @@ export function buildIssueContext(
     lines.push(`## Notes (${notes.length})`);
     lines.push('');
     for (const note of notes) {
-      lines.push(`**${note.author.username}:** ${note.body}`);
+      const safeAuthor = sanitizeText(note.author?.username || 'unknown', 100);
+      const safeBody = sanitizeText(note.body, 20000, true);
+      lines.push(`**${safeAuthor}:** ${safeBody}`);
       lines.push('');
     }
   }
diff --git a/tests/test_cli_build_commands.py b/tests/test_cli_build_commands.py
index 7774fa89..cdcc50b5 100644
--- a/tests/test_cli_build_commands.py
+++ b/tests/test_cli_build_commands.py
@@ -2030,9 +2030,6 @@ class TestHandleBuildInterruptEdgeCases:
         # so we can check the resume instructions
         with patch("cli.build_commands.select_menu", return_value="skip"):
             with patch("agent.run_autonomous_agent") as mock_agent:
-                async def agent_fn(*args, **kwargs):
-                    return (True, "Success")
-                mock_agent.side_effect = agent_fn
                 mock_agent.side_effect = SystemExit(0)
 
                 # Execute - will exit after trying to resume
@@ -2571,27 +2568,33 @@ class TestBuildCommandsModuleImport:
         module_path = build_cmd_module.__file__
         parent_dir = str(Path(module_path).parent.parent)
 
-        # Save original sys.path
+        # Save original state
         original_path = sys.path.copy()
+        original_modules = {k: sys.modules[k] for k in sys.modules.keys() if k.startswith('cli.build_commands')}
 
-        # Remove the parent directory from sys.path to make the condition True
-        while parent_dir in sys.path:
-            sys.path.remove(parent_dir)
+        try:
+            # Remove the parent directory from sys.path to make the condition True
+            while parent_dir in sys.path:
+                sys.path.remove(parent_dir)
 
-        # Remove module and its submodules from sys.modules to force re-import
-        modules_to_remove = [k for k in sys.modules.keys() if k.startswith('cli.build_commands')]
-        for mod_name in modules_to_remove:
-            del sys.modules[mod_name]
+            # Remove module and its submodules from sys.modules to force re-import
+            modules_to_remove = [k for k in sys.modules.keys() if k.startswith('cli.build_commands')]
+            for mod_name in modules_to_remove:
+                del sys.modules[mod_name]
 
-        # Now import it fresh - this should execute line 15 under coverage
-        import importlib.util
-        spec = importlib.util.spec_from_file_location("cli.build_commands", module_path)
-        module = importlib.util.module_from_spec(spec)
-        sys.modules['cli.build_commands'] = module
-        spec.loader.exec_module(module)
+            # Now import it fresh - this should execute line 15 under coverage
+            import importlib.util
+            spec = importlib.util.spec_from_file_location("cli.build_commands", module_path)
+            module = importlib.util.module_from_spec(spec)
+            sys.modules['cli.build_commands'] = module
+            spec.loader.exec_module(module)
 
-        # Verify the module loaded correctly
-        assert hasattr(module, 'handle_build_command')
-
-        # Restore original sys.path
-        sys.path[:] = original_path
+            # Verify the module loaded correctly
+            assert hasattr(module, 'handle_build_command')
+        finally:
+            # Always restore original state, even if an exception occurred
+            sys.path[:] = original_path
+            # Restore saved modules (if they still exist in sys.modules, skip to avoid conflicts)
+            for mod_name, mod_obj in original_modules.items():
+                if mod_name not in sys.modules:
+                    sys.modules[mod_name] = mod_obj