From 66bd8fdbcdbc3329694614870d1930718cf5baa7 Mon Sep 17 00:00:00 2001 From: openhands Date: Tue, 21 Jan 2025 19:12:09 +0000 Subject: [PATCH 1/6] Enable strict type checking with mypy - Update mypy configuration with stricter type checking rules - Add more type stubs to pre-commit configuration - Run mypy both through pre-commit and directly in CI - Install project in editable mode for better type checking - Set correct PYTHONPATH in CI environment --- .github/workflows/lint.yml | 12 ++++++++++ dev_config/python/.pre-commit-config.yaml | 26 +++++++++++++++++++-- dev_config/python/mypy.ini | 28 +++++++++++++++++++---- 3 files changed, 60 insertions(+), 6 deletions(-) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 789a938e1d7e..170ea3560e27 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -51,8 +51,20 @@ jobs: cache: 'pip' - name: Install pre-commit run: pip install pre-commit==3.7.0 + - name: Install project in editable mode + run: pip install -e . - name: Run pre-commit hooks run: pre-commit run --files openhands/**/* evaluation/**/* tests/**/* --show-diff-on-failure --config ./dev_config/python/.pre-commit-config.yaml + - name: Run mypy directly (for better error reporting) + if: always() # Run even if pre-commit fails + run: | + pip install mypy==1.9.0 \ + types-requests types-setuptools types-pyyaml types-toml \ + types-redis types-protobuf types-python-dateutil types-pyjwt \ + types-docutils types-Pillow types-boto3 types-cryptography \ + types-Jinja2 types-MarkupSafe types-click types-filelock \ + types-decorator types-docopt types-emoji types-enum34 types-paramiko + PYTHONPATH=$PWD mypy --config-file dev_config/python/mypy.ini --scripts-are-modules openhands/ # Check version consistency across documentation check-version-consistency: diff --git a/dev_config/python/.pre-commit-config.yaml b/dev_config/python/.pre-commit-config.yaml index 0ae868fe70fd..25569a132af1 100644 --- a/dev_config/python/.pre-commit-config.yaml +++ b/dev_config/python/.pre-commit-config.yaml @@ -36,8 +36,30 @@ repos: rev: v1.9.0 hooks: - id: mypy + name: mypy (with strict type checking) additional_dependencies: - [types-requests, types-setuptools, types-pyyaml, types-toml] - entry: mypy --config-file dev_config/python/mypy.ini openhands/ + - types-requests + - types-setuptools + - types-pyyaml + - types-toml + - types-redis + - types-protobuf + - types-python-dateutil + - types-pyjwt + - types-docutils + - types-Pillow + - types-boto3 + - types-cryptography + - types-Jinja2 + - types-MarkupSafe + - types-click + - types-filelock + - types-decorator + - types-docopt + - types-emoji + - types-enum34 + - types-paramiko + entry: mypy --config-file dev_config/python/mypy.ini --scripts-are-modules openhands/ always_run: true pass_filenames: false + verbose: true diff --git a/dev_config/python/mypy.ini b/dev_config/python/mypy.ini index 84b97d720b2f..72081060a20d 100644 --- a/dev_config/python/mypy.ini +++ b/dev_config/python/mypy.ini @@ -1,9 +1,29 @@ [mypy] -warn_unused_configs = True +# Error output +show_error_codes = True +pretty = True +show_column_numbers = True + +# Import discovery ignore_missing_imports = True +follow_imports = normal + +# Untyped definitions and calls +disallow_untyped_defs = True check_untyped_defs = True -explicit_package_bases = True -warn_unreachable = True -warn_redundant_casts = True +disallow_incomplete_defs = True +disallow_untyped_decorators = True + +# None and Optional handling no_implicit_optional = True strict_optional = True + +# Warnings +warn_unused_configs = True +warn_redundant_casts = True +warn_unused_ignores = True +warn_return_any = True +warn_unreachable = True + +# Misc +explicit_package_bases = True From 7a259915c1748b1b9e999f5f02450658e95e4c7e Mon Sep 17 00:00:00 2001 From: Graham Neubig Date: Tue, 21 Jan 2025 14:47:51 -0500 Subject: [PATCH 2/6] Update .github/workflows/lint.yml --- .github/workflows/lint.yml | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 170ea3560e27..db7ae1f9c90f 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -55,16 +55,6 @@ jobs: run: pip install -e . - name: Run pre-commit hooks run: pre-commit run --files openhands/**/* evaluation/**/* tests/**/* --show-diff-on-failure --config ./dev_config/python/.pre-commit-config.yaml - - name: Run mypy directly (for better error reporting) - if: always() # Run even if pre-commit fails - run: | - pip install mypy==1.9.0 \ - types-requests types-setuptools types-pyyaml types-toml \ - types-redis types-protobuf types-python-dateutil types-pyjwt \ - types-docutils types-Pillow types-boto3 types-cryptography \ - types-Jinja2 types-MarkupSafe types-click types-filelock \ - types-decorator types-docopt types-emoji types-enum34 types-paramiko - PYTHONPATH=$PWD mypy --config-file dev_config/python/mypy.ini --scripts-are-modules openhands/ # Check version consistency across documentation check-version-consistency: From 64ebef3646487a92ca2acd639a262620f9c67ac5 Mon Sep 17 00:00:00 2001 From: Graham Neubig Date: Tue, 21 Jan 2025 14:52:56 -0500 Subject: [PATCH 3/6] Update .github/workflows/lint.yml --- .github/workflows/lint.yml | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index db7ae1f9c90f..170ea3560e27 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -55,6 +55,16 @@ jobs: run: pip install -e . - name: Run pre-commit hooks run: pre-commit run --files openhands/**/* evaluation/**/* tests/**/* --show-diff-on-failure --config ./dev_config/python/.pre-commit-config.yaml + - name: Run mypy directly (for better error reporting) + if: always() # Run even if pre-commit fails + run: | + pip install mypy==1.9.0 \ + types-requests types-setuptools types-pyyaml types-toml \ + types-redis types-protobuf types-python-dateutil types-pyjwt \ + types-docutils types-Pillow types-boto3 types-cryptography \ + types-Jinja2 types-MarkupSafe types-click types-filelock \ + types-decorator types-docopt types-emoji types-enum34 types-paramiko + PYTHONPATH=$PWD mypy --config-file dev_config/python/mypy.ini --scripts-are-modules openhands/ # Check version consistency across documentation check-version-consistency: From 9db253110f848932fa22098293271b14e6f74878 Mon Sep 17 00:00:00 2001 From: openhands Date: Mon, 24 Feb 2025 01:20:39 +0000 Subject: [PATCH 4/6] Fix mypy errors in security/invariant directory - Fix default_factory type in Event class - Add missing type annotations to __rich_repr__ method - Add missing type annotations to _Policy and _Monitor classes --- openhands/security/invariant/client.py | 8 ++++---- openhands/security/invariant/nodes.py | 5 +++-- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/openhands/security/invariant/client.py b/openhands/security/invariant/client.py index c41828745658..f2ccc78bd61f 100644 --- a/openhands/security/invariant/client.py +++ b/openhands/security/invariant/client.py @@ -50,7 +50,7 @@ def close_session(self) -> Union[None, Exception]: return None class _Policy: - def __init__(self, invariant): + def __init__(self, invariant: 'InvariantClient') -> None: self.server = invariant.server self.session_id = invariant.session_id @@ -77,7 +77,7 @@ def get_template(self) -> tuple[str | None, Exception | None]: except (ConnectionError, Timeout, HTTPError) as err: return None, err - def from_string(self, rule: str): + def from_string(self, rule: str) -> 'InvariantClient._Policy': policy_id, err = self._create_policy(rule) if err: raise err @@ -97,7 +97,7 @@ def analyze(self, trace: list[dict]) -> Union[Any, Exception]: return None, err class _Monitor: - def __init__(self, invariant): + def __init__(self, invariant: 'InvariantClient') -> None: self.server = invariant.server self.session_id = invariant.session_id self.policy = '' @@ -114,7 +114,7 @@ def _create_monitor(self, rule: str) -> tuple[str | None, Exception | None]: except (ConnectionError, Timeout, HTTPError) as err: return None, err - def from_string(self, rule: str): + def from_string(self, rule: str) -> 'InvariantClient._Monitor': monitor_id, err = self._create_monitor(rule) if err: raise err diff --git a/openhands/security/invariant/nodes.py b/openhands/security/invariant/nodes.py index 47410264743b..c3d7b9713bea 100644 --- a/openhands/security/invariant/nodes.py +++ b/openhands/security/invariant/nodes.py @@ -1,3 +1,4 @@ +from typing import Any, Iterable, Tuple from pydantic import BaseModel, Field from pydantic.dataclasses import dataclass @@ -10,7 +11,7 @@ class LLM: class Event(BaseModel): metadata: dict | None = Field( - default_factory=dict, description='Metadata associated with the event' + default_factory=lambda: dict(), description='Metadata associated with the event' ) @@ -30,7 +31,7 @@ class Message(Event): content: str | None tool_calls: list[ToolCall] | None = None - def __rich_repr__(self): + def __rich_repr__(self) -> Iterable[Any | tuple[Any] | tuple[str, Any] | tuple[str, Any, Any]]: # Print on separate line yield 'role', self.role yield 'content', self.content From 18290a102a2b3c1c1fdadd1cfea01ddf88428504 Mon Sep 17 00:00:00 2001 From: openhands Date: Mon, 24 Feb 2025 01:22:14 +0000 Subject: [PATCH 5/6] Revert changes outside of security/invariant directory --- .github/workflows/lint.yml | 12 ---------- dev_config/python/.pre-commit-config.yaml | 26 ++------------------- dev_config/python/mypy.ini | 28 ++++------------------- 3 files changed, 6 insertions(+), 60 deletions(-) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 170ea3560e27..789a938e1d7e 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -51,20 +51,8 @@ jobs: cache: 'pip' - name: Install pre-commit run: pip install pre-commit==3.7.0 - - name: Install project in editable mode - run: pip install -e . - name: Run pre-commit hooks run: pre-commit run --files openhands/**/* evaluation/**/* tests/**/* --show-diff-on-failure --config ./dev_config/python/.pre-commit-config.yaml - - name: Run mypy directly (for better error reporting) - if: always() # Run even if pre-commit fails - run: | - pip install mypy==1.9.0 \ - types-requests types-setuptools types-pyyaml types-toml \ - types-redis types-protobuf types-python-dateutil types-pyjwt \ - types-docutils types-Pillow types-boto3 types-cryptography \ - types-Jinja2 types-MarkupSafe types-click types-filelock \ - types-decorator types-docopt types-emoji types-enum34 types-paramiko - PYTHONPATH=$PWD mypy --config-file dev_config/python/mypy.ini --scripts-are-modules openhands/ # Check version consistency across documentation check-version-consistency: diff --git a/dev_config/python/.pre-commit-config.yaml b/dev_config/python/.pre-commit-config.yaml index 25569a132af1..0ae868fe70fd 100644 --- a/dev_config/python/.pre-commit-config.yaml +++ b/dev_config/python/.pre-commit-config.yaml @@ -36,30 +36,8 @@ repos: rev: v1.9.0 hooks: - id: mypy - name: mypy (with strict type checking) additional_dependencies: - - types-requests - - types-setuptools - - types-pyyaml - - types-toml - - types-redis - - types-protobuf - - types-python-dateutil - - types-pyjwt - - types-docutils - - types-Pillow - - types-boto3 - - types-cryptography - - types-Jinja2 - - types-MarkupSafe - - types-click - - types-filelock - - types-decorator - - types-docopt - - types-emoji - - types-enum34 - - types-paramiko - entry: mypy --config-file dev_config/python/mypy.ini --scripts-are-modules openhands/ + [types-requests, types-setuptools, types-pyyaml, types-toml] + entry: mypy --config-file dev_config/python/mypy.ini openhands/ always_run: true pass_filenames: false - verbose: true diff --git a/dev_config/python/mypy.ini b/dev_config/python/mypy.ini index 72081060a20d..84b97d720b2f 100644 --- a/dev_config/python/mypy.ini +++ b/dev_config/python/mypy.ini @@ -1,29 +1,9 @@ [mypy] -# Error output -show_error_codes = True -pretty = True -show_column_numbers = True - -# Import discovery +warn_unused_configs = True ignore_missing_imports = True -follow_imports = normal - -# Untyped definitions and calls -disallow_untyped_defs = True check_untyped_defs = True -disallow_incomplete_defs = True -disallow_untyped_decorators = True - -# None and Optional handling +explicit_package_bases = True +warn_unreachable = True +warn_redundant_casts = True no_implicit_optional = True strict_optional = True - -# Warnings -warn_unused_configs = True -warn_redundant_casts = True -warn_unused_ignores = True -warn_return_any = True -warn_unreachable = True - -# Misc -explicit_package_bases = True From b4e1a638dfd065840c886e9edff1d844e972079a Mon Sep 17 00:00:00 2001 From: openhands Date: Mon, 24 Feb 2025 01:26:11 +0000 Subject: [PATCH 6/6] Fix mypy error in analyzer.py Handle case where monitor.check() returns an Exception instead of a tuple --- openhands/security/invariant/analyzer.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/openhands/security/invariant/analyzer.py b/openhands/security/invariant/analyzer.py index f843e9304359..540a9341b822 100644 --- a/openhands/security/invariant/analyzer.py +++ b/openhands/security/invariant/analyzer.py @@ -307,11 +307,17 @@ async def security_risk(self, event: Action) -> ActionSecurityRisk: new_elements = parse_element(self.trace, event) input = [e.model_dump(exclude_none=True) for e in new_elements] # type: ignore [call-overload] self.trace.extend(new_elements) - result, err = self.monitor.check(self.input, input) + check_result = self.monitor.check(self.input, input) self.input.extend(input) risk = ActionSecurityRisk.UNKNOWN - if err: - logger.warning(f'Error checking policy: {err}') + + if isinstance(check_result, tuple): + result, err = check_result + if err: + logger.warning(f'Error checking policy: {err}') + return risk + else: + logger.warning(f'Error checking policy: {check_result}') return risk risk = self.get_risk(result)