Skip to content

Conversation

@ElmarKenguerli
Copy link

@ElmarKenguerli ElmarKenguerli commented Sep 26, 2025

User description

Description

Resolves: Enable Slack Notifications for Newly Reviewed PRs or Any PR-Agent Response #2025

Summary

This PR implements a provider-agnostic mechanism to emit PR-Agent outputs to external sinks (e.g., Slack) with zero coupling to any specific git provider API. It adds:

  • A generic push_outputs facility with channels (stdout, file, webhook)
  • Disabled-by-default [push_outputs] configuration
  • A small optional Slack relay service (FastAPI) that converts the generic payload to Slack’s expected format (Incoming Webhooks or Workflow “triggers”)
  • A concise README section describing how to enable and use the feature

This meets the requirements in #2025:

  • Agnostic across all supported git providers: no provider-specific APIs are used. Works with GitHub, GitLab, Bitbucket, Azure DevOps, etc.
  • Minimal API calls: zero external calls for stdout/file; a single HTTP POST for webhook; the optional relay makes exactly one POST to Slack.
  • Generic, easy-to-adapt presentation: a structured JSON payload plus a markdown field suitable for human reading and simple remapping.
  • Enable flag off by default: the feature is opt-in and doesn’t change existing behavior unless explicitly configured.

What changed

  • pr_agent/algo/utils.py
    • Added push_outputs(message_type, payload=None, markdown=None)
    • Behavior:
      • If [push_outputs].enable is false (default), does nothing
      • Channels:
        • stdout: prints one-line JSON
        • file: appends JSONL records (creates parent dir if needed)
        • webhook: POSTs JSON record to a configured URL
      • Non-fatal on errors (warn only)
  • pr_agent/tools/pr_reviewer.py
    • After generating the review markdown and before labels, calls push_outputs("review", payload=data['review'], markdown=markdown_text)
    • This is independent of config.publish_output (so you can disable PR comments and still push to Slack)
  • pr_agent/settings/configuration.toml
    • New [push_outputs] section (disabled by default):
      [push_outputs]
      enable = false
      channels = ["stdout"]
      file_path = "pr-agent-outputs/reviews.jsonl"
      webhook_url = ""
      presentation = "markdown"
  • pr_agent/servers/push_outputs_relay.py (optional)
    • FastAPI relay that accepts the generic payload and forwards to Slack:
      • For Incoming Webhooks (hooks.slack.com/services/…), sends {text, mrkdwn}
      • For Workflow Webhooks (hooks.slack.com/triggers/…), sends top-level {markdown, payload} to match Workflow variables
    • SLACK_WEBHOOK_URL is provided via environment variable
    • One POST per message; errors returned to caller with appropriate status code
  • pr_agent/README.md
    • Added a short “Provider-agnostic push outputs and Slack relay” section with quick start steps

Why this design

  • Provider-agnostic: Does not call or depend on any git provider API. Works uniformly across all providers and deployment modes (CLI, actions, webhooks).
  • Minimal API: stdout/file use no network; webhook is exactly one POST; optional relay does one POST to Slack.
  • Generic presentation: the record includes:
    • type: e.g., "review"
    • timestamp: ISO UTC
    • payload: structured review data for automation
    • markdown: human-readable text for notification systems
  • Disabled by default: no behavior change unless explicitly configured.

Configuration examples

  • Emit to Slack via the relay:
    [push_outputs]
    enable = true
    channels = ["webhook"]
    webhook_url = "http://your-relay-host:8000/relay"
    presentation = "markdown"
  • Emit to a local file only:
    [push_outputs]
    enable = true
    channels = ["file"]
    file_path = "pr-agent-outputs/reviews.jsonl"

Optional Slack relay usage

  • Incoming Webhook:
  • Slack Workflow (triggers):
    • Create a Workflow with a Webhook trigger and variables named markdown (and optionally payload)
    • Set SLACK_WEBHOOK_URL to the triggers URL
    • Relay sends top-level {markdown, payload} so the Workflow “Send a message” step can insert the markdown variable

Failure handling

  • push_outputs is non-fatal; if webhook/file fails, the agent run is not interrupted
  • File channel ensures parent directories are created
  • Webhook timeouts/logging use warnings

Backward compatibility

  • Default config keeps the feature disabled; existing behavior is unchanged
  • No changes to provider classes or PR publishing flows
  • The new integration point merely adds an optional emit path

Security and privacy

  • No provider tokens are exposed
  • Relay best practice: keep private or behind an auth gateway
  • Slack webhook lives in the relay environment, not in PR-Agent config
  • No data retention changes; optional file channel writes to local disk where configured

Performance impact

  • Negligible. One JSON dump and optionally a single POST when enabled

Known limitations and future work

  • Slack presentation uses markdown; richer block layouts could be added later if desired
  • Additional channels (e.g., direct “slack” channel without relay) can be added in future if maintainers prefer

Testing performed

  • Local: Verified push_outputs stdout and file channels
  • Webhook error path: Verified non-fatal behavior on connection failure
  • Slack relay:
    • Incoming Webhook path confirmed posts message
    • Workflow “triggers” path confirmed variables markdown/payload accepted and posted as message
  • README instructions followed to reproduce

Notes

  • All local testing artifacts used during development (repo-level overrides, ad-hoc test scripts) were removed before this PR
  • Only production-ready code and docs are included

Request

  • Please review the overall approach and configuration defaults
  • Feedback welcome on config naming and README placement

PR Type

Enhancement


Description

  • Add provider-agnostic push outputs mechanism with stdout/file/webhook channels

  • Integrate Slack relay server for external notifications

  • Enable review data emission without git provider APIs

  • Add configuration section with disabled-by-default settings


Diagram Walkthrough

flowchart LR
  A["PR Review"] --> B["push_outputs()"]
  B --> C["stdout channel"]
  B --> D["file channel"]
  B --> E["webhook channel"]
  E --> F["Slack Relay Server"]
  F --> G["Slack Webhook"]
Loading

File Walkthrough

Relevant files
Enhancement
utils.py
Add generic push outputs mechanism                                             

pr_agent/algo/utils.py

  • Add push_outputs() function supporting stdout, file, and webhook
    channels
  • Include timestamp, payload, and markdown in output records
  • Implement error handling with non-fatal warnings
+57/-0   
push_outputs_relay.py
Add Slack relay server implementation                                       

pr_agent/servers/push_outputs_relay.py

  • Create FastAPI relay server for Slack integration
  • Support both Incoming Webhooks and Workflow triggers
  • Transform generic payload to Slack-compatible format
  • Include comprehensive documentation and usage examples
+106/-0 
pr_reviewer.py
Integrate push outputs into reviewer                                         

pr_agent/tools/pr_reviewer.py

  • Import push_outputs function
  • Call push_outputs() after review generation with payload and markdown
  • Add non-fatal error handling for push operations
+8/-1     
Documentation
README.md
Document push outputs feature                                                       

README.md

  • Add documentation section for push outputs and Slack relay
  • Include setup instructions for relay server
  • Provide configuration examples for .pr_agent.toml
+27/-0   
Configuration changes
configuration.toml
Add push outputs configuration                                                     

pr_agent/settings/configuration.toml

  • Add [push_outputs] configuration section
  • Set default values with feature disabled
  • Include channels, file path, and webhook URL options
+10/-0   

ElmarKeng and others added 4 commits September 26, 2025 14:42
- Implement provider-agnostic push_outputs function supporting stdout, file, and webhook channels
- Add FastAPI-based Slack webhook relay server for external integrations
- Integrate push_outputs into PR reviewer tool to emit review data
- Add configuration section for push_outputs with default disabled state
docs: add provider-agnostic push outputs and Slack relay usage

- Adds `push_outputs` with channels (stdout/file/webhook), disabled by default
- Integrates into review flow; 
- Includes optional Slack relay service
- Updates `README` with usage instructions. No changes to default behavior.
@qodo-merge-for-open-source
Copy link
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis ✅

2025 - PR Code Verified

Compliant requirements:

  • Provide a provider-agnostic solution that works with all supported git providers.
  • Minimize API calls to the git provider (ideally none).
  • Present messages in a generic, easy-to-adapt format.
  • Add an enable flag for the feature, disabled by default.

Requires further human verification:

  • Validate end-to-end Slack delivery using both Incoming Webhooks and Workflow triggers in a real Slack workspace.
  • Confirm documentation clarity by having a user enable and configure the feature successfully.
⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
✅ No TODO sections
🔒 Security concerns

Input validation and SSRF:
The relay posts to a Slack URL from environment, which is fine, but it accepts arbitrary JSON and forwards content. If exposed publicly, it could be abused for message spamming. Recommend authentication/rate-limiting as hinted in comments and possibly size limits on payload.

⚡ Recommended focus areas for review

Missing imports

The new function uses datetime, json, os, requests, and get_logger/get_settings but the diff doesn't show added imports; verify they exist in this module to avoid NameError at runtime.

def push_outputs(message_type: str, payload: dict | None = None, markdown: str | None = None) -> None:
    try:
        cfg = get_settings().get('push_outputs', {}) or {}
        if not cfg.get('enable', False):
            return

        channels = cfg.get('channels', []) or []
        record = {
            "type": message_type,
            "timestamp": datetime.utcnow().isoformat() + "Z",
            "payload": payload or {},
        }
        if markdown is not None:
            record["markdown"] = markdown

        # stdout channel
        if "stdout" in channels:
            try:
                print(json.dumps(record, ensure_ascii=False))
            except Exception:
                # Do not fail the flow if stdout printing fails
                get_logger().warning("Failed to print push_outputs to stdout")

        # file channel (append JSONL)
        if "file" in channels:
            try:
                file_path = cfg.get('file_path', 'pr-agent-outputs/reviews.jsonl')
                folder = os.path.dirname(file_path)
                if folder:
                    os.makedirs(folder, exist_ok=True)
                with open(file_path, 'a', encoding='utf-8') as fh:
                    fh.write(json.dumps(record, ensure_ascii=False) + "\n")
            except Exception as e:
                get_logger().warning(f"Failed to write push_outputs to file: {e}")

        # webhook channel (generic HTTP POST)
        if "webhook" in channels:
            url = cfg.get('webhook_url', '')
            if url:
                try:
                    headers = {'Content-Type': 'application/json'}
                    requests.post(url, data=json.dumps(record), headers=headers, timeout=5)
                except Exception as e:
                    get_logger().warning(f"Failed to POST push_outputs to webhook: {e}")
    except Exception as e:
Webhook error handling

The webhook POST response is not checked for non-2xx status; consider logging status/text when response is >=300 to aid troubleshooting.

    if "webhook" in channels:
        url = cfg.get('webhook_url', '')
        if url:
            try:
                headers = {'Content-Type': 'application/json'}
                requests.post(url, data=json.dumps(record), headers=headers, timeout=5)
            except Exception as e:
                get_logger().warning(f"Failed to POST push_outputs to webhook: {e}")
except Exception as e:
Blocking requests in async

FastAPI async endpoint calls requests.post (blocking). Consider using an async client (httpx) or run in threadpool to avoid blocking the event loop.

@app.post("/relay")
async def relay(record: Dict[str, Any]):
    slack_url = os.environ.get("SLACK_WEBHOOK_URL", "").strip()
    if not slack_url:
        raise HTTPException(status_code=500, detail="SLACK_WEBHOOK_URL environment variable is not set")

    text = _to_slack_text(record)

    # If using a Slack Workflow "triggers" URL, the workflow expects top-level fields
    # that match the configured variables in the Workflow (e.g., "markdown", "payload").
    # Otherwise, for Incoming Webhooks ("services" URL), use the standard {text, mrkdwn}.
    if "hooks.slack.com/triggers/" in slack_url:
        body = {
            # Map our computed text to the workflow variable named "markdown"
            "markdown": text,
            # Provide original payload if the workflow defines a variable for it
            "payload": record.get("payload", {}),
        }
    else:
        body = {
            "text": text,
            "mrkdwn": True,
        }

    try:
        resp = requests.post(slack_url, json=body, timeout=8)
        if resp.status_code >= 300:
            raise HTTPException(status_code=resp.status_code, detail=f"Slack webhook error: {resp.text}")
    except HTTPException:
        raise
    except Exception as e:
        raise HTTPException(status_code=502, detail=f"Failed to post to Slack: {e}")

    return {"status": "ok"}

@qodo-merge-for-open-source
Copy link
Contributor

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Log exceptions instead of ignoring them

In pr_agent/tools/pr_reviewer.py, log exceptions that occur within the
push_outputs call as warnings instead of silently passing, to aid in debugging.

pr_agent/tools/pr_reviewer.py [274-278]

 try:
     push_outputs("review", payload=data.get('review', {}), markdown=markdown_text)
-except Exception:
+except Exception as e:
     # non-fatal
-    pass
+    get_logger().warning(f"Failed to push outputs: {e}")
  • Apply / Chat
Suggestion importance[1-10]: 8

__

Why: The suggestion correctly points out that silently swallowing exceptions is bad practice. Logging the error is crucial for debugging potential issues with the push_outputs feature, making this a significant improvement.

Medium
Load environment variable at startup

In pr_agent/servers/push_outputs_relay.py, load the SLACK_WEBHOOK_URL
environment variable at the module level (on startup) instead of inside the
/relay request handler for better performance and practice.

pr_agent/servers/push_outputs_relay.py [67-69]

-slack_url = os.environ.get("SLACK_WEBHOOK_URL", "").strip()
-if not slack_url:
-    raise HTTPException(status_code=500, detail="SLACK_WEBHOOK_URL environment variable is not set")
+SLACK_WEBHOOK_URL = os.environ.get("SLACK_WEBHOOK_URL", "").strip()
 
+
+@app.post("/relay")
+async def relay(record: Dict[str, Any]):
+    if not SLACK_WEBHOOK_URL:
+        raise HTTPException(status_code=500, detail="SLACK_WEBHOOK_URL environment variable is not set")
+    slack_url = SLACK_WEBHOOK_URL
+
  • Apply / Chat
Suggestion importance[1-10]: 5

__

Why: The suggestion proposes a good practice of loading configuration at startup rather than per-request, which improves performance slightly and follows standard server application patterns.

Low
Possible issue
Check webhook response for errors

In pr_agent/algo/utils.py, check the HTTP response from requests.post and use
raise_for_status() to ensure webhook errors are caught and logged.

pr_agent/algo/utils.py [1316]

-requests.post(url, data=json.dumps(record), headers=headers, timeout=5)
+response = requests.post(url, data=json.dumps(record), headers=headers, timeout=5)
+response.raise_for_status()
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies that HTTP error responses from the webhook are not handled, and proposes using raise_for_status() which will cause these errors to be logged, improving robustness.

Medium
High-level
Consider a serverless deployment for the relay

To reduce operational overhead, it is suggested to document or provide an
example of deploying the new Slack relay as a serverless function, rather than
as a long-running server process.

Examples:

pr_agent/servers/push_outputs_relay.py [101-106]
if __name__ == "__main__":
    # Allow running directly: python -m pr_agent.servers.push_outputs_relay
    import uvicorn

    port = int(os.environ.get("PORT", "8000"))
    uvicorn.run("pr_agent.servers.push_outputs_relay:app", host="0.0.0.0", port=port, reload=False)
README.md [336-341]
1) Start the relay (in a separate shell):
   - Set an Incoming Webhook URL for Slack:
     - CMD:  set SLACK_WEBHOOK_URL=https://hooks.slack.com/services/TXXXX/BXXXX/XXXXXXXX
     - PS:   $env:SLACK_WEBHOOK_URL="https://hooks.slack.com/services/TXXXX/BXXXX/XXXXXXXX"
   - Run:
     uvicorn pr_agent.servers.push_outputs_relay:app --host 0.0.0.0 --port 8000

Solution Walkthrough:

Before:

# README.md - Instructions to run a persistent server
1) Start the relay (in a separate shell):
   ...
   uvicorn pr_agent.servers.push_outputs_relay:app --host 0.0.0.0 --port 8000

# pr_agent/servers/push_outputs_relay.py - Server implementation
app = FastAPI(...)

@app.post("/relay")
async def relay(record: Dict[str, Any]):
    # ... logic to process and forward to Slack

if __name__ == "__main__":
    # Allows running as a standalone server process
    import uvicorn
    uvicorn.run("pr_agent.servers.push_outputs_relay:app", ...)

After:

# Documentation update (e.g., in README.md)
## Serverless Deployment (Example: AWS Lambda)

To deploy the relay as a serverless function, you can use a wrapper like Mangum.

`handler.py`:
```python
from pr_agent.servers.push_outputs_relay import app
from mangum import Mangum

handler = Mangum(app)

Then, configure your serverless framework to deploy this handler, for example via an HTTP API Gateway trigger. This avoids the need to manage a long-running server.





<details><summary>Suggestion importance[1-10]: 6</summary>

__

Why: The suggestion correctly identifies the operational overhead of the new long-running relay service and proposes a valid serverless alternative, which would improve the feature's scalability and maintainability.


</details></details></td><td align=center>Low

</td></tr><tr><td rowspan=1>Learned<br>best practice</td>
<td>



<details><summary>Validate channels configuration</summary>

___

**Normalize and validate <code>channels</code> from settings to ensure it is a list of known <br>strings and default safely if invalid.**

[pr_agent/algo/utils.py [1277-1281]](https://github.com/qodo-ai/pr-agent/pull/2043/files#diff-6b9df72d53c6f0d89fb142c210238a276c0782305e0024d16fbfcaf72c2e2b53R1277-R1281)

```diff
 cfg = get_settings().get('push_outputs', {}) or {}
-if not cfg.get('enable', False):
+enable = bool(cfg.get('enable', False))
+if not enable:
     return
 
-channels = cfg.get('channels', []) or []
+raw_channels = cfg.get('channels', [])
+channels = raw_channels if isinstance(raw_channels, list) else []
+allowed = {"stdout", "file", "webhook"}
+channels = [c for c in channels if isinstance(c, str) and c in allowed]
+if not channels:
+    channels = []
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why:
Relevant best practice - Validate and normalize user-provided configuration values and guard nested access with safe defaults.

Low
  • More
  • Author self-review: I have reviewed the PR code suggestions, and addressed the relevant ones.

@justin8736
Copy link

Preparing answer...

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants