Fix view clearing bug in #45 and add Windows RE operations by kiwids0220 · Pull Request #61 · fosdickio/binary_ninja_mcp

kiwids0220 · 2026-01-17T18:05:20Z

Please test this before merging 😄 I rushed the fix because I needed to test the binja mcp with my other mcp servers. So Please be aware of any changes that may have broke other functionalities.

kiwids0220 · 2026-01-17T18:05:51Z

@CX330Blake The new PR is here, but please review and test before merging 😄

CX330Blake · 2026-01-18T04:37:15Z

@kiwids0220 Could you please briefly explain what feature did you add, so that the review can be much more easier for us? Thanks for the PR! Oh, and please use Ruff to format the codes before committing to pass the linter check.

Copilot

Pull request overview

This PR addresses a view clearing bug (#45) where the current binary view was being constantly reset by UI interaction monitoring, and adds comprehensive Windows reverse engineering operations to the MCP interface.

Changes:

Fixed view clearing bug by removing UI-driven view resets that caused constant "Cleared/Set current binary view" log spam
Added new WindowsREOperations class with 20+ specialized Windows RE analysis functions
Replaced Python 3.10+ union type hints with typing module equivalents for better compatibility
Removed get_stack_frame_vars endpoint (replaced by stackLayout in Windows RE ops)

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 43 comments.

Show a summary per file

File	Description
plugin/init.py	CRITICAL FIX: Removed UI-driven view reset logic causing the bug
plugin/core/binary_operations.py	Fixed _prune_views to preserve current_view; updated type hints
plugin/core/windows_re_operations.py	NEW: Comprehensive Windows RE operations (1153 lines)
plugin/server/http_server.py	Added 15+ Windows RE HTTP endpoints; contains unreachable code bug
bridge/binja_mcp_bridge.py	Added 20+ MCP tools for Windows RE operations
plugin/api/endpoints.py	Removed get_stack_frame_vars; updated type hints
plugin/utils/python_detection.py	Fixed type hints for Optional parameters
scripts/*.py	Formatting/import reordering only
README.md	Updated docs to remove deprecated features; removed development section

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-28T08:19:54Z

+        # Try all formats as fallback
+        try:
+            return json.loads(post_data)
+        except json.JSONDecodeError:
+            try:
+                parsed = dict(urllib.parse.parse_qsl(post_data))
+                if parsed:
+                    return parsed
+            except (ValueError, TypeError):
+                pass
+
+            return {"name": post_data.strip()}
+


Critical bug: Lines 213-224 contain unreachable code. This fallback parsing logic appears after a return statement on line 211, so it will never execute. This code should either be moved before the return statement or removed if it's dead code. This appears to be an accidental duplication or misplaced code block.

Suggested change

# Try all formats as fallback

try:

return json.loads(post_data)

except json.JSONDecodeError:

try:

parsed = dict(urllib.parse.parse_qsl(post_data))

if parsed:

return parsed

except (ValueError, TypeError):

pass

return {"name": post_data.strip()}

Copilot · 2026-01-28T08:19:54Z


    # Numeric forms
    signed = False
+    neg = text.startswith("-")


The variable neg is assigned but never used in the function. If this was intended for some negative number handling logic, it's incomplete. Either implement the logic or remove the unused variable.

Suggested change

neg = text.startswith("-")

Copilot · 2026-01-28T08:19:54Z


-    def get_stack_frame_vars(self, function_identifier: str | int) -> list[dict[str, Any]]:
-        """Get stack frame variable information for a function.
+    # display_as removed per request


The function get_stack_frame_vars has been removed from the endpoints, but the corresponding endpoint handler /getStackFrameVars has also been removed from http_server.py without replacement. However, a new endpoint /stackLayout has been added in the Windows RE operations that appears to serve a similar purpose. Ensure this is intentional and that all callers have been updated to use the new endpoint.

Copilot · 2026-01-28T08:19:55Z

                    if func_length <= 0:
                        func_length = 1024  # Use a reasonable default if length not available
-                except Exception:
+                except:


Using bare except: is generally discouraged as it catches all exceptions including KeyboardInterrupt and SystemExit. Consider catching specific exceptions or at least using except Exception: to allow system exceptions to propagate.

Suggested change

except:

except Exception:

Copilot · 2026-01-28T08:19:55Z



-def create_venv_with_system_python(venv_dir: str, requirements_file: str | None = None) -> str:
+def create_venv_with_system_python(venv_dir: str, requirements_file: str = None) -> str:


The type hint requirements_file: str = None is incorrect. In Python, using None as a default with a non-Optional type hint will cause type checkers to complain. This should be requirements_file: Optional[str] = None to match the actual usage where None is a valid value.

Copilot · 2026-01-28T08:20:02Z

+                                callees.append(node)
+                    except Exception:
+                        continue
+            except Exception:


'except' clause does nothing but pass and there is no explanatory comment.

Copilot · 2026-01-28T08:20:02Z

+            except Exception:
+                pass


'except' clause does nothing but pass and there is no explanatory comment.

Suggested change

except Exception:

pass

except Exception as e:

bn.log_error(f"Error while collecting callees for function {getattr(f, 'name', '<unknown>')}: {e}")

Copilot · 2026-01-28T08:20:02Z

+                                "tag_type": tag.type.name,
+                                "comment": tag.data if hasattr(tag, 'data') else "",
+                            })
+                    except Exception:


'except' clause does nothing but pass and there is no explanatory comment.

Suggested change

except Exception:

except Exception:

# Some Binary Ninja versions/plugins may raise when accessing or iterating

# function_tags; ignore these per-function errors and continue scanning.

Copilot · 2026-01-28T08:20:03Z

+                                    pass
+


'except' clause does nothing but pass and there is no explanatory comment.

Suggested change

pass

# Some HLIL instructions may not support get_possible_values(); ignore and continue.

pass

Copilot · 2026-01-28T08:20:03Z

+                    except Exception:
+                        pass


'except' clause does nothing but pass and there is no explanatory comment.

Suggested change

except Exception:

pass

except Exception as inner_exc:

# MLIL SSA analysis is best-effort; log and continue if it fails

bn.log_debug(f"MLIL SSA analysis failed for function {func.name}: {inner_exc}")

Fix view clearing bug in fosdickio#45 and add Windows RE operations

141b6c4

CX330Blake requested a review from Copilot January 28, 2026 08:13

Copilot started reviewing on behalf of CX330Blake January 28, 2026 08:14 View session

Copilot AI reviewed Jan 28, 2026

View reviewed changes



		def create_venv_with_system_python(venv_dir: str, requirements_file: str \| None = None) -> str:
		def create_venv_with_system_python(venv_dir: str, requirements_file: str = None) -> str:

-            except Exception:
-                pass
+            except Exception as e:
+                bn.log_error(f"Error while collecting callees for function {getattr(f, 'name', '<unknown>')}: {e}")

-                                    pass
+                                    # Some HLIL instructions may not support get_possible_values(); ignore and continue.
+                                    pass

-                    except Exception:
-                        pass
+                    except Exception as inner_exc:
+                        # MLIL SSA analysis is best-effort; log and continue if it fails
+                        bn.log_debug(f"MLIL SSA analysis failed for function {func.name}: {inner_exc}")

Conversation

kiwids0220 commented Jan 17, 2026

Uh oh!

kiwids0220 commented Jan 17, 2026

Uh oh!

CX330Blake commented Jan 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

CX330Blake commented Jan 18, 2026 •

edited

Loading