[AISOS-2392] Forge ignores PR review comments when developer shares the bot's GitHub account - #302
Open
ekuris-redhat wants to merge 11 commits into
Open
[AISOS-2392] Forge ignores PR review comments when developer shares the bot's GitHub account #302ekuris-redhat wants to merge 11 commits into
ekuris-redhat wants to merge 11 commits into
Conversation
Detailed description: - Added forge_bot_comment_prefix str setting in Settings class inside src/forge/config.py, defaulting to an empty string. - Documented and exposed FORGE_BOT_COMMENT_PREFIX in .env.example under GitHub Configuration section. - Added comprehensive unit tests in tests/unit/test_config_bot_signature.py to verify defaults and loading from environment variables. Closes: AISOS-2395
Detailed description: - Created the prepend_bot_prefix utility function in src/forge/workflow/utils/automated_review_triage.py to prepend bot signatures/prefixes to comment bodies. - Handled automatic wrapping of the prefix in HTML comments <!-- ... --> if it's not already wrapped. - Integrated fallback to settings.forge_bot_comment_prefix when no prefix parameter is supplied. - Avoided double-prepending by returning the comment body as-is if it already starts with the wrapped prefix. - Created robust unit tests covering fallback behavior, empty/whitespace prefixes, already-wrapped prefixes, and prefix overrides. Closes: AISOS-2396
Detailed description: - Integrated outbound comment signing prefix in all low-level comment creation and reply methods in the GitHubClient class (`create_review_comment`, `create_issue_comment`, and `reply_to_review_comment`). - Ensured comments are sent unaltered when `forge_bot_comment_prefix` is empty or disabled. - Added comprehensive unit tests in a new test file `tests/unit/integrations/github/test_outbound_signature.py` to assert the signature is correctly prepended or sent unaltered based on configuration settings. Closes: AISOS-2397
Detailed description: - Implemented the `is_self_comment` core helper function in `src/forge/workflow/utils/automated_review_triage.py` to identify comments belonging to the bot using dual-check or legacy username logic. - Supported prefix/signature checks with O(1) startswith complexity. - Enabled immediate self-comment resolution for logins ending in `[bot]`. - Added comprehensive unit tests in `tests/unit/workflow/utils/test_automated_review_triage.py`. Closes: AISOS-2398
…pers and resolve type-check errors Detailed description: - Updated 'is_self_comment' in 'src/forge/workflow/utils/automated_review_triage.py' to check both '<!-- prefix -->' (with space) and '<!--prefix-->' (without space) for prefix matching. - Annotated and typed 'prefixes_to_check' as 'tuple[str, ...]' to resolve mypy type validation errors. - Added corresponding assertions to 'tests/unit/workflow/utils/test_automated_review_triage.py' to verify the new suffix/wrapper logic and prevent regressions. Closes: AISOS-2398
Detailed description: - Added comprehensive unit tests in tests/unit/workflow/utils/test_automated_review_triage.py for is_self_comment logic. - Implemented tests for SC-001 (prefix configured, body starts with prefix vs contains but not at start vs incorrect username with prefix). - Implemented tests for SC-002 (empty/disabled prefix, matching vs different username). - Implemented tests for SC-003 (configured prefix, matching username, body doesn't start with prefix). - Implemented tests for SC-004 (sender username ending in [bot] case-insensitively). - Formatted and linted the test file with Ruff. Closes: AISOS-2399
…andlers Detailed description: - Imported and integrated 'is_self_comment' in 'src/forge/orchestrator/worker.py' across all webhook resume logic endpoints (inline review comment replies, PRD issue comments, spec issue comments, PR review events, and proposal replies) to replace direct username equality checks. - Safely loaded 'forge_bot_comment_prefix' settings using 'getattr' fallback to maintain cross-branch configuration compatibility. - Fixed a bug in 'is_self_comment' in 'src/forge/workflow/utils/automated_review_triage.py' where other non-Forge bots ending in '[bot]' were mistakenly ignored as self-comments. Now properly validates they match the Forge bot name base. - Adjusted tests in 'tests/unit/workflow/utils/test_automated_review_triage.py' to align with the correct identity matching behavior. - Added comprehensive unit tests in 'tests/unit/orchestrator/test_worker_prd_pr.py' to cover the new signature-checking worker handlers. Closes: AISOS-2400
Detailed description: - Added comprehensive unit and integration tests inside tests/unit/orchestrator/test_worker.py to verify dual-check logic rules, legacy fallback, custom PAT account comments, and standard App bot suffix behaviors. - Added tests in tests/unit/api/routes/test_github_webhook.py to verify that standard App bot and custom dev PAT comment webhook deliveries are received, parsed, and successfully queued. - Resolved pre-existing linter warnings (SIM117) inside tests/unit/api/routes/test_github_webhook.py to ensure complete compliance. Closes: AISOS-2401
Detailed description: - Handled potential None types for comment_body in is_self_comment and prepend_bot_prefix to prevent runtime AttributeError crashes during webhook processing. - Normalized comment_body to an empty string when it evaluates to falsy, ensuring robustness against null values. - Updated the type annotations for comment_body to str | None. Closes: AISOS-2392-review
…tting Detailed description: - Added description of FORGE_BOT_COMMENT_PREFIX environment variable to docs/reference/config.md under GitHub section. - This documents the new signature prefix settings for the bot comment signature and webhook self-comment filtering loop prevention logic. Closes: AISOS-2392-docs
Collaborator
Author
🛠️ Forge PR CommandsThis pull request was created by Forge! You can use the following commands by commenting on this PR:
Feel free to use these commands to manage your workflow! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This pull request implements an automated outbound comment signing mechanism and a dual-check webhook self-comment filtering strategy for the Forge bot. By appending or prepending a configurable HTML comment signature to outbound GitHub posts, the system can reliably distinguish its own automated updates from human feedback, preventing infinite webhook loops even when sharing identical credentials or personal access tokens (PATs) in local development environments.
Changes
Configuration & Settings
src/forge/config.py: Added theforge_bot_comment_prefixfield of typestr(defaulting to"") to theSettingsclass with PydanticFieldvalidation..env.example: Exposed and documented theFORGE_BOT_COMMENT_PREFIXenvironment variable under the GitHub configuration section.docs/reference/config.md: Updated documentation to detail the purpose and utilization of the new signature prefix.Core Bot Signature Utilities
src/forge/workflow/utils/automated_review_triage.py:prepend_bot_prefixto safely prepend the bot signature (wrapped inside HTML comments<!-- ... -->if not already wrapped) and guard against double-prepending andNonebody payloads.is_self_commentwith[bot]are correctly validated against the Forge bot login base, preventing non-Forge bots from being mistakenly ignored as self-comments.Outbound Client Integration
src/forge/integrations/github/client.py: Modified low-level comment and reply methods (create_review_comment,create_issue_comment, andreply_to_review_comment) to dynamically sign outgoing comments using the configured signature prefix.Orchestrator Worker Event Handlers
src/forge/orchestrator/worker.py: Integratedis_self_commentfiltering across all event loop endpoints (inline review comment replies, PRD issue comments, spec issue comments, PR review events, and proposal replies) to replace fragile, direct username equality comparisons. Enabled safe loading of settings using agetattrfallback.Testing Suite
tests/unit/test_config_bot_signature.pyto verify configuration defaults and loading dynamics.tests/unit/workflow/utils/test_automated_review_triage.pyto assert edge-case scenarios (SC-001 through SC-004) for prefix checks, fallback username matching, and bot suffix rules.tests/unit/integrations/github/test_outbound_signature.pyto verify client-level comment signing behavior.tests/unit/orchestrator/test_worker.pyandtests/unit/api/routes/test_github_webhook.pyto verify loop prevention rules.tests/unit/orchestrator/test_worker_prd_pr.pyto cover the new signature-checking worker handlers.Implementation Notes
prepend_bot_prefixandis_self_commentcoerce falsy orNonepayloads to empty strings to avoid runtimeAttributeErrorexceptions when parsing raw webhook events.<!--prefix-->) and standard spaced (<!-- prefix -->) comment structures to guarantee compatibility regardless of automated formatting.Testing
is_self_commentevaluation logic.mypyandruffformatting guidelines.Related Tickets
Generated by Forge SDLC Orchestrator
Auto-Review Notes
The following review criteria could not be resolved after all retry attempts.
Human reviewers should pay particular attention to these areas.
implement_task — AISOS-2399
Skill: implement-task | Retries: 2/2 exhausted
Fixed #246