Bitbucket: Add push_commands support when PR is updated

[sharoneyal]

PR Type

Enhancement

---

Description

• Add support for push_commands when Bitbucket PR is updated

• Implement validation to ensure push commands only trigger from actual commits

• Validate time delta between last commit and PR update (max 15 seconds)

• Verify commit author matches PR updater to prevent false triggers

---

Diagram Walkthrough

flowchart LR
  A["Bitbucket pullrequest:updated event"] --> B["Validate push trigger enabled"]
  B --> C["Check time delta between commit and PR update"]
  C --> D["Verify commit author matches PR updater"]
  D --> E["Execute push_commands if valid"]
  D --> F["Skip if validation fails"]

Loading

File Walkthrough

Relevant files

Enhancement

bitbucket_app.py Add push_commands validation and pullrequest:updated handler pr_agent/servers/bitbucket_app.py Added _validate_time_from_last_commit_to_pr_update() function to validate push events by checking time delta and author match Added handler for pullrequest:updated event to trigger push_commands with validation Added configuration check for bitbucket_app.handle_push_trigger setting Fixed agent instantiation to reuse existing agent instead of creating new PRAgent() instance +76/-1

---

[qodo-merge-for-open-source]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢	No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Consistent Naming Conventions Objective: All new variables, functions, and classes must follow the project's established naming standards Status: Passed
	No Dead or Commented-Out Code Objective: Keep the codebase clean by ensuring all submitted code is active and necessary Status: Passed
	When relevant, utilize early return Objective: In a code snippet containing multiple logic conditions (such as 'if-else'), prefer an early return on edge cases than deep nesting Status: Passed
⚪	Robust Error Handling Objective: Ensure potential errors and edge cases are anticipated and handled gracefully throughout the code Status: Time parsing robustness: The new validation parses ISO timestamps with datetime.fromisoformat without handling timezone/Z suffix variants or non-ISO responses, which may raise exceptions or miscompute diffs depending on Bitbucket formats. Referred Code from datetime import datetime ts1 = datetime.fromisoformat(time_pr_updated) ts2 = datetime.fromisoformat(time_last_commit) diff = (ts1 - ts2).total_seconds() max_delta_seconds = 15 if diff > 0 and diff < max_delta_seconds: is_valid_push = True else: get_logger().debug(f"Too much time passed since last commit", artifact={'updated': time_pr_updated, 'last_commit': time_last_commit})
	Single Responsibility for Functions Objective: Each function should have a single, well-defined responsibility Status: Mixed concerns: _perform_commands_bitbucket handles config checks, event gating, validation, command iteration, and logging, which may span multiple responsibilities. Referred Code async def _perform_commands_bitbucket(commands_conf: str, agent: PRAgent, api_url: str, log_context: dict, data: dict): apply_repo_settings(api_url) if commands_conf == "pr_commands" and get_settings().config.disable_auto_feedback: # auto commands for PR, and auto feedback is disabled get_logger().info(f"Auto feedback is disabled, skipping auto commands for PR {api_url=}") return if commands_conf == "push_commands": if not get_settings().get("bitbucket_app.handle_push_trigger"): get_logger().info( "Bitbucket push trigger handling disabled via 'bitbucket_app.handle_push_trigger'; skipping push commands") return if data.get("event", "") == "pullrequest:created": if not should_process_pr_logic(data): return commands = get_settings().get(f"bitbucket_app.{commands_conf}", {}) get_settings().set("config.is_auto_command", True) if commands_conf == "push_commands": is_valid_push = await _validate_time_from_last_commit_to_pr_update(data) if not is_valid_push: get_logger().info(f"Bitbucket skipping 'pullrequest:updated' for push commands") return for command in commands:

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-merge-for-open-source]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
High-level	Use an async HTTP client The async function _validate_time_from_last_commit_to_pr_update uses the synchronous requests library for an API call, which blocks the event loop. This should be replaced with an asynchronous HTTP client like httpx to avoid performance bottlenecks. Examples: pr_agent/servers/bitbucket_app.py [86-138] async def _validate_time_from_last_commit_to_pr_update(data: dict) -> bool: is_valid_push = False try: data_inner = data.get('data', {}) if not data_inner: get_logger().error("No data found in the webhook payload") return True pull_request = data_inner.get('pullrequest', {}) commits_api = pull_request.get('links', {}).get('commits', {}).get('href') if not commits_api: ... (clipped 43 lines) Solution Walkthrough: Before: async def _validate_time_from_last_commit_to_pr_update(data: dict) -> bool: try: # ... commits_api = pull_request.get('links', {}).get('commits', {}).get('href') # ... headers = { 'Authorization': f'Bearer {bearer_token}', 'Accept': 'application/json' } # Synchronous call blocking the event loop response = requests.get(commits_api, headers=headers) # ... except Exception as e: # ... return is_valid_push After: import httpx # or aiohttp async def _validate_time_from_last_commit_to_pr_update(data: dict) -> bool: try: # ... commits_api = pull_request.get('links', {}).get('commits', {}).get('href') # ... headers = { 'Authorization': f'Bearer {bearer_token}', 'Accept': 'application/json' } # Asynchronous call that doesn't block async with httpx.AsyncClient() as client: response = await client.get(commits_api, headers=headers) # ... except Exception as e: # ... return is_valid_push Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies a blocking requests.get() call within an async function, which is a critical performance issue that undermines the async architecture of the server.	High
Possible issue	Avoid blocking the event loop In the _validate_time_from_last_commit_to_pr_update async function, wrap the synchronous requests.get() call with asyncio.get_event_loop().run_in_executor() to prevent blocking the event loop. pr_agent/servers/bitbucket_app.py [104] -response = requests.get(commits_api, headers=headers) +import asyncio +... +response = await asyncio.get_event_loop().run_in_executor( + None, lambda: requests.get(commits_api, headers=headers, timeout=30) +) [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies a blocking I/O call (requests.get) inside an async function, which can degrade server performance by blocking the event loop. The proposed fix is appropriate.	Medium
General	Simplify commit data extraction logic Refactor the commit data extraction logic by first storing the latest commit in a variable and then safely accessing its nested properties using .get(). This will simplify the code, reduce repetition, and improve readability. pr_agent/servers/bitbucket_app.py [112-123] -if (not values or not isinstance(values, list) or not values[0].get('author') or not values[0]['author'].get('user') - or not values[0]['author']['user'].get('display_name')): - get_logger().warning("No commits returned for pull request or one of the required fields missing; skipping push validation", +values = commits_data.get('values') or [] +if not values or not isinstance(values, list): + get_logger().warning("No commits returned for pull request; skipping push validation", artifact={'values': values}) return False -commit_username = commits_data['values'][0]['author']['user']['display_name'] + +latest_commit = values[0] +commit_username = latest_commit.get('author', {}).get('user', {}).get('display_name') +time_last_commit = latest_commit.get('date') + +if not commit_username or not time_last_commit: + get_logger().warning("Latest commit is missing required fields (author's display_name or date); skipping push validation", + artifact={'commit': latest_commit}) + return False + if username != commit_username: get_logger().warning(f"Mismatch in username {username} vs. commit_username {commit_username}") return False time_pr_updated = pull_request['updated_on'] -time_last_commit = commits_data['values'][0]['date'] Apply / Chat Suggestion importance[1-10]: 6 __ Why: The suggestion correctly identifies repetitive and complex dictionary access. The proposed refactoring improves code readability and maintainability by reducing repetition and using safer access patterns.	Low
	Make the time delta configurable Replace the hardcoded max_delta_seconds value with a configurable setting retrieved via get_settings() to improve maintainability. pr_agent/servers/bitbucket_app.py [128] -max_delta_seconds = 15 +max_delta_seconds = get_settings().get("bitbucket_app.push_trigger_max_commit_time_delta", 15) Apply / Chat Suggestion importance[1-10]: 5 __ Why: The suggestion correctly points out a hardcoded value and proposes making it configurable, which is a good practice for maintainability and flexibility.	Low
Learned best practice	Safely access bearer token Safely access bitbucket_bearer_token and validate it before use to avoid sending an invalid Authorization header. Log a clear warning and abort validation when missing. pr_agent/servers/bitbucket_app.py [99-104] -bearer_token = context.get('bitbucket_bearer_token') +bearer_token = context.get('bitbucket_bearer_token', None) +if not bearer_token: + get_logger().warning("Missing Bitbucket bearer token; skipping push validation") + return False headers = { 'Authorization': f'Bearer {bearer_token}', 'Accept': 'application/json' } response = requests.get(commits_api, headers=headers) Apply / Chat Suggestion importance[1-10]: 6 __ Why: Relevant best practice - Guard access to nested settings and attributes with safe accessors and validation, providing defaults or clear errors when missing.	Low
	Robust timestamp parsing Parse ISO timestamps defensively to handle timezone offsets/Z suffix and validate types before computing the diff; fall back gracefully on parsing errors. pr_agent/servers/bitbucket_app.py [122-130] -time_pr_updated = pull_request['updated_on'] -time_last_commit = commits_data['values'][0]['date'] -from datetime import datetime -ts1 = datetime.fromisoformat(time_pr_updated) -ts2 = datetime.fromisoformat(time_last_commit) +time_pr_updated = pull_request.get('updated_on') +time_last_commit = commits_data.get('values', [{}])[0].get('date') +from datetime import datetime, timezone +def _parse_iso(ts: str): + if not isinstance(ts, str): return None + ts = ts.strip().replace('Z', '+00:00') + try: + return datetime.fromisoformat(ts) + except Exception: + return None +ts1 = _parse_iso(time_pr_updated) +ts2 = _parse_iso(time_last_commit) +if not ts1 or not ts2: + get_logger().warning("Failed to parse timestamps for push validation", + artifact={'updated': time_pr_updated, 'last_commit': time_last_commit}) + return False diff = (ts1 - ts2).total_seconds() max_delta_seconds = 15 -if diff > 0 and diff < max_delta_seconds: +if 0 < diff < max_delta_seconds: is_valid_push = True Apply / Chat Suggestion importance[1-10]: 5 __ Why: Relevant best practice - Replace brittle string operations and ad-hoc logic with robust parsing and validation for external inputs.	Low
More

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

[ofir-frd]

/implement
Remove bitbucket_app.handle_push_trigger' from the comment to make it easier to maintain in the future. Draft 3 alternatives.

[qodo-merge-for-open-source]

Code Implementation 🛠️

Implementation: Update the log message to remove the hardcoded 'bitbucket_app.handle_push_trigger' configuration key reference. Provide three concise alternative messages that are easier to maintain.

Suggested change

	"Bitbucket push trigger handling disabled via 'bitbucket_app.handle_push_trigger'; skipping push commands")
	# Alternative 1:
	"Bitbucket push trigger handling disabled; skipping push commands")
	# Alternative 2:
	"Push trigger handling is disabled; skipping push commands")
	# Alternative 3:
	"Push-trigger handling disabled; skipping push commands")

See review comment here