Skip to content

[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
forge-sdlc:mainfrom
ekuris-redhat:forge/aisos-2392
Open

[AISOS-2392] Forge ignores PR review comments when developer shares the bot's GitHub account #302
ekuris-redhat wants to merge 11 commits into
forge-sdlc:mainfrom
ekuris-redhat:forge/aisos-2392

Conversation

@ekuris-redhat

@ekuris-redhat ekuris-redhat commented Aug 11, 2026

Copy link
Copy Markdown
Collaborator

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 the forge_bot_comment_prefix field of type str (defaulting to "") to the Settings class with Pydantic Field validation.
  • .env.example: Exposed and documented the FORGE_BOT_COMMENT_PREFIX environment 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:
    • Implemented prepend_bot_prefix to safely prepend the bot signature (wrapped inside HTML comments <!-- ... --> if not already wrapped) and guard against double-prepending and None body payloads.
    • Implemented is_self_comment with $O(1)$ complexity. It applies a dual-check rule when a prefix is configured (matching username and body must start with prefix) and falls back to username equality if the prefix is empty. It also ensures GitHub App accounts ending in [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, and reply_to_review_comment) to dynamically sign outgoing comments using the configured signature prefix.

Orchestrator Worker Event Handlers

  • src/forge/orchestrator/worker.py: Integrated is_self_comment filtering 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 a getattr fallback.

Testing Suite

  • Created tests/unit/test_config_bot_signature.py to verify configuration defaults and loading dynamics.
  • Created/updated unit tests in tests/unit/workflow/utils/test_automated_review_triage.py to assert edge-case scenarios (SC-001 through SC-004) for prefix checks, fallback username matching, and bot suffix rules.
  • Created tests/unit/integrations/github/test_outbound_signature.py to verify client-level comment signing behavior.
  • Updated integration and webhook handlers in tests/unit/orchestrator/test_worker.py and tests/unit/api/routes/test_github_webhook.py to verify loop prevention rules.
  • Created tests/unit/orchestrator/test_worker_prd_pr.py to cover the new signature-checking worker handlers.

Implementation Notes

  • Robust Null-Safety: Both prepend_bot_prefix and is_self_comment coerce falsy or None payloads to empty strings to avoid runtime AttributeError exceptions when parsing raw webhook events.
  • Whitespace & Spacer Resilience: The signature lookup parser checks for both space-omitted (<!--prefix-->) and standard spaced (<!-- prefix -->) comment structures to guarantee compatibility regardless of automated formatting.
  • Developer Local Loop Support: When a signature prefix is configured, a comment coming from the bot's username that does not contain the prefix is successfully processed as a human comment. This allows developers to use a single bot PAT for both automated workflows and local manual testing without being blocked by loop prevention.

Testing

  • Unit Testing: Executed extensive unit tests covering the configuration settings, the prepend wrapper functionality, and the core is_self_comment evaluation logic.
  • Integration Testing: Verified that orchestrator worker events successfully parse webhook deliveries, filter out authentic self-comments, and proceed with human developer actions.
  • Linting & Code Quality: Validated that all new/modified files conform cleanly to mypy and ruff formatting 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

Verdict could not be parsed

Fixed #246

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
@ekuris-redhat

Copy link
Copy Markdown
Collaborator Author

🛠️ Forge PR Commands

This pull request was created by Forge! You can use the following commands by commenting on this PR:

  • /forge rebase - Merge the base branch (e.g. main) into this PR branch, with conflicts resolved by AI.
  • /forge skip-gate <name> - Skip a named CI check (substring match) for this PR. This setting persists across subsequent pushes.
  • /forge unskip-gate <name> - Remove a previously set CI check skip.

Feel free to use these commands to manage your workflow!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

dev: Forge ignores PR review comments when developer shares the bot's GitHub account

1 participant