-
Notifications
You must be signed in to change notification settings - Fork 61
fix: sanitize @OpenHands only in GitHub comments (backend) #1020
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: sanitize @OpenHands only in GitHub comments (backend) #1020
Conversation
…nt self-mention loops in comments and reports
|
[Automatic Post]: It has been a while since there was any activity on this PR. @YakshithK, are you still working on it? If so, please go ahead, if not then please request review, close it, or request that someone else follow up. |
|
Make sure that sanitize_openhands_mentions is applied to all agents outputs that is then posted as a comment to github. |
|
I'm on it! simonrosenberg can track my progress at all-hands.dev |
…les.yml - Replace inline sanitize_comment function with import from openhands.sdk.utils.github - Add sanitization to read_report step for scheduled/workflow_dispatch events - Ensures all GitHub comments from agent outputs are properly sanitized Co-authored-by: openhands <[email protected]>
Summary of ChangesI've successfully addressed your request to ensure Changes Made:1. Refactored
Verification:✅ All GitHub comment locations now use sanitization:
✅ No more duplicate definitions: The inline sanitization logic has been removed in favor of importing from the SDK utils module ✅ Pre-commit checks passed: All formatting and linting checks passed Changes Pushed:The commit has been pushed to the View PR: #1020 |
| echo "$COMMENT_BODY" | gh pr comment ${{ github.event.pull_request.number }} --body-file - | ||
| env: | ||
| GH_TOKEN: ${{ github.token }} | ||
| PYTHONPATH: ${{ github.workspace }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it necessary to define PYTHONPATH?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to OpenHands
The workflow runs uv sync --dev which properly installs all workspace packages including openhands-sdk
Once installed via uv sync, the package is available in the Python environment without needing PYTHONPATH
The uv run python command already uses the correct virtual environment with the installed packages
Let me remove the unnecessary PYTHONPATH settings:```
|
@OpenHands Do a /codereview-roasted on this PR. |
|
I'm on it! enyst can track my progress at all-hands.dev |
|
Summary of work: I performed a “/codereview-roasted” review of the PR on branch fix/prevent-self-mentioning-loops. I reviewed the diff vs. origin/main and examined these files:
Key findings and recommendations:
No code changes were made or pushed. |
|
@openhands-ai[bot] please re-login into OpenHands Cloud before starting a job. |
|
[Automatic Post]: It has been a while since there was any activity on this PR. @YakshithK, are you still working on it? If so, please go ahead, if not then please request review, close it, or request that someone else follow up. |
|
@OpenHands please resolve conflicts |
|
I'm on it! simonrosenberg can track my progress at all-hands.dev |
Resolved conflicts in openhands-sdk/openhands/sdk/utils/__init__.py by keeping both: - sanitize_openhands_mentions from the current branch - deprecation utilities from main Co-authored-by: openhands <[email protected]>
SummaryI have successfully resolved the merge conflicts in PR #1020 ( What was done:
The PR is now up-to-date with the main branch and ready for review. The merge brought in 140 changed files from main (including new features like deprecation utilities, critic implementations, and various improvements) while preserving the sanitization functionality from the feature branch. |
c525b7e to
1c8ca72
Compare
…comprehensive tests - Simplify regex pattern to use re.IGNORECASE with capture group for case preservation - Add detailed documentation about ZWJ choice and usage - Normalize ZWJ to lowercase hex escape (\u200d) - Add comprehensive unit test suite covering: - Basic mention sanitization - Case-insensitive matching with case preservation - Multiple mentions - Punctuation handling - Partial word rejection - No-op cases - Line boundaries - Multiline text - URL handling - Whitespace preservation - Improve workflow polish: use printf instead of echo for piping multi-line content Addresses code review feedback on PR OpenHands#1020 Co-authored-by: openhands <[email protected]>
The PYTHONPATH setting is not needed because 'uv sync --dev' properly installs all workspace packages (including openhands-sdk) into the virtual environment. The packages are then accessible via 'uv run python' without requiring PYTHONPATH manipulation. Co-authored-by: openhands <[email protected]>
Summary
This PR implements backend sanitization of @OpenHands mentions in GitHub comments to prevent self-mention loops. Instead of mentioning the OpenHands agent, the sanitization inserts a zero-width joiner after the @ symbol, making the mention non-clickable while maintaining readability.
Changes
sanitize_openhands_mentions()function inopenhands/sdk/utils/github.pythat uses a zero-width joiner (U+200D) to prevent @OpenHands mentions from being parsed as clickable referencesTechnical Details
Related PR
See OpenHands/OpenHands#11550 for the corresponding changes in the main repository.