diff --git a/.github/workflows/base.yml b/.github/workflows/base.yml index cd6a7d4..db26a73 100644 --- a/.github/workflows/base.yml +++ b/.github/workflows/base.yml @@ -46,4 +46,4 @@ jobs: pip install poetry==1.7.1 poetry install poetry run pip install pyspark==$SPARK_VERSION - poetry run python -m pytest -s tests --ignore=tests/test_bot.py + poetry run python -m pytest -s tests diff --git a/.github/workflows/issue-bot.yml b/.github/workflows/issue-bot.yml index 96a2056..dd20644 100644 --- a/.github/workflows/issue-bot.yml +++ b/.github/workflows/issue-bot.yml @@ -1,9 +1,11 @@ -name: PyDeequ Bot +name: PyDeequ Bot # load-bearing: auto-approve.yml keys on this exact name +# To upgrade the engine, bump the SHA in BOTH `uses:` and `shadow_ref` below +# (GitHub forbids expressions in `uses:`, so they can't share a variable). on: issues: types: [opened, reopened] - pull_request_target: # Runs base branch code with secrets; safe because bot fetches diff via API, never executes PR code. NEVER add ref: to checkout. + pull_request_target: # base-branch checkout only; never add ref: (see SECURITY A1) types: [opened, reopened, synchronize] issue_comment: types: [created] @@ -17,114 +19,41 @@ on: type: boolean default: true -# Serialize per issue/PR to prevent duplicate comments +# Union of the reusable workflow's nested-job permissions; a caller must grant +# these or the call fails at startup (each nested job still narrows its own set). +permissions: + contents: read + id-token: write + pull-requests: write + issues: write + +# inputs.issue_number fallback: workflow_dispatch has no event PR/issue number. concurrency: group: bot-${{ github.event.issue.number || github.event.pull_request.number || inputs.issue_number }} cancel-in-progress: false jobs: - analyze: - runs-on: ubuntu-latest - timeout-minutes: 10 + shadow: + # Avoid self-triggered loops (bot's own comments) and double-reviewing a PR + # from both issue_comment and pull_request_target; dispatch is always manual. if: >- (github.event_name == 'workflow_dispatch') || (github.actor != 'github-actions[bot]' && (github.event.issue.pull_request == null || github.event_name == 'pull_request_target')) - permissions: - contents: read - id-token: write - - steps: - - name: Checkout repository - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 - with: - persist-credentials: false - - - name: Configure AWS credentials - uses: aws-actions/configure-aws-credentials@e3dd6a429d7300a6a4c196c26e071d42e0343502 # v4.0.2 - with: - role-to-assume: ${{ secrets.AWS_ROLE_ARN }} - aws-region: us-east-1 - - - name: Set up Python - uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5.6.0 - with: - python-version: "3.12" - - - name: Install dependencies - run: pip install requests==2.33.1 boto3==1.42.94 - - - name: Run analysis - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - GITHUB_REPOSITORY: ${{ github.repository }} - ISSUE_NUMBER: ${{ github.event.issue.number || github.event.pull_request.number || inputs.issue_number }} - EVENT_TYPE: ${{ github.event_name }} - EVENT_ACTION: ${{ github.event.action }} - EVENT_BEFORE: ${{ github.event.before }} - EVENT_AFTER: ${{ github.event.pull_request.head.sha || github.event.after }} - GITHUB_ACTOR: ${{ github.actor }} - KB_S3_BUCKET: ${{ secrets.KB_S3_BUCKET }} - KB_S3_KEY: ${{ secrets.KB_S3_KEY }} - BEDROCK_MODEL_ID: ${{ secrets.BEDROCK_MODEL_ID }} - GUARDRAIL_ID: ${{ secrets.GUARDRAIL_ID }} - GUARDRAIL_VERSION: ${{ secrets.GUARDRAIL_VERSION }} - SM_ISSUE_CLASSIFY_PROMPT: pydeequ-bot/issue-classify-prompt - SM_ISSUE_RESPOND_PROMPT: pydeequ-bot/issue-respond-prompt - SM_PR_FILE_REVIEW_PROMPT: pydeequ-bot/pr-file-review-prompt - SM_FOLLOWUP_PROMPT: pydeequ-bot/followup-prompt - CODEBASE_SRC_DIR: pydeequ - CODEBASE_FILE_EXT: .py - DRY_RUN: ${{ inputs.dry_run || 'false' }} - ARTIFACT_PATH: ${{ runner.temp }}/bot_result.json - run: python -m issue_bot.main analyze - working-directory: scripts - - - name: Upload artifact - uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 - with: - name: bot-result - path: ${{ runner.temp }}/bot_result.json - retention-days: 30 - - act: - runs-on: ubuntu-latest - timeout-minutes: 1 - needs: analyze - permissions: - contents: read - issues: write - pull-requests: write - - steps: - - name: Checkout repository - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 - with: - persist-credentials: false - - - name: Set up Python - uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5.6.0 - with: - python-version: "3.12" - - - name: Install dependencies - run: pip install requests==2.33.1 boto3==1.42.94 - - - name: Download artifact - uses: actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093 # v4.3.0 - with: - name: bot-result - path: ${{ runner.temp }} - - - name: Execute actions - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - GITHUB_REPOSITORY: ${{ github.repository }} - ISSUE_NUMBER: ${{ github.event.issue.number || github.event.pull_request.number || inputs.issue_number }} - EVENT_TYPE: ${{ github.event_name }} - EVENT_ACTION: ${{ github.event.action }} - SLACK_WEBHOOK_URL: ${{ secrets.SLACK_WEBHOOK_URL }} - DRY_RUN: ${{ inputs.dry_run || 'false' }} - ARTIFACT_PATH: ${{ runner.temp }}/bot_result.json - run: python -m issue_bot.main act - working-directory: scripts + uses: sudsali/shadow/.github/workflows/shadow-review.yml@54ec94e0ca8c90d9b58ff95a2a06b175a115784e + with: + pr_number: ${{ inputs.issue_number }} + dry_run: ${{ inputs.dry_run && 'true' || 'false' }} + shadow_ref: 54ec94e0ca8c90d9b58ff95a2a06b175a115784e + aws_region: us-east-1 + prompt_sm_prefix: pydeequ-bot + secrets: + AWS_ROLE_ARN: ${{ secrets.AWS_ROLE_ARN }} + GUARDRAIL_ID: ${{ secrets.GUARDRAIL_ID }} + GUARDRAIL_VERSION: ${{ secrets.GUARDRAIL_VERSION }} + KB_S3_BUCKET: ${{ secrets.KB_S3_BUCKET }} + KB_S3_KEY: ${{ secrets.KB_S3_KEY }} + BEDROCK_MODEL_ID: ${{ secrets.BEDROCK_MODEL_ID }} + BEDROCK_REPORTER_MODEL_ID: ${{ vars.BEDROCK_REPORTER_MODEL_ID }} + BEDROCK_CRITIC_MODEL_ID: ${{ vars.BEDROCK_CRITIC_MODEL_ID }} + SLACK_WEBHOOK_URL: ${{ secrets.SLACK_WEBHOOK_URL }} diff --git a/.github/workflows/update-kb.yml b/.github/workflows/update-kb.yml index 42a4912..41f7372 100644 --- a/.github/workflows/update-kb.yml +++ b/.github/workflows/update-kb.yml @@ -5,8 +5,6 @@ on: branches: [master] paths-ignore: - '.github/workflows/**' - - 'scripts/issue_bot/**' - - 'tests/test_bot.py' workflow_dispatch: jobs: diff --git a/.shadow.yml b/.shadow.yml new file mode 100644 index 0000000..3f09bbb --- /dev/null +++ b/.shadow.yml @@ -0,0 +1,20 @@ +# Shadow engine config for awslabs/python-deequ. + +codebase: + src_dir: pydeequ + file_ext: .py + test_dir: tests + language: python + +bot: + name: deequ-bot # not pydeequ-bot: must render the marker auto-approve.yml greps for + attribution: "Reviewed by Shadow · github.com/sudsali/shadow" + escalate_label: needs-human + max_replies: 2 + max_runs_per_hour: 20 + +models: + investigator: us.anthropic.claude-opus-4-7 + critic: us.anthropic.claude-opus-4-7 + reporter: us.anthropic.claude-haiku-4-5-20251001-v1:0 + issue: us.anthropic.claude-sonnet-4-6 diff --git a/scripts/issue_bot/.gitignore b/scripts/issue_bot/.gitignore deleted file mode 100644 index c18dd8d..0000000 --- a/scripts/issue_bot/.gitignore +++ /dev/null @@ -1 +0,0 @@ -__pycache__/ diff --git a/scripts/issue_bot/__init__.py b/scripts/issue_bot/__init__.py deleted file mode 100644 index 8b13789..0000000 --- a/scripts/issue_bot/__init__.py +++ /dev/null @@ -1 +0,0 @@ - diff --git a/scripts/issue_bot/bedrock_client.py b/scripts/issue_bot/bedrock_client.py deleted file mode 100644 index 30a0f43..0000000 --- a/scripts/issue_bot/bedrock_client.py +++ /dev/null @@ -1,105 +0,0 @@ -import logging - -import boto3 -from botocore.config import Config as BotoConfig -from botocore.exceptions import ClientError, BotoCoreError - -logger = logging.getLogger("issue_bot") - -_CIRCUIT_BREAKER_THRESHOLD = 3 - - -class BedrockClient: - def __init__(self, cfg): - self._model_id = cfg.bedrock_model_id - self._client = boto3.client( - "bedrock-runtime", - config=BotoConfig( - read_timeout=cfg.bedrock_timeout, - connect_timeout=cfg.bedrock_timeout, - retries={"max_attempts": 3, "mode": "adaptive"}, - ), - ) - self._guardrail_id = cfg.guardrail_id - self._guardrail_version = cfg.guardrail_version - self._failures = 0 - self._circuit_open = False # Resets per-process; GHA runs are ephemeral - - @property - def available(self): - return not self._circuit_open - - def invoke(self, system_prompt, user_prompt, max_tokens=4096, - temperature=0.3, json_schema=None): - """Invoke Bedrock Converse API with guardrail on user message only. - - Follows the GlueML pattern (BedrockModelHelper.java): - - system_prompt: Instructions + trusted context (KB, diffs, codebase). - Passed as plain text SystemContentBlock with cachePoint. The - guardrail does NOT assess system prompts without guardContent. - - user_prompt: Untrusted user input (issue title/body, PR title/body, - comments). When guardrail is configured, wrapped in guardContent - so the guardrail scans it for prompt injection. - """ - if self._circuit_open: - logger.warning("Circuit breaker open, skipping Bedrock call") - return None - try: - if self._guardrail_id: - user_content = [{"guardContent": {"text": {"text": user_prompt}}}] - else: - user_content = [{"text": user_prompt}] - - kwargs = { - "modelId": self._model_id, - "messages": [{"role": "user", "content": user_content}], - "inferenceConfig": {"maxTokens": max_tokens, "temperature": temperature}, - } - - if system_prompt: - kwargs["system"] = [ - {"text": system_prompt}, - {"cachePoint": {"type": "default"}}, - ] - - if json_schema: - kwargs["outputConfig"] = { - "textFormat": { - "type": "json_schema", - "structure": {"jsonSchema": { - "schema": json_schema, - "name": "bot_response", - }}, - } - } - - if self._guardrail_id: - kwargs["guardrailConfig"] = { - "guardrailIdentifier": self._guardrail_id, - "guardrailVersion": self._guardrail_version, - "trace": "enabled", - } - - resp = self._client.converse(**kwargs) - - if resp.get("stopReason") == "guardrail_intervened": - logger.warning("Guardrail intervened: %s", resp.get("trace", "")) - return None - - output = resp.get("output", {}).get("message", {}).get("content", []) - if not output: - raise ValueError("Empty Bedrock response") - - self._failures = 0 - usage = resp.get("usage", {}) - logger.info("Bedrock: input=%s, output=%s, cacheRead=%s, cacheWrite=%s", - usage.get("inputTokens"), usage.get("outputTokens"), - usage.get("cacheReadInputTokens"), usage.get("cacheWriteInputTokens")) - return output[0]["text"].strip() - except (ClientError, BotoCoreError, ValueError, ConnectionError) as e: - self._failures += 1 - logger.error(f"Bedrock failed ({self._failures}/{_CIRCUIT_BREAKER_THRESHOLD}): {e}") - if self._failures >= _CIRCUIT_BREAKER_THRESHOLD: - self._circuit_open = True - logger.error("Circuit breaker OPEN") - return None diff --git a/scripts/issue_bot/config.py b/scripts/issue_bot/config.py deleted file mode 100644 index 5fff510..0000000 --- a/scripts/issue_bot/config.py +++ /dev/null @@ -1,55 +0,0 @@ -import os -import sys -import logging - -logging.basicConfig(level=logging.INFO, format="%(asctime)s %(levelname)s %(message)s") -logger = logging.getLogger("issue_bot") - - -class Config: - def __init__(self): - self.github_token = _require("GITHUB_TOKEN") - self.event_type = _require("EVENT_TYPE") - self.event_action = os.getenv("EVENT_ACTION", "") - self.issue_number = _require("ISSUE_NUMBER") - if not self.issue_number.isdigit(): - logger.error(f"ISSUE_NUMBER must be numeric: {self.issue_number}") - sys.exit(1) - self.repo = _require("GITHUB_REPOSITORY") - self.actor = os.getenv("GITHUB_ACTOR", "") - self.event_before = os.getenv("EVENT_BEFORE", "") - self.event_after = os.getenv("EVENT_AFTER", "") - - self.bedrock_model_id = os.getenv("BEDROCK_MODEL_ID", "us.anthropic.claude-opus-4-6-v1") - - self.kb_s3_bucket = os.getenv("KB_S3_BUCKET", "") - self.kb_s3_key = os.getenv("KB_S3_KEY", "") - - self.slack_webhook_url = os.getenv("SLACK_WEBHOOK_URL", "") - self.guardrail_id = os.getenv("GUARDRAIL_ID", "") - self.guardrail_version = os.getenv("GUARDRAIL_VERSION") or "DRAFT" - - self.dry_run = os.getenv("DRY_RUN", "false").lower() == "true" - self.enable_slack = bool(self.slack_webhook_url) - self.enable_repo_search = os.getenv("ENABLE_REPO_SEARCH", "true").lower() == "true" - - self.upstream_repo = os.getenv("UPSTREAM_REPO", "awslabs/python-deequ") - self.codebase_src_dir = os.getenv("CODEBASE_SRC_DIR", "pydeequ") - self.codebase_file_ext = os.getenv("CODEBASE_FILE_EXT", ".py") - - self.bedrock_timeout = 240 - self.max_context_chars = 800000 - self.max_github_search_results = 8 - self.github_api_timeout = 10 - self.allowed_labels = { - "bug", "enhancement", "question", "documentation", - "help wanted", "python", - } - - -def _require(name): - val = os.getenv(name) - if not val: - logger.error(f"Missing required env var: {name}") - sys.exit(1) - return val diff --git a/scripts/issue_bot/github_client.py b/scripts/issue_bot/github_client.py deleted file mode 100644 index 5c2d046..0000000 --- a/scripts/issue_bot/github_client.py +++ /dev/null @@ -1,279 +0,0 @@ -import logging -import os -import requests - -logger = logging.getLogger("issue_bot") - - -class GitHubClient: - def __init__(self, cfg): - self._token = cfg.github_token - self._repo = cfg.repo - self._timeout = cfg.github_api_timeout - self._dry_run = cfg.dry_run - self._cfg = cfg - self._repo_root = os.getenv("GITHUB_WORKSPACE", os.path.abspath(os.path.join(os.path.dirname(__file__), "..", ".."))) - self._headers = { - "Authorization": f"token {self._token}", - "Accept": "application/vnd.github.v3+json", - } - - def get_issue(self, number): - return self._get(f"/repos/{self._repo}/issues/{number}") - - def get_comments(self, number, max_pages=10): - comments = [] - page = 1 - while page <= max_pages: - batch = self._get(f"/repos/{self._repo}/issues/{number}/comments?per_page=100&page={page}") - if not batch: - break - comments.extend(batch) - if len(batch) < 100: - break - page += 1 - return comments - - def get_pr(self, number): - return self._get(f"/repos/{self._repo}/pulls/{number}") - - def get_pr_diff(self, number): - headers = {**self._headers, "Accept": "application/vnd.github.v3.diff"} - try: - resp = requests.get( - f"https://api.github.com/repos/{self._repo}/pulls/{number}", - headers=headers, timeout=self._timeout, - ) - return resp.text if resp.status_code == 200 else "" - except Exception as e: - logger.error(f"PR diff fetch failed: {e}") - return "" - - def get_compare_diff(self, base_sha, head_sha): - """Fetch the diff between two commits using the Compare API. - Returns the diff text, or empty string on failure (e.g. force-push - where base_sha no longer exists).""" - headers = {**self._headers, "Accept": "application/vnd.github.v3.diff"} - try: - resp = requests.get( - f"https://api.github.com/repos/{self._repo}/compare/{base_sha}...{head_sha}", - headers=headers, timeout=self._timeout, - ) - if resp.status_code == 200: - return resp.text - logger.warning(f"Compare API {base_sha[:7]}...{head_sha[:7]}: {resp.status_code}") - return "" - except Exception as e: - logger.error(f"Compare diff failed: {e}") - return "" - - def get_ci_status(self, sha): - """Check commit statuses and check runs. Returns (passed, summary). - passed: True (all green), False (something failed), None (pending/unknown).""" - status = self._get(f"/repos/{self._repo}/commits/{sha}/status") - if status is None: - return None, "CI status unavailable" - combined_state = status.get("state", "pending") - - check_data = self._get(f"/repos/{self._repo}/commits/{sha}/check-runs") - runs = check_data.get("check_runs", []) if check_data else [] - - def _is_own_check(name): - lower = name.lower() - return "bot" in lower and ("analyze" in lower or "/ act" in lower) - - external_runs = [r for r in runs if not _is_own_check(r.get("name", ""))] - - failed = [] - pending = [] - for r in external_runs: - if r.get("status") != "completed": - pending.append(r["name"]) - elif r.get("conclusion") not in ("success", "neutral", "skipped"): - failed.append(r["name"]) - - if failed: - return False, f"CI failing: {', '.join(failed)}" - if pending: - return None, f"CI pending: {', '.join(pending)}" - if combined_state == "failure": - return False, "CI failing (status checks)" - if combined_state == "pending": - return None, "CI pending (status checks)" - return True, "CI passed" - - def get_pr_files(self, number): - return self._get(f"/repos/{self._repo}/pulls/{number}/files") or [] - - def get_pr_review_comments(self, number, max_pages=10): - comments = [] - page = 1 - while page <= max_pages: - batch = self._get(f"/repos/{self._repo}/pulls/{number}/comments?per_page=100&page={page}") - if not batch: - break - comments.extend(batch) - if len(batch) < 100: - break - page += 1 - return comments - - def get_codebase_map(self): - """List source files (excluding tests) as relative paths.""" - src_dir = self._cfg.codebase_src_dir - file_ext = self._cfg.codebase_file_ext - full_dir = os.path.join(self._repo_root, src_dir) - prefix = self._repo_root.rstrip("/") + "/" - try: - paths = [] - for root, dirs, files in os.walk(full_dir): - dirs[:] = [d for d in dirs if d not in ("examples", "__pycache__", ".git", "tests", "test")] - for f in files: - if f.endswith(file_ext): - full = os.path.join(root, f) - rel = full[len(prefix):] if full.startswith(prefix) else full - paths.append(rel) - return "\n".join(sorted(paths)) - except Exception as e: - logger.error(f"Codebase map failed: {e}") - return "" - - def read_local_file(self, path): - repo_root = os.path.realpath(self._repo_root) - if repo_root == "/": - logger.error("Blocked: repo root is /") - return "" - full_path = os.path.realpath(os.path.join(self._repo_root, path)) - if not (full_path.startswith(repo_root + os.sep) or full_path == repo_root): - logger.error(f"Blocked path traversal: {path}") - return "" - try: - with open(full_path, "r", errors="replace") as f: - return f.read() - except Exception: - return "" - - def get_file_content(self, path, repo=None, ref=None): - target = repo or self._repo - url = f"https://api.github.com/repos/{target}/contents/{path}" - if ref: - url += f"?ref={ref}" - headers = {**self._headers, "Accept": "application/vnd.github.v3.raw"} - try: - resp = requests.get(url, headers=headers, timeout=self._timeout) - return resp.text if resp.status_code == 200 else "" - except Exception as e: - logger.error(f"File fetch failed ({path}): {e}") - return "" - - def post_comment(self, number, body): - if self._dry_run: - logger.info(f"[DRY RUN] Comment on #{number}: {body[:80]}...") - return True - return self._post(f"/repos/{self._repo}/issues/{number}/comments", {"body": body}) - - def post_pr_review(self, number, summary, inline_comments, event="COMMENT"): - if self._dry_run: - logger.info(f"[DRY RUN] PR review on #{number}: {len(inline_comments)} inline comments, event={event}") - return True - - # Get valid diff lines per file from the PR - valid_lines = self._get_valid_diff_lines(number) - - valid_comments = [] - invalid_comments = [] - for ic in inline_comments: - line = ic.get("line") - path = ic.get("file", "") - if line and path in valid_lines and line in valid_lines[path]: - valid_comments.append({"path": path, "body": ic["comment"], "line": line, "side": "RIGHT"}) - else: - invalid_comments.append(ic) - - body = summary - if invalid_comments: - body += "\n\n**Additional feedback:**\n" - for ic in invalid_comments: - line_ref = f":{ic['line']}" if ic.get('line') else "" - body += f"\n`{ic['file']}{line_ref}` — {ic['comment']}\n" - - payload = {"body": body, "event": event} - if valid_comments: - payload["comments"] = valid_comments - - try: - resp = requests.post( - f"https://api.github.com/repos/{self._repo}/pulls/{number}/reviews", - headers=self._headers, json=payload, timeout=self._timeout, - ) - if resp.status_code in (200, 201): - return True - logger.error(f"PR review API failed: {resp.status_code}, falling back to comment") - logger.error(f"Response: {resp.text[:500]}") - except Exception as e: - logger.error(f"PR review API failed: {e}, falling back to comment") - - # Fallback: post as regular comment if review API fails - body = summary - if inline_comments: - body += "\n\n**Inline feedback:**\n" - for ic in inline_comments: - line_ref = f":{ic['line']}" if ic.get('line') else "" - body += f"\n`{ic['file']}{line_ref}` — {ic['comment']}\n" - return self._post(f"/repos/{self._repo}/issues/{number}/comments", {"body": body}) - - def _get_valid_diff_lines(self, number): - """Extract valid right-side line numbers from each file's diff hunks.""" - import re - valid = {} - files = self.get_pr_files(number) - for f in files: - path = f.get("filename", "") - patch = f.get("patch", "") - if not patch: - continue - lines = set() - current_line = None - for line in patch.split("\n"): - hunk = re.match(r'^@@ -\d+(?:,\d+)? \+(\d+)(?:,\d+)? @@', line) - if hunk: - current_line = int(hunk.group(1)) - continue - if current_line is None: - continue - if line.startswith("-"): - continue - if line.startswith("\\"): - continue - lines.add(current_line) - current_line += 1 - valid[path] = lines - return valid - - def add_labels(self, number, labels): - if not labels: - return True - if self._dry_run: - logger.info(f"[DRY RUN] Labels on #{number}: {labels}") - return True - return self._post(f"/repos/{self._repo}/issues/{number}/labels", {"labels": labels}) - - def _get(self, path): - try: - resp = requests.get(f"https://api.github.com{path}", headers=self._headers, timeout=self._timeout) - if resp.status_code == 200: - return resp.json() - logger.error(f"GET {path}: {resp.status_code}") - except Exception as e: - logger.error(f"GET {path}: {e}") - return None - - def _post(self, path, payload): - try: - resp = requests.post(f"https://api.github.com{path}", headers=self._headers, json=payload, timeout=self._timeout) - if resp.status_code in (200, 201): - return True - logger.error(f"POST {path}: {resp.status_code}") - except Exception as e: - logger.error(f"POST {path}: {e}") - return False diff --git a/scripts/issue_bot/knowledge_base.py b/scripts/issue_bot/knowledge_base.py deleted file mode 100644 index 23ae27d..0000000 --- a/scripts/issue_bot/knowledge_base.py +++ /dev/null @@ -1,52 +0,0 @@ -import logging -import boto3 - -logger = logging.getLogger("issue_bot") - - -class KnowledgeBase: - def __init__(self, cfg): - self._bucket = cfg.kb_s3_bucket - self._key = cfg.kb_s3_key - self._max_chars = cfg.max_context_chars - self._content = "" - - def load(self): - if self._bucket and self._key: - try: - resp = boto3.client("s3").get_object(Bucket=self._bucket, Key=self._key) - self._content = resp["Body"].read().decode("utf-8") - logger.info(f"KB loaded from S3: {len(self._content)} chars") - return - except Exception as e: - logger.warning(f"S3 KB failed: {e}") - logger.warning("No KB available") - - def build_context(self, issue_text, repo_snippets=""): - parts = [] - if self._content: - parts.append(self._content) - if repo_snippets: - parts.append(f"## Relevant Source Code\n{repo_snippets}") - context = "\n\n".join(parts) - if len(context) > self._max_chars: - context = self._truncate_by_relevance(context, issue_text) - return context - - def _truncate_by_relevance(self, content, issue_text): - keywords = set(issue_text.lower().split()) - sections = content.split("\n## ") - if len(sections) <= 1: - return content[:self._max_chars] - scored = [] - for i, s in enumerate(sections): - score = sum(1 for w in keywords if w in s.lower()) + max(0, 10 - i) - scored.append((score, s)) - scored.sort(key=lambda x: x[0], reverse=True) - result = "" - for _, s in scored: - chunk = f"\n## {s}" if result else s - if len(result) + len(chunk) > self._max_chars: - break - result += chunk - return result diff --git a/scripts/issue_bot/main.py b/scripts/issue_bot/main.py deleted file mode 100644 index 593eaa8..0000000 --- a/scripts/issue_bot/main.py +++ /dev/null @@ -1,715 +0,0 @@ -""" -PyDeequ Bot — two-phase orchestration. - - analyze: read-only phase, produces JSON artifact - act: write-only phase, reads artifact and posts to GitHub/Slack -""" - -import json -import re -import sys -import os -import datetime -import logging -import uuid - -from .config import Config -from .bedrock_client import BedrockClient -from .github_client import GitHubClient -from .knowledge_base import KnowledgeBase -from .slack_client import SlackClient -from .sanitizer import sanitize -from . import prompts - -logger = logging.getLogger("issue_bot") - -ARTIFACT_PATH = os.getenv("ARTIFACT_PATH", "/tmp/bot_result.json") -_MAX_BOT_REPLIES = 2 - - -def _render(template_str, **kwargs): - """Render a prompt template safely using unique tokens per invocation. - Prevents cross-variable injection (user body containing {context} won't leak KB).""" - token_id = uuid.uuid4().hex - tokens = {} - result = template_str - for key, value in kwargs.items(): - token = f"__TMPL_{token_id}_{key}__" - result = result.replace("{" + key + "}", token) - tokens[token] = str(value) - for token, value in tokens.items(): - result = result.replace(token, value) - return result - - -def _load_schema(name): - """Load a JSON schema file from the schemas directory.""" - path = os.path.join(os.path.dirname(__file__), "schemas", name) - with open(path) as f: - return f.read() - - -ISSUE_RESPONSE_SCHEMA = _load_schema("issue_response.json") -PR_REVIEW_SCHEMA = _load_schema("pr_review_response.json") -FOLLOWUP_SCHEMA = _load_schema("followup_response.json") - - -def analyze(): - cfg = Config() - gh = GitHubClient(cfg) - bedrock = BedrockClient(cfg) - kb = KnowledgeBase(cfg) - kb.load() - - number = cfg.issue_number - is_followup = cfg.event_type == "issue_comment" and cfg.event_action == "created" - - item = None - if cfg.event_type in ("pull_request", "pull_request_target"): - is_pr = True - elif cfg.event_type in ("issues", "issue_comment"): - is_pr = False - else: - # workflow_dispatch or unknown — check via API - item = gh.get_issue(number) - is_pr = bool(item and item.get("pull_request")) - - if item is None: - item = gh.get_pr(number) if is_pr else gh.get_issue(number) - if not item: - _write_artifact({"action": "SKIP", "reason": "fetch_failed"}) - return - - author = item.get("user", {}).get("login", "") - if author.endswith("[bot]"): - _write_artifact({"action": "SKIP", "reason": "author_is_bot"}) - return - - if item.get("state") == "closed" and not is_pr: - _write_artifact({"action": "SKIP", "reason": "issue_closed"}) - return - - title = item.get("title", "") or "" - body = item.get("body", "") or "" - html_url = item.get("html_url", "") - comments_data = gh.get_comments(number) - comments_text = _format_comments(comments_data) - - is_pr_update = is_pr and cfg.event_action == "synchronize" - is_reopened = not is_pr and cfg.event_action == "reopened" - - if is_reopened: - _write_artifact({ - "action": "ESCALATE", "labels": [], "response": "", - "reason": "issue_reopened", "title": title, - "html_url": html_url, "number": number, "is_pr": is_pr, - "prompt_id": "n/a", "model_id": cfg.bedrock_model_id, - }) - return - - if not is_followup and not is_pr_update and any( - c.get("user", {}).get("login") == "github-actions[bot]" for c in comments_data): - _write_artifact({"action": "SKIP", "reason": "already_commented"}) - return - - if is_followup and comments_data: - if comments_data[-1].get("user", {}).get("login") == "github-actions[bot]": - _write_artifact({"action": "SKIP", "reason": "bot_last_comment"}) - return - if _already_replied_to_latest(comments_data): - _write_artifact({"action": "SKIP", "reason": "already_replied_to_comment"}) - return - if _bot_reply_count(comments_data) >= _MAX_BOT_REPLIES: - _write_artifact({ - "action": "ESCALATE", "labels": [], "response": "", - "reason": "max_replies_reached", "title": title, - "html_url": html_url, "number": number, "is_pr": is_pr, - "prompt_id": "n/a", "model_id": cfg.bedrock_model_id, - }) - return - if _user_dissatisfied(comments_data): - _write_artifact({ - "action": "ESCALATE", "labels": [], "response": "", - "reason": "user_dissatisfied", "title": title, - "html_url": html_url, "number": number, "is_pr": is_pr, - "prompt_id": "n/a", "model_id": cfg.bedrock_model_id, - }) - return - - issue_text = f"{title} {body}" - context = kb.build_context(issue_text) - codebase_map = gh.get_codebase_map() if not is_followup else "" - - if is_pr: - tmpl = prompts.get_pr_file_review_prompt() - if not tmpl: - _write_artifact({"action": "ESCALATE", "labels": [], "response": "", - "reason": "prompt_load_failed", "title": title, "html_url": html_url, - "number": number, "is_pr": True, "prompt_id": "n/a", "model_id": cfg.bedrock_model_id}) - return - diff = gh.get_pr_diff(number) - review_comments = gh.get_pr_review_comments(number) - existing_feedback = _format_pr_feedback(comments_data, review_comments) - - # Incremental review: on synchronize, compute what changed since last push - incremental_diff = "" - incremental_files = set() - if is_pr_update and cfg.event_before and cfg.event_after: - incremental_diff = gh.get_compare_diff(cfg.event_before, cfg.event_after) - if incremental_diff: - incremental_files = _extract_diff_files(incremental_diff) - - # Fetch full source files at the SHA the diff is anchored to - head_sha = cfg.event_after or item.get("head", {}).get("sha", "") - pr_files = gh.get_pr_files(number) - full_sources = "" - for pf in pr_files: - fname = pf.get("filename", "") - content = gh.get_file_content(fname, ref=head_sha) if head_sha else gh.get_file_content(fname) - if content: - entry = f"\n### `{fname}`\n```\n{content}\n```\n" - if len(full_sources) + len(entry) > 3_000_000: - full_sources += f"\n### `{fname}` — SKIPPED (context budget)\n" - break - full_sources += entry - - # Build incremental review instructions - incremental_section = "" - if incremental_diff: - incremental_section = ( - "\n\n" - "This is a RE-REVIEW after the author pushed new commits. " - "The below shows ONLY what changed since the last push. " - "You MUST limit your comments to lines/files in the incremental diff. " - "Do NOT re-raise issues on unchanged code — the author already saw prior feedback. " - "Do NOT comment on lines that are not part of the incremental diff. " - "If the incremental diff only fixes issues from prior feedback, respond with zero comments." - "\n\n" - f"\n{incremental_diff}\n\n" - ) - - # System prompt: instructions + all trusted context (not scanned by guardrail) - system_prompt = _render(tmpl, current_date=datetime.date.today().isoformat()) + ( - f"\n\n\n{context}\n\n" - f"\n{codebase_map}\n\n" - f"\n{full_sources}\n\n" - f"\n{diff}\n\n" - f"\n{existing_feedback}\n\n" - f"{incremental_section}" - ) - # User prompt: only user-authored content (scanned by guardrail) - user_prompt = f"\nTitle: {title}\nBody: {body}\n" - raw = bedrock.invoke(system_prompt, user_prompt, - max_tokens=8000, json_schema=PR_REVIEW_SCHEMA) - if raw is None: - _write_artifact({ - "action": "ESCALATE", "reason": "bedrock_unavailable", "title": title, - "html_url": html_url, "number": number, "is_pr": True, - "prompt_id": prompts.prompt_version(tmpl), "model_id": cfg.bedrock_model_id, - }) - return - try: - pr_result = json.loads(raw) - inline_comments = pr_result.get("comments", []) - except json.JSONDecodeError: - inline_comments = _parse_file_review_multi(raw) - - # Hard filter: on incremental review, drop comments on files not in the incremental diff - if incremental_files and inline_comments: - inline_comments = [ - c for c in inline_comments - if c.get("file", "") in incremental_files - ] - - # Hard filter: drop NITs on re-reviews (code-enforced, not prompt-dependent) - if is_pr_update and inline_comments: - inline_comments = [ - c for c in inline_comments - if c.get("severity", "").upper() != "NIT" - ] - - # Format comments: prepend severity, append evidence as context - for c in inline_comments: - severity = c.get("severity", "") - evidence = c.get("evidence", "") - prefix = f"**{severity}**: " if severity else "" - suffix = "\n\n> " + evidence.replace("\n", "\n> ") if evidence else "" - c["comment"] = prefix + c.get("comment", "") + suffix - - # Check CI status to give accurate signal to human reviewers - ci_passed, ci_summary = gh.get_ci_status(head_sha) if head_sha else (None, "") - - if not inline_comments: - if ci_passed is True: - response = "No issues found. CI is passing.\n" - elif ci_passed is False: - response = f"No code issues found, but {ci_summary}." - else: - response = "No issues found.\n" - else: - response = "" - - _write_artifact({ - "action": "RESPOND", - "labels": [], "response": response, - "inline_comments": inline_comments, - "title": title, "html_url": html_url, "number": number, - "is_pr": True, "is_incremental": bool(incremental_diff), - "prompt_id": prompts.prompt_version(tmpl), - "model_id": cfg.bedrock_model_id, - }) - return - - elif is_followup: - tmpl = prompts.get_followup_prompt() - if not tmpl: - _write_artifact({"action": "ESCALATE", "labels": [], "response": "", - "reason": "prompt_load_failed", "title": title, "html_url": html_url, - "number": number, "is_pr": is_pr, "prompt_id": "n/a", "model_id": cfg.bedrock_model_id}) - return - system_prompt = tmpl + f"\n\n\n{context}\n" - user_prompt = f"\nTitle: {title}\nBody: {body}\n\n\n{comments_text}\n" - prompt_id = prompts.prompt_version(tmpl) - else: - tmpl = prompts.get_issue_prompt() - if not tmpl: - _write_artifact({"action": "ESCALATE", "labels": [], "response": "", - "reason": "prompt_load_failed", "title": title, "html_url": html_url, - "number": number, "is_pr": is_pr, "prompt_id": "n/a", "model_id": cfg.bedrock_model_id}) - return - system_prompt = tmpl + ( - f"\n\n\n{context}\n\n" - f"\n{codebase_map}\n" - ) - user_prompt = f"\nTitle: {title}\nBody: {body}\n\n\n{comments_text}\n" - prompt_id = prompts.prompt_version(tmpl) - - schema = FOLLOWUP_SCHEMA if is_followup else ISSUE_RESPONSE_SCHEMA - raw = bedrock.invoke(system_prompt, user_prompt, json_schema=schema) - - if raw is None: - _write_artifact({ - "action": "ESCALATE", "labels": [], "response": "", - "reason": "bedrock_unavailable", "title": title, - "html_url": html_url, "number": number, "is_pr": is_pr, - "prompt_id": prompt_id, "model_id": cfg.bedrock_model_id, - }) - return - - parsed = _parse_response(raw, is_pr) - - if parsed.get("read_files") and cfg.enable_repo_search: - snippets = _read_requested_files(gh, parsed["read_files"], cfg) - if snippets: - respond_tmpl = prompts.get_issue_respond_prompt() - if respond_tmpl: - respond_system = respond_tmpl + ( - f"\n\n\n{context}\n\n" - f"\n{snippets}\n" - ) - respond_user = f"\nTitle: {title}\nBody: {body}\n\n\n{comments_text}\n" - raw2 = bedrock.invoke(respond_system, respond_user, - json_schema=ISSUE_RESPONSE_SCHEMA) - if raw2: - parsed2 = _parse_response(raw2, is_pr) - parsed2["labels"] = parsed2.get("labels") or parsed.get("labels", []) - parsed = parsed2 - - _write_artifact({ - "action": parsed["action"], "labels": parsed.get("labels", []), - "response": parsed.get("response", ""), - "inline_comments": parsed.get("inline_comments", []), - "title": title, "html_url": html_url, "number": number, - "is_pr": is_pr, "prompt_id": prompt_id, "model_id": cfg.bedrock_model_id, - }) - - -def act(): - cfg = Config() - gh = GitHubClient(cfg) - slack = SlackClient(cfg) - - result = _read_artifact() - if not result: - logger.error("No artifact found") - return - - # Validate artifact has required fields - action = result.get("action", "SKIP") - if action not in ("SKIP", "RESPOND", "ESCALATE", "CLOSE"): - logger.error(f"Invalid action in artifact: {action}") - return - - number = result.get("number", cfg.issue_number) - is_pr = result.get("is_pr", False) - title = str(result.get("title", ""))[:200] # Truncate to prevent injection - html_url = result.get("html_url", "") - if html_url and not html_url.startswith("https://github.com/"): - html_url = "" - raw_labels = result.get("labels", []) - if not isinstance(raw_labels, list): - raw_labels = [] - labels = [l for l in raw_labels if isinstance(l, str) and l in cfg.allowed_labels] - response = result.get("response", "") - prompt_id = result.get("prompt_id", "unknown") - model_id = result.get("model_id", "unknown") - - if action == "SKIP": - logger.info(f"Skip #{number}: {result.get('reason')}") - return - - footer = ( - f"\n\n---\n*Generated by AI (model: {model_id}, prompt: {prompt_id}) " - f"— may not be fully accurate. Reply if this doesn't help.*" - ) - - # Pre-process: sanitize response before dispatch - if action == "RESPOND": - safe = sanitize(response) - if safe is None: - action = "ESCALATE" - response = "" - elif not safe and not result.get("inline_comments"): - action = "ESCALATE" - response = "" - else: - response = safe or "" - - if action == "RESPOND": - inline_comments = result.get("inline_comments", []) - # Sanitize inline comment text and keep the sanitized version - sanitized_comments = [] - for ic in inline_comments: - safe_comment = sanitize(ic.get("comment", "")) - if safe_comment is not None: - sanitized_comments.append({**ic, "comment": safe_comment}) - inline_comments = sanitized_comments - if is_pr and inline_comments: - gh.post_pr_review(number, response + footer, inline_comments, event="COMMENT") - elif is_pr and response and not inline_comments: - gh.post_pr_review(number, response + footer, [], event="COMMENT") - elif not response and not inline_comments: - logger.info(f"Skip #{number}: nothing to post after sanitization") - else: - gh.post_comment(number, response + footer) - gh.add_labels(number, labels) - if "bug" in labels: - slack.send_escalation(number, title, html_url, labels) - elif "enhancement" in labels: - slack.send_escalation(number, title, html_url, labels) - logger.info(f"Responded to #{number}") - - elif action == "ESCALATE": - reason = result.get("reason", "") - if reason == "user_dissatisfied": - ack = ( - "I understand my previous response wasn't helpful. " - "I've notified the maintainer team and they will follow up directly." + footer - ) - elif reason == "max_replies_reached": - ack = ( - "I've reached the limit of what I can assist with on this issue. " - "The maintainer team has been notified and will take over." + footer - ) - elif reason == "issue_reopened": - ack = ( - "This issue has been reopened. " - "A maintainer has been notified and will follow up." + footer - ) - else: - if response: - ack = ( - response + "\n\n" - "This has also been flagged for our maintainer team to review." + footer - ) - else: - ack = ( - "Thank you for reporting this.\n\n" - "This has been flagged for review by our maintainer team. " - "We'll get back to you as soon as possible." + footer - ) - gh.post_comment(number, ack) - gh.add_labels(number, labels) - slack.send_escalation(number, title, html_url, labels) - logger.info(f"Escalated #{number}") - - elif action == "CLOSE" and not is_pr: - msg = ( - "This issue may not be related to the PyDeequ library. " - "The maintainer team has been notified and will review." + footer - ) - gh.post_comment(number, msg) - gh.add_labels(number, labels) - slack.send_escalation(number, title, html_url, labels) - logger.info(f"Flagged #{number} as potentially off-topic") - - else: - logger.warning(f"Unhandled action '{action}' for #{number}, escalating") - gh.post_comment(number, "This has been flagged for review by our maintainer team." + footer) - slack.send_escalation(number, title, html_url, labels) - - -def _bot_reply_count(comments): - return sum(1 for c in comments if c.get("user", {}).get("login") == "github-actions[bot]") - - -def _already_replied_to_latest(comments): - """True if the bot already posted after the most recent non-bot comment.""" - last_user_idx = -1 - last_bot_idx = -1 - for i, c in enumerate(comments): - if c.get("user", {}).get("login") == "github-actions[bot]": - last_bot_idx = i - else: - last_user_idx = i - return last_bot_idx > last_user_idx >= 0 - - -_DISSATISFACTION_SIGNALS = [ - "that's wrong", "thats wrong", "that is wrong", - "this is wrong", "this is incorrect", "incorrect answer", - "didn't help", "doesn't help", "not helpful", "unhelpful", - "wrong answer", "bad answer", "not correct", "that's not right", - "still broken", "still not working", "doesn't work", - "please escalate", "need a human", "talk to a human", - "maintainer", "real person", -] - - -def _user_dissatisfied(comments): - bot_has_replied = any(c.get("user", {}).get("login") == "github-actions[bot]" for c in comments) - if not bot_has_replied: - return False - for c in reversed(comments): - login = c.get("user", {}).get("login", "") - if login == "github-actions[bot]": - break - if not login: - continue - body = (c.get("body") or "").lower() - if any(s in body for s in _DISSATISFACTION_SIGNALS): - return True - return False - - -_HEADER_PREFIXES = ("ACTION:", "LABELS:", "READ_FILES:", "SEARCH:", "SEARCH_TERMS:") - - -def _parse_response(raw, is_pr): - # Try structured JSON first (from Bedrock structured output) - try: - parsed = json.loads(raw) - result = { - "action": parsed.get("action", "ESCALATE"), - "labels": parsed.get("labels", []), - "read_files": parsed.get("read_files", []), - "response": parsed.get("response", ""), - "inline_comments": [], - } - if is_pr and result["action"] == "CLOSE": - result["action"] = "ESCALATE" - return result - except (json.JSONDecodeError, TypeError): - pass - - # Fallback: parse free-text format - lines = raw.strip().split("\n") - result = {"action": "ESCALATE", "labels": [], "response": "", "read_files": [], "inline_comments": []} - response_lines = [] - - for line in lines: - upper = line.strip().upper() - if upper.startswith("ACTION:"): - val = line.split(":", 1)[1].strip().upper() - if val in ("RESPOND", "ESCALATE", "CLOSE"): - result["action"] = val - continue - elif upper.startswith("LABELS:"): - raw_labels = line.split(":", 1)[1].strip() - result["labels"] = [l.strip() for l in raw_labels.split(",") if l.strip().lower() not in ("none", "")] - continue - elif upper.startswith("READ_FILES:"): - raw_files = line.split(":", 1)[1].strip() - result["read_files"] = [f.strip() for f in raw_files.split(",") if f.strip().lower() not in ("none", "")] - continue - elif upper.startswith(("SEARCH:", "SEARCH_TERMS:")): - continue - response_lines.append(line) - - full_text = "\n".join(response_lines).strip() - - if is_pr and "INLINE:" in full_text and "FILE:" in full_text: - result["response"], result["inline_comments"] = _parse_pr_review(full_text) - else: - result["response"] = _clean_response(full_text) - - if is_pr and result["action"] == "CLOSE": - result["action"] = "ESCALATE" - return result - - -def _parse_file_review_multi(raw): - """Parse multi-file review output into inline comments.""" - comments = [] - current_file = None - current_line = None - current_comment = [] - - for line in raw.strip().split("\n"): - stripped = line.strip() - upper = stripped.upper() - if upper.startswith("FILE:"): - if current_file and current_line and current_comment: - comments.append({"file": current_file, "line": current_line, "comment": "\n".join(current_comment).strip()}) - current_file = stripped.split(":", 1)[1].strip() - current_line = None - current_comment = [] - elif upper.startswith("LINE:"): - if current_file and current_line and current_comment: - comments.append({"file": current_file, "line": current_line, "comment": "\n".join(current_comment).strip()}) - try: - current_line = int(stripped.split(":", 1)[1].strip()) - current_comment = [] - except ValueError: - current_line = None - elif upper.startswith("COMMENT:"): - current_comment = [stripped.split(":", 1)[1].strip()] - elif current_comment is not None and current_file: - current_comment.append(stripped) - - if current_file and current_line and current_comment: - comments.append({"file": current_file, "line": current_line, "comment": "\n".join(current_comment).strip()}) - - return comments - - - - -def _parse_pr_review(text): - """Split PR review into summary and inline comments.""" - summary_part = "" - inline_comments = [] - - parts = text.split("INLINE:") - summary_part = parts[0].replace("SUMMARY:", "").strip() - - if len(parts) > 1: - inline_text = parts[1].strip() - if inline_text.lower() == "none": - return _clean_response(summary_part), [] - - current = {} - for line in inline_text.split("\n"): - stripped = line.strip() - upper = stripped.upper() - if upper.startswith("FILE:"): - if current.get("file") and current.get("comment"): - inline_comments.append(current) - current = {"file": stripped.split(":", 1)[1].strip()} - elif upper.startswith("LINE:"): - try: - current["line"] = int(stripped.split(":", 1)[1].strip()) - except ValueError: - pass - elif upper.startswith("COMMENT:"): - current["comment"] = stripped.split(":", 1)[1].strip() - elif current.get("comment"): - current["comment"] += "\n" + stripped - - if current.get("file") and current.get("comment"): - inline_comments.append(current) - - return _clean_response(summary_part), inline_comments - - -def _clean_response(text): - """Remove any leaked headers or internal thinking from the response.""" - lines = text.split("\n") - cleaned = [] - for line in lines: - upper = line.strip().upper() - if upper.startswith(_HEADER_PREFIXES): - continue - cleaned.append(line) - result = "\n".join(cleaned).strip() - # Remove leading preamble like "Let me request..." or "I'll analyze..." - while result and result.split("\n")[0].strip().lower().startswith(( - "let me ", "i'll ", "i will ", "i need to ", "first,", "sure,", - "since i don't", "since i do not", - )): - result = "\n".join(result.split("\n")[1:]).strip() - return result - - -def _format_comments(comments): - if not comments: - return "(none)" - return "\n".join( - f"{c.get('user', {}).get('login', '?')}: {c.get('body', '') or ''}" - for c in comments - ) - - -def _format_pr_feedback(issue_comments, review_comments): - parts = [] - for c in issue_comments: - author = c.get("user", {}).get("login", "?") - body = c.get("body", "") or "" - parts.append(f"{author}: {body}") - for c in review_comments: - author = c.get("user", {}).get("login", "?") - path = c.get("path", "") - line = c.get("line") or c.get("original_line") or "?" - body = c.get("body", "") or "" - parts.append(f"{author} on {path}:{line}: {body}") - return "\n".join(parts) if parts else "(no existing feedback)" - - -def _extract_diff_files(diff_text): - """Extract the set of file paths touched in a unified diff.""" - files = set() - for line in diff_text.split("\n"): - m = re.match(r'^diff --git a/.+ b/(.+)$', line) - if m: - files.add(m.group(1)) - return files - - -def _read_requested_files(gh, file_paths, cfg): - snippets = [] - for path in file_paths[:cfg.max_github_search_results]: - if ".." in path or path.startswith("/"): - continue - content = gh.read_local_file(path) - if not content: - content = gh.get_file_content(path, repo=cfg.upstream_repo) - if content: - snippets.append(f"### {path}\n```python\n{content}\n```") - return "\n\n".join(snippets) - - -def _write_artifact(data): - os.makedirs(os.path.dirname(ARTIFACT_PATH) or "/tmp", exist_ok=True) - with open(ARTIFACT_PATH, "w") as f: - json.dump(data, f) - logger.info(f"Artifact: action={data.get('action')}") - - -def _read_artifact(): - try: - with open(ARTIFACT_PATH) as f: - return json.load(f) - except Exception as e: - logger.error(f"Artifact read failed: {e}") - return None - - -def main(): - if len(sys.argv) < 2 or sys.argv[1] not in ("analyze", "act"): - print("Usage: python -m issue_bot.main ") - sys.exit(1) - {"analyze": analyze, "act": act}[sys.argv[1]]() - - -if __name__ == "__main__": - main() diff --git a/scripts/issue_bot/prompts.py b/scripts/issue_bot/prompts.py deleted file mode 100644 index 9fc97ea..0000000 --- a/scripts/issue_bot/prompts.py +++ /dev/null @@ -1,54 +0,0 @@ -import hashlib -import logging -import os - -import boto3 - -logger = logging.getLogger("issue_bot") - -_sm_client = None - - -def _get_sm_client(): - global _sm_client - if _sm_client is None: - _sm_client = boto3.client("secretsmanager") - return _sm_client - - -def _read_from_sm(secret_id): - if not secret_id: - return "" - try: - resp = _get_sm_client().get_secret_value(SecretId=secret_id) - return resp["SecretString"] - except Exception as e: - logger.error("Failed to read prompt from Secrets Manager: %s", type(e).__name__) - return "" - - -def _get_prompt(env_var, sm_env_var): - val = os.getenv(env_var, "") - if val: - return val - return _read_from_sm(os.getenv(sm_env_var, "")) - - -def get_issue_prompt(): - return _get_prompt("ISSUE_CLASSIFY_PROMPT", "SM_ISSUE_CLASSIFY_PROMPT") - - -def get_issue_respond_prompt(): - return _get_prompt("ISSUE_RESPOND_PROMPT", "SM_ISSUE_RESPOND_PROMPT") - - -def get_pr_file_review_prompt(): - return _get_prompt("PR_FILE_REVIEW_PROMPT", "SM_PR_FILE_REVIEW_PROMPT") - - -def get_followup_prompt(): - return _get_prompt("FOLLOWUP_PROMPT", "SM_FOLLOWUP_PROMPT") - - -def prompt_version(template): - return hashlib.sha256(template.encode()).hexdigest()[:8] diff --git a/scripts/issue_bot/sanitizer.py b/scripts/issue_bot/sanitizer.py deleted file mode 100644 index 07fe24f..0000000 --- a/scripts/issue_bot/sanitizer.py +++ /dev/null @@ -1,55 +0,0 @@ -import re -import logging - -logger = logging.getLogger("issue_bot") - -# Primary defense: Bedrock Guardrails. These are the required backup layer. -_SECRET_PATTERNS = [ - re.compile(r"AKIA[0-9A-Z]{16}"), - re.compile(r"ghp_[0-9a-zA-Z]{36}"), - re.compile(r"gho_[0-9a-zA-Z]{36}"), - re.compile(r"ghs_[0-9a-zA-Z]{36}"), - re.compile(r"github_pat_[A-Za-z0-9_]{22,}"), - re.compile(r"xox[bpras]-[A-Za-z0-9\-]+"), - re.compile(r"https?://[^\s]*\.corp\.amazon\.com[^\s]*"), - re.compile(r"https?://[^\s]*\.a2z\.com[^\s]*"), - re.compile(r"https?://[^\s]*\.amazon\.dev[^\s]*"), - re.compile(r"hooks\.slack\.com/services/\S+"), -] - -_INJECTION_MARKERS = [ - "my system prompt is", - "my instructions are", - "here are my internal", - "ignore previous instructions", -] - - -def sanitize(text): - if not text: - return text - for p in _SECRET_PATTERNS: - if p.search(text): - logger.error(f"BLOCKED: secret pattern {p.pattern}") - return None - lower = text.lower() - for m in _INJECTION_MARKERS: - if m in lower: - logger.error(f"BLOCKED: injection marker '{m}'") - return None - text = _fix_accidental_issue_refs(text) - return text - - -def _fix_accidental_issue_refs(text): - """Wrap #N references outside code blocks in backticks to prevent GitHub auto-linking.""" - lines = text.split("\n") - in_code_block = False - fixed = [] - for line in lines: - if line.strip().startswith("```"): - in_code_block = not in_code_block - if not in_code_block: - line = re.sub(r'(?", ">") - label_text = ", ".join(f"`{l}`" for l in labels) if labels else "_none_" - text = ( - f"*PyDeequ Issue #{number}*\n" - f">{safe_title}\n\n" - f"*Labels:* {label_text}\n" - f"*Status:* Bot posted analysis on the issue\n\n" - f"<{url}|View on GitHub>" - ) - self._send({"text": text}) - - def _send(self, payload): - try: - resp = requests.post(self._webhook, json=payload, timeout=10) - if resp.status_code != 200: - logger.error(f"Slack: {resp.status_code}") - except Exception as e: - logger.error(f"Slack failed: {e}") diff --git a/tests/test_bot.py b/tests/test_bot.py deleted file mode 100644 index 90f0f66..0000000 --- a/tests/test_bot.py +++ /dev/null @@ -1,1110 +0,0 @@ -# -*- coding: utf-8 -*- -"""Unit tests for the issue bot parsing and validation functions.""" -import json -import sys -import os - -import pytest - -# Add scripts dir to path so we can import issue_bot -sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..", "scripts")) - -from issue_bot.main import ( - _parse_response, - _parse_file_review_multi, - _already_replied_to_latest, - _bot_reply_count, - _user_dissatisfied, - _clean_response, - _render, - _extract_diff_files, -) -from issue_bot.sanitizer import sanitize, _fix_accidental_issue_refs - - -class TestParseResponse: - @pytest.mark.parametrize("action", ["RESPOND", "ESCALATE", "CLOSE"]) - def test_json_actions(self, action): - raw = json.dumps({"action": action, "labels": [], "read_files": [], "response": "text"}) - assert _parse_response(raw, is_pr=False)["action"] == action - - def test_json_with_labels_and_files(self): - raw = json.dumps({"action": "RESPOND", "labels": ["bug", "question"], - "read_files": ["pydeequ/checks.py"], "response": ""}) - result = _parse_response(raw, is_pr=False) - assert result["labels"] == ["bug", "question"] - assert result["read_files"] == ["pydeequ/checks.py"] - - def test_close_on_pr_becomes_escalate(self): - raw = json.dumps({"action": "CLOSE", "labels": [], "read_files": [], "response": ""}) - assert _parse_response(raw, is_pr=True)["action"] == "ESCALATE" - - def test_fallback_to_text_parsing(self): - raw = "ACTION: RESPOND\nLABELS: bug, question\nREAD_FILES: none\n\nHere is the answer." - result = _parse_response(raw, is_pr=False) - assert result["action"] == "RESPOND" - assert "bug" in result["labels"] - assert "Here is the answer." in result["response"] - - def test_text_defaults_to_escalate(self): - raw = "Some unstructured text without headers" - result = _parse_response(raw, is_pr=False) - assert result["action"] == "ESCALATE" - - def test_empty_json_defaults(self): - raw = json.dumps({}) - result = _parse_response(raw, is_pr=False) - assert result["action"] == "ESCALATE" - assert result["labels"] == [] - - -class TestParseFileReviewMulti: - def test_single_comment(self): - raw = "FILE: src/foo.py\nLINE: 42\nCOMMENT: Missing null check" - comments = _parse_file_review_multi(raw) - assert len(comments) == 1 - assert comments[0] == {"file": "src/foo.py", "line": 42, "comment": "Missing null check"} - - def test_multiple_comments(self): - raw = "FILE: a.py\nLINE: 1\nCOMMENT: issue one\nFILE: b.py\nLINE: 2\nCOMMENT: issue two" - assert len(_parse_file_review_multi(raw)) == 2 - - def test_multiline_comment(self): - raw = "FILE: a.py\nLINE: 10\nCOMMENT: first line\nsecond line" - comments = _parse_file_review_multi(raw) - assert "second line" in comments[0]["comment"] - - def test_invalid_line_number_skipped(self): - raw = "FILE: a.py\nLINE: not_a_number\nCOMMENT: bad" - assert len(_parse_file_review_multi(raw)) == 0 - - def test_empty_input(self): - assert _parse_file_review_multi("") == [] - - -class TestSanitize: - def test_none_passthrough(self): - assert sanitize(None) is None - - def test_empty_passthrough(self): - assert sanitize("") == "" - - def test_clean_text_passes(self): - assert sanitize("Normal response about PyDeequ.") is not None - - @pytest.mark.parametrize("marker", [ - "my system prompt is", - "here are my internal", - "ignore previous instructions", - ]) - def test_blocks_injection_markers(self, marker): - assert sanitize(f"Some text with {marker} embedded") is None - - -class TestFixIssueRefs: - def test_wraps_in_backticks(self): - assert _fix_accidental_issue_refs("see #42") == "see `#42`" - - def test_preserves_code_blocks(self): - text = "```\n#42\n```" - assert _fix_accidental_issue_refs(text) == text - - def test_no_match_on_non_numeric(self): - assert _fix_accidental_issue_refs("#abc") == "#abc" - - def test_multiple_refs(self): - result = _fix_accidental_issue_refs("fixes #1 and #2") - assert "`#1`" in result - assert "`#2`" in result - - -def _make_comment(login, body="text"): - return {"user": {"login": login}, "body": body} - - -class TestBotReplyCount: - def test_zero(self): - assert _bot_reply_count([_make_comment("user1")]) == 0 - - def test_counts_bot_only(self): - comments = [_make_comment("user1"), _make_comment("github-actions[bot]"), - _make_comment("user2"), _make_comment("github-actions[bot]")] - assert _bot_reply_count(comments) == 2 - - -class TestAlreadyRepliedToLatest: - def test_bot_after_user(self): - assert _already_replied_to_latest( - [_make_comment("user1"), _make_comment("github-actions[bot]")]) is True - - def test_user_after_bot(self): - assert _already_replied_to_latest( - [_make_comment("github-actions[bot]"), _make_comment("user1")]) is False - - def test_empty(self): - assert _already_replied_to_latest([]) is False - - -class TestUserDissatisfied: - def test_no_bot_reply_means_not_dissatisfied(self): - assert _user_dissatisfied([_make_comment("user1", "that's wrong")]) is False - - def test_dissatisfied_after_bot(self): - comments = [_make_comment("github-actions[bot]"), _make_comment("user1", "that's wrong")] - assert _user_dissatisfied(comments) is True - - def test_happy_after_bot(self): - comments = [_make_comment("github-actions[bot]"), _make_comment("user1", "thanks!")] - assert _user_dissatisfied(comments) is False - - @pytest.mark.parametrize("signal", [ - "didn't help", "not helpful", "still broken", "please escalate", "need a human", - ]) - def test_various_signals(self, signal): - comments = [_make_comment("github-actions[bot]"), _make_comment("user1", signal)] - assert _user_dissatisfied(comments) is True - - -class TestRender: - def test_basic(self): - assert _render("Hello {name}", name="world") == "Hello world" - - def test_braces_in_value_dont_crash(self): - result = _render("Title: {title}", title="Fix {broken} thing") - assert "{broken}" in result - - def test_missing_var_preserved(self): - result = _render("{present} {missing}", present="yes") - assert "yes" in result - assert "{missing}" in result - - def test_no_cross_variable_injection(self): - """User content containing {context} must NOT leak the actual context value.""" - result = _render("KB: {context}\nBody: {body}", context="SECRET", body="{context}") - assert result == "KB: SECRET\nBody: {context}" - - def test_no_reverse_injection(self): - """Context containing {body} must NOT be replaced by body value.""" - result = _render("KB: {context}\nBody: {body}", context="{body}", body="SECRET") - assert result == "KB: {body}\nBody: SECRET" - - -class TestCleanResponse: - def test_strips_header_lines(self): - text = "ACTION: RESPOND\nLABELS: bug\nActual response here" - assert "Actual response here" in _clean_response(text) - assert "ACTION:" not in _clean_response(text) - - def test_strips_preamble(self): - text = "Let me analyze this issue.\nThe actual answer." - assert _clean_response(text) == "The actual answer." - - def test_preserves_normal_text(self): - text = "This is a normal response." - assert _clean_response(text) == text - - -class TestGetCiStatus: - """Tests for GitHubClient.get_ci_status method.""" - - def _make_client(self): - import unittest.mock as mock - with mock.patch.dict(os.environ, { - "GITHUB_TOKEN": "fake", "GITHUB_REPOSITORY": "awslabs/test", - "ISSUE_NUMBER": "1", "EVENT_TYPE": "issues", "EVENT_ACTION": "opened", - "GITHUB_WORKFLOW": "PyDeequ Bot", - }): - from issue_bot.config import Config - from issue_bot.github_client import GitHubClient - cfg = Config() - client = GitHubClient(cfg) - return client - - def test_all_checks_passed(self): - import unittest.mock as mock - client = self._make_client() - client._get = mock.MagicMock(side_effect=[ - {"state": "success"}, # commit status - {"check_runs": [ - {"name": "Java CI", "status": "completed", "conclusion": "success"}, - {"name": "CodeQL", "status": "completed", "conclusion": "success"}, - ]}, - ]) - passed, summary = client.get_ci_status("abc123") - assert passed is True - assert "passed" in summary.lower() - - def test_check_run_failed(self): - import unittest.mock as mock - client = self._make_client() - client._get = mock.MagicMock(side_effect=[ - {"state": "success"}, - {"check_runs": [ - {"name": "Java CI", "status": "completed", "conclusion": "failure"}, - {"name": "CodeQL", "status": "completed", "conclusion": "success"}, - ]}, - ]) - passed, summary = client.get_ci_status("abc123") - assert passed is False - assert "Java CI" in summary - - def test_check_run_pending(self): - import unittest.mock as mock - client = self._make_client() - client._get = mock.MagicMock(side_effect=[ - {"state": "pending"}, - {"check_runs": [ - {"name": "Java CI", "status": "in_progress", "conclusion": None}, - ]}, - ]) - passed, summary = client.get_ci_status("abc123") - assert passed is None - assert "pending" in summary.lower() - - def test_bot_check_filtered_out(self): - import unittest.mock as mock - client = self._make_client() - client._get = mock.MagicMock(side_effect=[ - {"state": "success"}, - {"check_runs": [ - {"name": "Java CI", "status": "completed", "conclusion": "success"}, - {"name": "PyDeequ Bot / analyze", "status": "completed", "conclusion": "success"}, - {"name": "PyDeequ Bot / act", "status": "completed", "conclusion": "success"}, - ]}, - ]) - passed, _ = client.get_ci_status("abc123") - assert passed is True - - def test_non_bot_check_with_bot_in_name_not_filtered(self): - import unittest.mock as mock - client = self._make_client() - client._get = mock.MagicMock(side_effect=[ - {"state": "success"}, - {"check_runs": [ - {"name": "robot-tests", "status": "completed", "conclusion": "failure"}, - ]}, - ]) - passed, _ = client.get_ci_status("abc123") - assert passed is False - - def test_skipped_and_neutral_count_as_passed(self): - import unittest.mock as mock - client = self._make_client() - client._get = mock.MagicMock(side_effect=[ - {"state": "success"}, - {"check_runs": [ - {"name": "Optional Check", "status": "completed", "conclusion": "skipped"}, - {"name": "Info Check", "status": "completed", "conclusion": "neutral"}, - ]}, - ]) - passed, _ = client.get_ci_status("abc123") - assert passed is True - - def test_api_failure_returns_unknown(self): - import unittest.mock as mock - client = self._make_client() - client._get = mock.MagicMock(return_value=None) - passed, summary = client.get_ci_status("abc123") - assert passed is None - - -class TestAutoApproveSignal: - """Tests that bot posts the correct signal for the auto-approve workflow to act on.""" - - def _make_artifact(self, tmp_path, response, inline_comments=None): - artifact = { - "action": "RESPOND", - "labels": [], - "response": response, - "inline_comments": inline_comments or [], - "title": "Fix", "html_url": "https://github.com/x", - "number": 42, "is_pr": True, "is_incremental": False, - "prompt_id": "abc123", "model_id": "test", - } - path = str(tmp_path / "result.json") - with open(path, "w") as f: - json.dump(artifact, f) - return path - - def test_no_issues_posts_pr_review_with_signal(self, tmp_path, monkeypatch): - """Bot posts 'No issues found' as a PR review — auto-approve.yml looks for this in listReviews.""" - import unittest.mock as mock - path = self._make_artifact(tmp_path, response="No issues found. CI is passing.") - monkeypatch.setenv("GITHUB_TOKEN", "fake") - monkeypatch.setenv("GITHUB_REPOSITORY", "awslabs/test") - monkeypatch.setenv("ISSUE_NUMBER", "42") - monkeypatch.setenv("EVENT_TYPE", "pull_request_target") - monkeypatch.setenv("EVENT_ACTION", "opened") - import issue_bot.main as bot_main - monkeypatch.setattr(bot_main, "ARTIFACT_PATH", path) - - with mock.patch("issue_bot.github_client.GitHubClient.post_pr_review") as mock_review, \ - mock.patch("issue_bot.github_client.GitHubClient.post_comment") as mock_comment, \ - mock.patch("issue_bot.github_client.GitHubClient.add_labels"), \ - mock.patch("issue_bot.slack_client.SlackClient.send_escalation"): - mock_review.return_value = True - bot_main.act() - mock_review.assert_called_once() - mock_comment.assert_not_called() - body = mock_review.call_args[0][1] - assert "No issues found" in body - - def test_with_issues_posts_review_not_comment(self, tmp_path, monkeypatch): - """Bot posts inline review when there are issues — no approve signal.""" - import unittest.mock as mock - path = self._make_artifact(tmp_path, response="", - inline_comments=[{"file": "a.py", "line": 1, "comment": "BUG: issue"}]) - monkeypatch.setenv("GITHUB_TOKEN", "fake") - monkeypatch.setenv("GITHUB_REPOSITORY", "awslabs/test") - monkeypatch.setenv("ISSUE_NUMBER", "42") - monkeypatch.setenv("EVENT_TYPE", "pull_request_target") - monkeypatch.setenv("EVENT_ACTION", "opened") - import issue_bot.main as bot_main - monkeypatch.setattr(bot_main, "ARTIFACT_PATH", path) - - with mock.patch("issue_bot.github_client.GitHubClient.post_pr_review") as mock_review, \ - mock.patch("issue_bot.github_client.GitHubClient.post_comment") as mock_comment, \ - mock.patch("issue_bot.github_client.GitHubClient.add_labels"), \ - mock.patch("issue_bot.slack_client.SlackClient.send_escalation"): - mock_review.return_value = True - bot_main.act() - mock_review.assert_called_once() - mock_comment.assert_not_called() - - -class TestPrompts: - def test_env_var_takes_precedence(self, monkeypatch): - monkeypatch.setenv("PR_FILE_REVIEW_PROMPT", "from env") - monkeypatch.setenv("SM_PR_FILE_REVIEW_PROMPT", "deequ-bot/pr-file-review-prompt") - from issue_bot.prompts import get_pr_file_review_prompt - assert get_pr_file_review_prompt() == "from env" - - def test_empty_env_var_falls_through_to_sm(self, monkeypatch): - import unittest.mock as mock - monkeypatch.setenv("PR_FILE_REVIEW_PROMPT", "") - monkeypatch.setenv("SM_PR_FILE_REVIEW_PROMPT", "deequ-bot/pr-file-review-prompt") - with mock.patch("issue_bot.prompts._read_from_sm", return_value="from sm") as m: - from issue_bot.prompts import get_pr_file_review_prompt - result = get_pr_file_review_prompt() - assert result == "from sm" - m.assert_called_once_with("deequ-bot/pr-file-review-prompt") - - def test_no_sm_env_var_returns_empty(self, monkeypatch): - monkeypatch.setenv("PR_FILE_REVIEW_PROMPT", "") - monkeypatch.setenv("SM_PR_FILE_REVIEW_PROMPT", "") - from issue_bot.prompts import get_pr_file_review_prompt - # No env var, no SM secret name → empty string - assert get_pr_file_review_prompt() == "" - - def test_sm_failure_returns_empty(self, monkeypatch): - import unittest.mock as mock - monkeypatch.setenv("FOLLOWUP_PROMPT", "") - monkeypatch.setenv("SM_FOLLOWUP_PROMPT", "deequ-bot/followup-prompt") - with mock.patch("issue_bot.prompts._get_sm_client") as mock_client: - mock_client.return_value.get_secret_value.side_effect = Exception("timeout") - from issue_bot.prompts import get_followup_prompt - assert get_followup_prompt() == "" - - -class TestSmoke: - def test_main_module_imports(self): - from issue_bot import main - assert hasattr(main, 'analyze') - assert hasattr(main, 'act') - - def test_sanitizer_imports(self): - from issue_bot import sanitizer - assert hasattr(sanitizer, 'sanitize') - - def test_schemas_loadable(self): - from issue_bot.main import ISSUE_RESPONSE_SCHEMA, PR_REVIEW_SCHEMA, FOLLOWUP_SCHEMA - import json - assert json.loads(ISSUE_RESPONSE_SCHEMA)["type"] == "object" - assert json.loads(PR_REVIEW_SCHEMA)["type"] == "object" - assert json.loads(FOLLOWUP_SCHEMA)["type"] == "object" - - -class TestArtifactValidation: - def test_invalid_action_rejected(self): - """Actions not in the allowed set should be treated as invalid.""" - valid = {"SKIP", "RESPOND", "ESCALATE", "CLOSE"} - assert "DROP_TABLE" not in valid - assert "RESPOND" in valid - - def test_title_truncated(self): - title = "A" * 500 - truncated = str(title)[:200] - assert len(truncated) == 200 - - def test_non_github_url_cleared(self): - url = "https://evil.com/steal" - result = "" if not url.startswith("https://github.com/") else url - assert result == "" - - def test_github_url_preserved(self): - url = "https://github.com/awslabs/python-deequ/issues/1" - result = "" if not url.startswith("https://github.com/") else url - assert result == url - - def test_empty_url_preserved(self): - url = "" - result = "" if url and not url.startswith("https://github.com/") else url - assert result == "" - - -class TestSplitPrompt: - """Test that invoke() follows GlueML pattern: system=trusted, user=guarded.""" - - def _make_client(self, guardrail_id=""): - class FakeCfg: - bedrock_model_id = "test" - bedrock_timeout = 10 - guardrail_id = "" - guardrail_version = "DRAFT" - cfg = FakeCfg() - cfg.guardrail_id = guardrail_id - from issue_bot.bedrock_client import BedrockClient - import unittest.mock as mock - with mock.patch("boto3.client"): - client = BedrockClient(cfg) - return client - - def _mock_converse(self, client): - import unittest.mock as mock - client._client = mock.MagicMock() - client._client.converse.return_value = { - "stopReason": "end_turn", - "output": {"message": {"content": [{"text": "ok"}]}}, - "usage": {}, - } - - def test_with_guardrail_user_is_guardcontent(self): - client = self._make_client(guardrail_id="gr-123") - self._mock_converse(client) - client.invoke("system instructions", "user input") - kwargs = client._client.converse.call_args[1] - content = kwargs["messages"][0]["content"] - assert len(content) == 1 - assert "guardContent" in content[0] - assert content[0]["guardContent"]["text"]["text"] == "user input" - - def test_without_guardrail_user_is_text(self): - client = self._make_client() - self._mock_converse(client) - client.invoke("system", "user input") - kwargs = client._client.converse.call_args[1] - content = kwargs["messages"][0]["content"] - assert len(content) == 1 - assert "text" in content[0] - assert content[0]["text"] == "user input" - - def test_system_prompt_is_plain_text_cached(self): - client = self._make_client(guardrail_id="gr-123") - self._mock_converse(client) - client.invoke("instructions + diff with ignore previous instructions", "Title: test") - kwargs = client._client.converse.call_args[1] - system = kwargs["system"] - assert system[0]["text"] == "instructions + diff with ignore previous instructions" - assert "cachePoint" in system[1] - # System prompt is NOT guardContent — guardrail won't scan it - assert "guardContent" not in system[0] - - def test_guardrail_config_present(self): - client = self._make_client(guardrail_id="gr-123") - self._mock_converse(client) - client.invoke("system", "user") - kwargs = client._client.converse.call_args[1] - assert "guardrailConfig" in kwargs - assert kwargs["guardrailConfig"]["guardrailIdentifier"] == "gr-123" - - def test_no_guardrail_no_config(self): - client = self._make_client() - self._mock_converse(client) - client.invoke("system", "user") - kwargs = client._client.converse.call_args[1] - assert "guardrailConfig" not in kwargs - - -class TestExtractDiffFiles: - def test_single_file(self): - diff = ( - "diff --git a/src/foo.py b/src/foo.py\n" - "index abc1234..def5678 100644\n" - "--- a/src/foo.py\n" - "+++ b/src/foo.py\n" - "@@ -1,3 +1,4 @@\n" - "+new line\n" - ) - assert _extract_diff_files(diff) == {"src/foo.py"} - - def test_multiple_files(self): - diff = ( - "diff --git a/a.py b/a.py\n" - "--- a/a.py\n" - "+++ b/a.py\n" - "@@ -1 +1 @@\n" - "-old\n" - "+new\n" - "diff --git a/b.py b/b.py\n" - "--- a/b.py\n" - "+++ b/b.py\n" - "@@ -1 +1 @@\n" - "-old\n" - "+new\n" - ) - assert _extract_diff_files(diff) == {"a.py", "b.py"} - - def test_empty_diff(self): - assert _extract_diff_files("") == set() - - def test_renamed_file(self): - diff = "diff --git a/old_name.py b/new_name.py\n" - assert _extract_diff_files(diff) == {"new_name.py"} - - def test_path_with_spaces(self): - diff = "diff --git a/path with spaces/file.py b/path with spaces/file.py\n" - assert _extract_diff_files(diff) == {"path with spaces/file.py"} - - -class TestIncrementalFiltering: - """Test that the incremental file filter drops comments on unrelated files.""" - - def test_comments_filtered_to_incremental_files(self): - incremental_files = {"src/changed.py"} - inline_comments = [ - {"file": "src/changed.py", "line": 10, "comment": "new issue"}, - {"file": "src/untouched.py", "line": 5, "comment": "old issue re-raised"}, - ] - filtered = [c for c in inline_comments if c.get("file", "") in incremental_files] - assert len(filtered) == 1 - assert filtered[0]["file"] == "src/changed.py" - - def test_empty_incremental_files_passes_all(self): - incremental_files = set() - inline_comments = [ - {"file": "src/any.py", "line": 1, "comment": "comment"}, - ] - # When incremental_files is empty (fallback to full review), no filtering - if incremental_files: - filtered = [c for c in inline_comments if c.get("file", "") in incremental_files] - else: - filtered = inline_comments - assert len(filtered) == 1 - - def test_all_comments_filtered_yields_empty(self): - incremental_files = {"src/only_this.py"} - inline_comments = [ - {"file": "src/other.py", "line": 1, "comment": "stale"}, - {"file": "src/another.py", "line": 2, "comment": "stale too"}, - ] - filtered = [c for c in inline_comments if c.get("file", "") in incremental_files] - assert filtered == [] - - -class TestNitFilterAndFormatting: - """Tests for hard NIT filter on re-reviews and evidence formatting.""" - - def test_nits_dropped_on_re_review(self, tmp_path, monkeypatch): - import unittest.mock as mock - monkeypatch.setenv("GITHUB_TOKEN", "fake") - monkeypatch.setenv("GITHUB_REPOSITORY", "awslabs/test") - monkeypatch.setenv("ISSUE_NUMBER", "10") - monkeypatch.setenv("EVENT_TYPE", "pull_request_target") - monkeypatch.setenv("EVENT_ACTION", "synchronize") - monkeypatch.setenv("EVENT_BEFORE", "aaa") - monkeypatch.setenv("EVENT_AFTER", "bbb") - monkeypatch.setenv("KB_S3_BUCKET", "") - monkeypatch.setenv("KB_S3_KEY", "") - monkeypatch.setenv("PR_FILE_REVIEW_PROMPT", "Review. Date: {current_date}") - import issue_bot.main as bot_main - monkeypatch.setattr(bot_main, "ARTIFACT_PATH", str(tmp_path / "result.json")) - - incremental = "diff --git a/f.py b/f.py\n--- a/f.py\n+++ b/f.py\n@@ -1 +1 @@\n-x\n+y\n" - - with mock.patch("issue_bot.github_client.GitHubClient.get_pr") as mock_pr, \ - mock.patch("issue_bot.github_client.GitHubClient.get_comments") as mock_comments, \ - mock.patch("issue_bot.github_client.GitHubClient.get_pr_diff"), \ - mock.patch("issue_bot.github_client.GitHubClient.get_pr_review_comments") as mock_rc, \ - mock.patch("issue_bot.github_client.GitHubClient.get_compare_diff") as mock_compare, \ - mock.patch("issue_bot.github_client.GitHubClient.get_pr_files") as mock_files, \ - mock.patch("issue_bot.github_client.GitHubClient.get_file_content") as mock_content, \ - mock.patch("issue_bot.github_client.GitHubClient.get_codebase_map"), \ - mock.patch("issue_bot.github_client.GitHubClient.get_ci_status") as mock_ci, \ - mock.patch("issue_bot.knowledge_base.KnowledgeBase.load"), \ - mock.patch("issue_bot.knowledge_base.KnowledgeBase.build_context", return_value=""), \ - mock.patch("issue_bot.bedrock_client.BedrockClient.__init__", return_value=None), \ - mock.patch("issue_bot.bedrock_client.BedrockClient.invoke") as mock_bedrock: - - mock_pr.return_value = { - "user": {"login": "dev"}, "title": "Fix", "body": "", - "state": "open", "html_url": "https://github.com/x", - "head": {"sha": "bbb"}, - } - mock_comments.return_value = [{"user": {"login": "github-actions[bot]"}, "body": "prior"}] - mock_rc.return_value = [] - mock_compare.return_value = incremental - mock_files.return_value = [{"filename": "f.py"}] - mock_content.return_value = "content" - mock_ci.return_value = (True, "CI passed") - mock_bedrock.return_value = json.dumps({"comments": [ - {"file": "f.py", "line": 1, "severity": "BUG", "comment": "real bug", - "evidence": "line 1 divides by zero"}, - {"file": "f.py", "line": 1, "severity": "NIT", "comment": "rename var", - "evidence": "x is not descriptive"}, - ]}) - - bot_main.analyze() - - with open(str(tmp_path / "result.json")) as f: - result = json.load(f) - - # NIT should be filtered, BUG should remain - assert result["action"] == "RESPOND" - assert len(result["inline_comments"]) == 1 - assert "real bug" in result["inline_comments"][0]["comment"] - - def test_nits_kept_on_first_review(self, tmp_path, monkeypatch): - import unittest.mock as mock - monkeypatch.setenv("GITHUB_TOKEN", "fake") - monkeypatch.setenv("GITHUB_REPOSITORY", "awslabs/test") - monkeypatch.setenv("ISSUE_NUMBER", "10") - monkeypatch.setenv("EVENT_TYPE", "pull_request_target") - monkeypatch.setenv("EVENT_ACTION", "opened") - monkeypatch.setenv("EVENT_BEFORE", "") - monkeypatch.setenv("EVENT_AFTER", "") - monkeypatch.setenv("KB_S3_BUCKET", "") - monkeypatch.setenv("KB_S3_KEY", "") - monkeypatch.setenv("PR_FILE_REVIEW_PROMPT", "Review. Date: {current_date}") - import issue_bot.main as bot_main - monkeypatch.setattr(bot_main, "ARTIFACT_PATH", str(tmp_path / "result.json")) - - with mock.patch("issue_bot.github_client.GitHubClient.get_pr") as mock_pr, \ - mock.patch("issue_bot.github_client.GitHubClient.get_comments") as mock_comments, \ - mock.patch("issue_bot.github_client.GitHubClient.get_pr_diff"), \ - mock.patch("issue_bot.github_client.GitHubClient.get_pr_review_comments") as mock_rc, \ - mock.patch("issue_bot.github_client.GitHubClient.get_pr_files") as mock_files, \ - mock.patch("issue_bot.github_client.GitHubClient.get_file_content") as mock_content, \ - mock.patch("issue_bot.github_client.GitHubClient.get_codebase_map"), \ - mock.patch("issue_bot.github_client.GitHubClient.get_ci_status") as mock_ci, \ - mock.patch("issue_bot.knowledge_base.KnowledgeBase.load"), \ - mock.patch("issue_bot.knowledge_base.KnowledgeBase.build_context", return_value=""), \ - mock.patch("issue_bot.bedrock_client.BedrockClient.__init__", return_value=None), \ - mock.patch("issue_bot.bedrock_client.BedrockClient.invoke") as mock_bedrock: - - mock_pr.return_value = { - "user": {"login": "dev"}, "title": "Fix", "body": "", - "state": "open", "html_url": "https://github.com/x", - "head": {"sha": "abc123"}, - } - mock_comments.return_value = [] - mock_rc.return_value = [] - mock_files.return_value = [{"filename": "f.py"}] - mock_content.return_value = "content" - mock_ci.return_value = (True, "CI passed") - mock_bedrock.return_value = json.dumps({"comments": [ - {"file": "f.py", "line": 1, "severity": "BUG", "comment": "bug", - "evidence": "evidence1"}, - {"file": "f.py", "line": 2, "severity": "NIT", "comment": "nit", - "evidence": "evidence2"}, - ]}) - - bot_main.analyze() - - with open(str(tmp_path / "result.json")) as f: - result = json.load(f) - - # Both BUG and NIT should be present on first review - assert len(result["inline_comments"]) == 2 - - def test_evidence_formatted_in_comment(self, tmp_path, monkeypatch): - import unittest.mock as mock - monkeypatch.setenv("GITHUB_TOKEN", "fake") - monkeypatch.setenv("GITHUB_REPOSITORY", "awslabs/test") - monkeypatch.setenv("ISSUE_NUMBER", "10") - monkeypatch.setenv("EVENT_TYPE", "pull_request_target") - monkeypatch.setenv("EVENT_ACTION", "opened") - monkeypatch.setenv("EVENT_BEFORE", "") - monkeypatch.setenv("EVENT_AFTER", "") - monkeypatch.setenv("KB_S3_BUCKET", "") - monkeypatch.setenv("KB_S3_KEY", "") - monkeypatch.setenv("PR_FILE_REVIEW_PROMPT", "Review. Date: {current_date}") - import issue_bot.main as bot_main - monkeypatch.setattr(bot_main, "ARTIFACT_PATH", str(tmp_path / "result.json")) - - with mock.patch("issue_bot.github_client.GitHubClient.get_pr") as mock_pr, \ - mock.patch("issue_bot.github_client.GitHubClient.get_comments") as mock_comments, \ - mock.patch("issue_bot.github_client.GitHubClient.get_pr_diff"), \ - mock.patch("issue_bot.github_client.GitHubClient.get_pr_review_comments") as mock_rc, \ - mock.patch("issue_bot.github_client.GitHubClient.get_pr_files") as mock_files, \ - mock.patch("issue_bot.github_client.GitHubClient.get_file_content") as mock_content, \ - mock.patch("issue_bot.github_client.GitHubClient.get_codebase_map"), \ - mock.patch("issue_bot.github_client.GitHubClient.get_ci_status") as mock_ci, \ - mock.patch("issue_bot.knowledge_base.KnowledgeBase.load"), \ - mock.patch("issue_bot.knowledge_base.KnowledgeBase.build_context", return_value=""), \ - mock.patch("issue_bot.bedrock_client.BedrockClient.__init__", return_value=None), \ - mock.patch("issue_bot.bedrock_client.BedrockClient.invoke") as mock_bedrock: - - mock_pr.return_value = { - "user": {"login": "dev"}, "title": "Fix", "body": "", - "state": "open", "html_url": "https://github.com/x", - "head": {"sha": "abc123"}, - } - mock_comments.return_value = [] - mock_rc.return_value = [] - mock_files.return_value = [{"filename": "f.py"}] - mock_content.return_value = "content" - mock_ci.return_value = (True, "CI passed") - mock_bedrock.return_value = json.dumps({"comments": [ - {"file": "f.py", "line": 5, "severity": "BUG", - "comment": "division by zero", - "evidence": "line 3 sets count=0, line 5 divides by count"}, - ]}) - - bot_main.analyze() - - with open(str(tmp_path / "result.json")) as f: - result = json.load(f) - - comment_text = result["inline_comments"][0]["comment"] - assert comment_text.startswith("**BUG**: ") - assert "division by zero" in comment_text - assert "line 3 sets count=0" in comment_text - - -class TestIncrementalReviewIntegration: - """End-to-end tests for the incremental review path through analyze().""" - - def _setup_env(self, tmp_path, monkeypatch, event_before="abc123", event_after="def456"): - monkeypatch.setenv("GITHUB_TOKEN", "fake") - monkeypatch.setenv("GITHUB_REPOSITORY", "awslabs/test") - monkeypatch.setenv("ISSUE_NUMBER", "99") - monkeypatch.setenv("EVENT_TYPE", "pull_request_target") - monkeypatch.setenv("EVENT_ACTION", "synchronize") - monkeypatch.setenv("EVENT_BEFORE", event_before) - monkeypatch.setenv("EVENT_AFTER", event_after) - monkeypatch.setenv("GITHUB_ACTOR", "contributor") - monkeypatch.setenv("KB_S3_BUCKET", "") - monkeypatch.setenv("KB_S3_KEY", "") - monkeypatch.setenv("PR_FILE_REVIEW_PROMPT", "Review this PR. Date: {current_date}") - import issue_bot.main as bot_main - monkeypatch.setattr(bot_main, "ARTIFACT_PATH", str(tmp_path / "result.json")) - - def test_incremental_review_filters_stale_comments(self, tmp_path, monkeypatch): - import unittest.mock as mock - self._setup_env(tmp_path, monkeypatch) - - incremental_diff = ( - "diff --git a/src/fixed.py b/src/fixed.py\n" - "--- a/src/fixed.py\n+++ b/src/fixed.py\n" - "@@ -1 +1 @@\n-old\n+new\n" - ) - - with mock.patch("issue_bot.github_client.GitHubClient.get_pr") as mock_pr, \ - mock.patch("issue_bot.github_client.GitHubClient.get_comments") as mock_comments, \ - mock.patch("issue_bot.github_client.GitHubClient.get_pr_diff") as mock_diff, \ - mock.patch("issue_bot.github_client.GitHubClient.get_pr_review_comments") as mock_rc, \ - mock.patch("issue_bot.github_client.GitHubClient.get_compare_diff") as mock_compare, \ - mock.patch("issue_bot.github_client.GitHubClient.get_pr_files") as mock_files, \ - mock.patch("issue_bot.github_client.GitHubClient.get_file_content") as mock_content, \ - mock.patch("issue_bot.github_client.GitHubClient.get_codebase_map") as mock_map, \ - mock.patch("issue_bot.knowledge_base.KnowledgeBase.load"), \ - mock.patch("issue_bot.knowledge_base.KnowledgeBase.build_context") as mock_kb, \ - mock.patch("issue_bot.bedrock_client.BedrockClient.__init__", return_value=None), \ - mock.patch("issue_bot.bedrock_client.BedrockClient.invoke") as mock_bedrock: - - mock_pr.return_value = {"user": {"login": "contributor"}, "title": "Fix bug", - "body": "Fixes the thing", "state": "open", "html_url": "https://github.com/x"} - mock_comments.return_value = [{"user": {"login": "github-actions[bot]"}, "body": "prior review"}] - mock_diff.return_value = "full diff here" - mock_rc.return_value = [] - mock_compare.return_value = incremental_diff - mock_files.return_value = [{"filename": "src/fixed.py"}, {"filename": "src/untouched.py"}] - mock_content.return_value = "file content" - mock_map.return_value = "" - mock_kb.return_value = "" - mock_bedrock.return_value = json.dumps({ - "comments": [ - {"file": "src/fixed.py", "line": 1, "comment": "new issue in changed file"}, - {"file": "src/untouched.py", "line": 5, "comment": "stale comment on unchanged file"}, - ] - }) - - from issue_bot.main import analyze - analyze() - - with open(str(tmp_path / "result.json")) as f: - result = json.load(f) - - assert result["action"] == "RESPOND" - assert result["is_incremental"] is True - assert len(result["inline_comments"]) == 1 - assert result["inline_comments"][0]["file"] == "src/fixed.py" - - def test_force_push_falls_back_to_full_review(self, tmp_path, monkeypatch): - import unittest.mock as mock - self._setup_env(tmp_path, monkeypatch) - - with mock.patch("issue_bot.github_client.GitHubClient.get_pr") as mock_pr, \ - mock.patch("issue_bot.github_client.GitHubClient.get_comments") as mock_comments, \ - mock.patch("issue_bot.github_client.GitHubClient.get_pr_diff") as mock_diff, \ - mock.patch("issue_bot.github_client.GitHubClient.get_pr_review_comments") as mock_rc, \ - mock.patch("issue_bot.github_client.GitHubClient.get_compare_diff") as mock_compare, \ - mock.patch("issue_bot.github_client.GitHubClient.get_pr_files") as mock_files, \ - mock.patch("issue_bot.github_client.GitHubClient.get_file_content") as mock_content, \ - mock.patch("issue_bot.github_client.GitHubClient.get_codebase_map") as mock_map, \ - mock.patch("issue_bot.knowledge_base.KnowledgeBase.load"), \ - mock.patch("issue_bot.knowledge_base.KnowledgeBase.build_context") as mock_kb, \ - mock.patch("issue_bot.bedrock_client.BedrockClient.__init__", return_value=None), \ - mock.patch("issue_bot.bedrock_client.BedrockClient.invoke") as mock_bedrock: - - mock_pr.return_value = {"user": {"login": "contributor"}, "title": "Fix bug", - "body": "Fixes the thing", "state": "open", "html_url": "https://github.com/x"} - mock_comments.return_value = [{"user": {"login": "github-actions[bot]"}, "body": "prior review"}] - mock_diff.return_value = "full diff here" - mock_rc.return_value = [] - mock_compare.return_value = "" # Force push — compare fails - mock_files.return_value = [{"filename": "src/a.py"}] - mock_content.return_value = "content" - mock_map.return_value = "" - mock_kb.return_value = "" - mock_bedrock.return_value = json.dumps({ - "comments": [ - {"file": "src/a.py", "line": 1, "comment": "issue found"}, - ] - }) - - from issue_bot.main import analyze - analyze() - - with open(str(tmp_path / "result.json")) as f: - result = json.load(f) - - # Falls back to full review — no filtering, not marked incremental - assert result["action"] == "RESPOND" - assert result["is_incremental"] is False - assert len(result["inline_comments"]) == 1 - - def test_no_before_sha_skips_incremental(self, tmp_path, monkeypatch): - import unittest.mock as mock - self._setup_env(tmp_path, monkeypatch, event_before="", event_after="def456") - - with mock.patch("issue_bot.github_client.GitHubClient.get_pr") as mock_pr, \ - mock.patch("issue_bot.github_client.GitHubClient.get_comments") as mock_comments, \ - mock.patch("issue_bot.github_client.GitHubClient.get_pr_diff") as mock_diff, \ - mock.patch("issue_bot.github_client.GitHubClient.get_pr_review_comments") as mock_rc, \ - mock.patch("issue_bot.github_client.GitHubClient.get_compare_diff") as mock_compare, \ - mock.patch("issue_bot.github_client.GitHubClient.get_pr_files") as mock_files, \ - mock.patch("issue_bot.github_client.GitHubClient.get_file_content") as mock_content, \ - mock.patch("issue_bot.github_client.GitHubClient.get_codebase_map") as mock_map, \ - mock.patch("issue_bot.knowledge_base.KnowledgeBase.load"), \ - mock.patch("issue_bot.knowledge_base.KnowledgeBase.build_context") as mock_kb, \ - mock.patch("issue_bot.bedrock_client.BedrockClient.__init__", return_value=None), \ - mock.patch("issue_bot.bedrock_client.BedrockClient.invoke") as mock_bedrock: - - mock_pr.return_value = {"user": {"login": "contributor"}, "title": "Fix", - "body": "Fix", "state": "open", "html_url": "https://github.com/x"} - mock_comments.return_value = [{"user": {"login": "github-actions[bot]"}, "body": "review"}] - mock_diff.return_value = "full diff" - mock_rc.return_value = [] - mock_compare.return_value = "should not be called" - mock_files.return_value = [{"filename": "src/a.py"}] - mock_content.return_value = "content" - mock_map.return_value = "" - mock_kb.return_value = "" - mock_bedrock.return_value = json.dumps({"comments": [ - {"file": "src/a.py", "line": 1, "comment": "issue"}, - ]}) - - from issue_bot.main import analyze - analyze() - - # Should NOT have called compare because event_before is empty - mock_compare.assert_not_called() - - with open(str(tmp_path / "result.json")) as f: - result = json.load(f) - assert result["is_incremental"] is False - - -class TestFileContentUsesHeadSha: - """Verify get_file_content is called with PR head SHA, not default branch.""" - - def _setup_env(self, tmp_path, monkeypatch): - monkeypatch.setenv("GITHUB_TOKEN", "fake") - monkeypatch.setenv("GITHUB_REPOSITORY", "awslabs/test") - monkeypatch.setenv("ISSUE_NUMBER", "42") - monkeypatch.setenv("EVENT_TYPE", "pull_request_target") - monkeypatch.setenv("EVENT_ACTION", "opened") - monkeypatch.setenv("EVENT_BEFORE", "") - monkeypatch.setenv("EVENT_AFTER", "") - monkeypatch.setenv("GITHUB_ACTOR", "contributor") - monkeypatch.setenv("KB_S3_BUCKET", "") - monkeypatch.setenv("KB_S3_KEY", "") - monkeypatch.setenv("PR_FILE_REVIEW_PROMPT", "Review this PR. Date: {current_date}") - import issue_bot.main as bot_main - monkeypatch.setattr(bot_main, "ARTIFACT_PATH", str(tmp_path / "result.json")) - - def test_file_content_fetched_with_head_sha(self, tmp_path, monkeypatch): - import unittest.mock as mock - self._setup_env(tmp_path, monkeypatch) - - with mock.patch("issue_bot.github_client.GitHubClient.get_pr") as mock_pr, \ - mock.patch("issue_bot.github_client.GitHubClient.get_comments") as mock_comments, \ - mock.patch("issue_bot.github_client.GitHubClient.get_pr_diff") as mock_diff, \ - mock.patch("issue_bot.github_client.GitHubClient.get_pr_review_comments") as mock_rc, \ - mock.patch("issue_bot.github_client.GitHubClient.get_pr_files") as mock_files, \ - mock.patch("issue_bot.github_client.GitHubClient.get_file_content") as mock_content, \ - mock.patch("issue_bot.github_client.GitHubClient.get_codebase_map") as mock_map, \ - mock.patch("issue_bot.knowledge_base.KnowledgeBase.load"), \ - mock.patch("issue_bot.knowledge_base.KnowledgeBase.build_context") as mock_kb, \ - mock.patch("issue_bot.bedrock_client.BedrockClient.__init__", return_value=None), \ - mock.patch("issue_bot.bedrock_client.BedrockClient.invoke") as mock_bedrock: - - mock_pr.return_value = { - "user": {"login": "contributor"}, "title": "Add feature", - "body": "New file", "state": "open", - "html_url": "https://github.com/x", - "head": {"sha": "abc123deadbeef"}, - } - mock_comments.return_value = [] - mock_diff.return_value = "diff content" - mock_rc.return_value = [] - mock_files.return_value = [ - {"filename": "src/new_file.py"}, - {"filename": "src/existing.py"}, - ] - mock_content.return_value = "file content" - mock_map.return_value = "" - mock_kb.return_value = "" - mock_bedrock.return_value = json.dumps({"comments": []}) - - from issue_bot.main import analyze - analyze() - - # Every get_file_content call must include ref=head_sha - for call in mock_content.call_args_list: - args, kwargs = call - assert kwargs.get("ref") == "abc123deadbeef" or \ - (len(args) > 1 and args[1] == "abc123deadbeef"), \ - f"get_file_content called without head SHA: {call}" - - def test_missing_head_sha_falls_back_gracefully(self, tmp_path, monkeypatch): - import unittest.mock as mock - self._setup_env(tmp_path, monkeypatch) - - with mock.patch("issue_bot.github_client.GitHubClient.get_pr") as mock_pr, \ - mock.patch("issue_bot.github_client.GitHubClient.get_comments") as mock_comments, \ - mock.patch("issue_bot.github_client.GitHubClient.get_pr_diff") as mock_diff, \ - mock.patch("issue_bot.github_client.GitHubClient.get_pr_review_comments") as mock_rc, \ - mock.patch("issue_bot.github_client.GitHubClient.get_pr_files") as mock_files, \ - mock.patch("issue_bot.github_client.GitHubClient.get_file_content") as mock_content, \ - mock.patch("issue_bot.github_client.GitHubClient.get_codebase_map") as mock_map, \ - mock.patch("issue_bot.knowledge_base.KnowledgeBase.load"), \ - mock.patch("issue_bot.knowledge_base.KnowledgeBase.build_context") as mock_kb, \ - mock.patch("issue_bot.bedrock_client.BedrockClient.__init__", return_value=None), \ - mock.patch("issue_bot.bedrock_client.BedrockClient.invoke") as mock_bedrock: - - # PR object without head.sha (shouldn't happen, but defensive) - mock_pr.return_value = { - "user": {"login": "contributor"}, "title": "Fix", - "body": "Fix", "state": "open", - "html_url": "https://github.com/x", - } - mock_comments.return_value = [] - mock_diff.return_value = "diff" - mock_rc.return_value = [] - mock_files.return_value = [{"filename": "src/a.py"}] - mock_content.return_value = "content" - mock_map.return_value = "" - mock_kb.return_value = "" - mock_bedrock.return_value = json.dumps({"comments": []}) - - from issue_bot.main import analyze - analyze() - - # Should still work — falls back to no ref (default branch) - with open(str(tmp_path / "result.json")) as f: - result = json.load(f) - # First review with 0 comments → RESPOND with CI-aware message - assert result["action"] == "RESPOND" - assert "No issues found" in result["response"] - - -class TestReviewEventType: - """Verify bot always uses COMMENT event type, never REQUEST_CHANGES.""" - - def _setup_env(self, tmp_path, monkeypatch, event_action="opened"): - monkeypatch.setenv("GITHUB_TOKEN", "fake") - monkeypatch.setenv("GITHUB_REPOSITORY", "awslabs/test") - monkeypatch.setenv("ISSUE_NUMBER", "50") - monkeypatch.setenv("EVENT_TYPE", "pull_request_target") - monkeypatch.setenv("EVENT_ACTION", event_action) - monkeypatch.setenv("EVENT_BEFORE", "aaa111" if event_action == "synchronize" else "") - monkeypatch.setenv("EVENT_AFTER", "bbb222" if event_action == "synchronize" else "") - monkeypatch.setenv("GITHUB_ACTOR", "contributor") - monkeypatch.setenv("KB_S3_BUCKET", "") - monkeypatch.setenv("KB_S3_KEY", "") - monkeypatch.setenv("PR_FILE_REVIEW_PROMPT", "Review. Date: {current_date}") - import issue_bot.main as bot_main - monkeypatch.setattr(bot_main, "ARTIFACT_PATH", str(tmp_path / "result.json")) - - def _run_and_get_artifact(self, tmp_path, monkeypatch, mock, event_action="opened"): - self._setup_env(tmp_path, monkeypatch, event_action=event_action) - - with mock.patch("issue_bot.github_client.GitHubClient.get_pr") as mock_pr, \ - mock.patch("issue_bot.github_client.GitHubClient.get_comments") as mock_comments, \ - mock.patch("issue_bot.github_client.GitHubClient.get_pr_diff") as mock_diff, \ - mock.patch("issue_bot.github_client.GitHubClient.get_pr_review_comments") as mock_rc, \ - mock.patch("issue_bot.github_client.GitHubClient.get_compare_diff") as mock_compare, \ - mock.patch("issue_bot.github_client.GitHubClient.get_pr_files") as mock_files, \ - mock.patch("issue_bot.github_client.GitHubClient.get_file_content") as mock_content, \ - mock.patch("issue_bot.github_client.GitHubClient.get_codebase_map") as mock_map, \ - mock.patch("issue_bot.knowledge_base.KnowledgeBase.load"), \ - mock.patch("issue_bot.knowledge_base.KnowledgeBase.build_context") as mock_kb, \ - mock.patch("issue_bot.bedrock_client.BedrockClient.__init__", return_value=None), \ - mock.patch("issue_bot.bedrock_client.BedrockClient.invoke") as mock_bedrock, \ - mock.patch("issue_bot.github_client.GitHubClient.post_pr_review") as mock_post: - - mock_pr.return_value = { - "user": {"login": "contributor"}, "title": "Fix", - "body": "Fix", "state": "open", - "html_url": "https://github.com/x", - "head": {"sha": "abc123"}, - } - mock_comments.return_value = ( - [{"user": {"login": "github-actions[bot]"}, "body": "prior"}] - if event_action == "synchronize" else [] - ) - mock_diff.return_value = "diff" - mock_rc.return_value = [] - mock_compare.return_value = ( - "diff --git a/f.py b/f.py\n--- a/f.py\n+++ b/f.py\n@@ -1 +1 @@\n-x\n+y\n" - if event_action == "synchronize" else "" - ) - mock_files.return_value = [{"filename": "f.py"}] - mock_content.return_value = "content" - mock_map.return_value = "" - mock_kb.return_value = "" - mock_bedrock.return_value = json.dumps({ - "comments": [{"file": "f.py", "line": 1, "comment": "issue"}] - }) - mock_post.return_value = True - - from issue_bot.main import analyze, act - analyze() - - with open(str(tmp_path / "result.json")) as f: - return json.load(f), mock_post - - def test_first_review_uses_comment_event(self, tmp_path, monkeypatch): - import unittest.mock as mock - result, _ = self._run_and_get_artifact(tmp_path, monkeypatch, mock, "opened") - assert result["action"] == "RESPOND" - assert result.get("is_incremental") is False - assert len(result["inline_comments"]) > 0 - - def test_incremental_review_uses_comment_event(self, tmp_path, monkeypatch): - import unittest.mock as mock - result, _ = self._run_and_get_artifact(tmp_path, monkeypatch, mock, "synchronize") - assert result["action"] == "RESPOND" - assert result.get("is_incremental") is True