Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions .claude/hooks/no-git-add.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
#!/bin/bash
# Bash hook: Block git add — ask user to stage files instead
INPUT=$(cat)
CMD=$(echo "$INPUT" | jq -r '.tool_input.command // ""')

if echo "$CMD" | grep -qE '(^|\s|&&|;)\s*git add\b'; then
jq -n '{
"decision": "block",
"reason": "BLOCKED: Do not run git add. Ask the user to stage files. Other sessions may have unstaged changes that would pollute the PR."
}'
exit 2
fi

exit 0
12 changes: 12 additions & 0 deletions .claude/settings.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,17 @@
{
"hooks": {
"PreToolUse": [
{
"matcher": "Bash",
"hooks": [
{
"type": "command",
"command": "/Users/rwest/Repositories/gitauto/.claude/hooks/no-git-add.sh",
"timeout": 5
}
]
}
],
"Stop": [
{
"matcher": "",
Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[project]
name = "GitAuto"
version = "1.15.0"
version = "1.17.0"
requires-python = ">=3.14"
dependencies = [
"annotated-doc==0.0.4",
Expand Down
12 changes: 8 additions & 4 deletions services/github/branches/get_required_status_checks.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
class StatusChecksResult:
status_code: int = 201
checks: list[str] | None = None
app_ids: set[int] | None = None
strict: bool = True


Expand Down Expand Up @@ -65,10 +66,13 @@ def get_required_status_checks(owner: str, repo: str, branch: str, token: str):

strict = required_status_checks.get("strict", False)
contexts = set(required_status_checks.get("contexts", []))
checks = {
check.get("context") for check in required_status_checks.get("checks", [])
}
checks_list = required_status_checks.get("checks", [])
checks = {check.get("context") for check in checks_list}
app_ids = {check.get("app_id") for check in checks_list if check.get("app_id")}

return StatusChecksResult(
status_code=200, checks=list(contexts | checks), strict=strict
status_code=200,
checks=list(contexts | checks),
app_ids=app_ids or None,
strict=strict,
)
6 changes: 6 additions & 0 deletions services/github/branches/test_get_required_status_checks.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ def test_get_required_status_checks_success(
"CircleCI Checks",
"Aikido Security",
}
assert result.app_ids == {12345, 67890}
assert result.strict is True


Expand All @@ -78,6 +79,7 @@ def test_get_required_status_checks_403_no_permission(

assert result.status_code == 403
assert result.checks is None
assert result.app_ids is None
assert result.strict is True


Expand All @@ -100,6 +102,7 @@ def test_get_required_status_checks_404_no_protection(

assert result.status_code == 404
assert not result.checks
assert result.app_ids is None
assert result.strict is False


Expand All @@ -123,6 +126,7 @@ def test_get_required_status_checks_no_required_checks(

assert result.status_code == 200
assert not result.checks
assert result.app_ids is None
assert result.strict is False


Expand Down Expand Up @@ -150,6 +154,7 @@ def test_get_required_status_checks_only_contexts(test_owner, test_repo, test_to

assert result.status_code == 200
assert result.checks == ["ci/circleci: test"]
assert result.app_ids is None
assert result.strict is True


Expand Down Expand Up @@ -177,6 +182,7 @@ def test_get_required_status_checks_only_checks(test_owner, test_repo, test_toke

assert result.status_code == 200
assert result.checks == ["CircleCI Checks"]
assert result.app_ids == {12345}
assert result.strict is False


Expand Down
13 changes: 13 additions & 0 deletions services/github/types/mergeable_state.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
from typing import Literal

# https://docs.github.com/en/graphql/reference/enums#mergestatestatus
MergeableState = Literal[
"behind",
"blocked",
"clean",
"dirty",
"draft",
"has_hooks",
"unknown",
"unstable",
]
3 changes: 2 additions & 1 deletion services/github/types/pull_request.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from typing import Optional, TypedDict

from services.github.types.label import Label
from services.github.types.mergeable_state import MergeableState
from services.github.types.ref import Ref
from services.github.types.user import User

Expand Down Expand Up @@ -45,7 +46,7 @@ class PullRequest(TypedDict):
merged: bool
mergeable: Optional[bool]
rebaseable: Optional[bool]
mergeable_state: str
mergeable_state: MergeableState
merged_by: Optional[User]
comments: int
review_comments: int
Expand Down
19 changes: 19 additions & 0 deletions services/github/types/test_mergeable_state.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
from typing import get_args

from services.github.types.mergeable_state import MergeableState

# https://docs.github.com/en/graphql/reference/enums#mergestatestatus
EXPECTED_STATES = {
"behind",
"blocked",
"clean",
"dirty",
"draft",
"has_hooks",
"unknown",
"unstable",
}


def test_mergeable_state_values():
assert set(get_args(MergeableState)) == EXPECTED_STATES
124 changes: 93 additions & 31 deletions services/webhook/successful_check_suite_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@
from services.github.pulls.get_pull_request_files import get_pull_request_files
from services.github.pulls.merge_pull_request import MergeMethod, merge_pull_request
from services.github.token.get_installation_token import get_installation_access_token
from services.github.types.check_suite import CheckSuite
from services.github.types.github_types import CheckSuiteCompletedPayload
from services.github.types.mergeable_state import MergeableState
from services.slack.slack_notify import slack_notify
from services.supabase.client import supabase
from services.supabase.repository_features.get_repository_features import (
Expand All @@ -28,6 +30,14 @@

BLOCKED = "Auto-merge blocked"

BLOCKED_STATE_REASONS: dict[MergeableState, str] = {
"behind": "PR branch is behind base branch",
"blocked": "The merge is blocked",
"dirty": "merge conflicts detected",
"draft": "PR is in draft mode",
"unknown": "GitHub still calculating mergeability",
}


@handle_exceptions(default_return_value=None, raise_on_error=False)
def handle_successful_check_suite(payload: CheckSuiteCompletedPayload):
Expand Down Expand Up @@ -84,19 +94,24 @@ def handle_successful_check_suite(payload: CheckSuiteCompletedPayload):
.execute()
)
if result.data:
logger.info("Marking usage record as test-passed")
usage_id = result.data[0]["id"]
(
supabase.table("usage")
.update({"is_test_passed": True})
.eq("id", usage_id)
.execute()
)
else:
logger.info(
"No usage record found for PR #%s, skipping test-passed update", pr_number
)

# Check if auto-merge is enabled BEFORE doing expensive API calls
# (branch protection, check suites, PR details, etc.)
repo_features = get_repository_features(owner_id=owner_id, repo_id=repo_id)
if not repo_features or not repo_features.get("auto_merge"):
logger.info("Auto-merge disabled for repo_id=%s", repo_id)
logger.info("Skipping because auto-merge is disabled for repo_id=%s", repo_id)
return

comment_args = cast(
Expand All @@ -123,39 +138,80 @@ def handle_successful_check_suite(payload: CheckSuiteCompletedPayload):
owner=owner_name, repo=repo_name, branch=base_branch, token=token
)

if protection.checks:
logger.info("Using required status checks: %s", protection.checks)
for suite in all_suites:
# Filter out ghost suites: GitHub auto-creates a check suite for every installed app on each push, but apps that don't intend to run (e.g. GitAuto AI, Codecov) never create check runs, leaving the suite perpetually "queued". These would block auto-merge forever. A real suite (GitHub Actions, CircleCI) creates check runs immediately, even while queued.
active_suites: list[CheckSuite] = []
for suite in all_suites:
app_name = suite["app"]["name"]
if suite["status"] == "queued" and suite["latest_check_runs_count"] == 0:
logger.info(
"Skipping ghost suite '%s' because it is queued with 0 check runs",
app_name,
)
continue

logger.info(
"Active suite '%s': status=%s, check_runs=%d",
app_name,
suite["status"],
suite["latest_check_runs_count"],
)
active_suites.append(suite)

if not active_suites:
logger.info(
"All %s suites are ghost suites, nothing to wait for", len(all_suites)
)
# Fall through to mergeable_state check below
elif protection.checks:
logger.info(
"Using required status checks: %s (app_ids=%s)",
protection.checks,
protection.app_ids,
)
for suite in active_suites:
app_name = suite["app"]["name"]
app_id = suite["app"]["id"]
status = suite["status"]
if app_name in protection.checks and status != "completed":
if (
protection.app_ids
and app_id in protection.app_ids
and status != "completed"
):
logger.info(
"Required check '%s' not completed: status=%s", app_name, status
"Returning because suite '%s' (app_id=%s) contains a required check and is not completed (status=%s)",
app_name,
app_id,
status,
)
return
logger.info("All required checks completed")
logger.info("All suites with required checks completed, proceeding to merge")
else:
if protection.checks is None:
logger.info(
"Could not read branch protection (status=%s), "
"waiting for all check suites to complete",
"will return if any check suite is not completed",
protection.status_code,
)
else:
logger.info(
"No required checks configured, "
"waiting for all check suites to complete"
"will return if any check suite is not completed"
)
for suite in all_suites:
for suite in active_suites:
app_name = suite["app"]["name"]
status = suite["status"]
if status != "completed":
logger.info(
"Check suite '%s' not completed: status=%s", app_name, status
"Returning because check suite '%s' is not completed (status=%s)",
app_name,
status,
)
return

logger.info("All %s check suites completed", len(all_suites))
logger.info(
"All %s active check suites completed, proceeding to merge",
len(active_suites),
)

# Fetch full PR details to get mergeable_state (not in simplified PR from check_suite webhook)
full_pr = get_pull_request(
Expand All @@ -168,37 +224,32 @@ def handle_successful_check_suite(payload: CheckSuiteCompletedPayload):
slack_notify(slack_msg)
return

# Check mergeable_state - only proceed if merging is allowed
mergeable_state = full_pr["mergeable_state"]
logger.info("PR mergeable_state=%s", mergeable_state)

# https://docs.github.com/en/graphql/reference/enums#mergestatestatus
mergeable_state = full_pr.get("mergeable_state", "")
logger.info("PR mergeable_state: %s", mergeable_state)

# Allow merge for: clean, unstable (failing non-required checks), has_hooks
if mergeable_state not in ["clean", "unstable", "has_hooks"]:
state_reasons = {
"behind": "PR branch is behind base branch",
"blocked": "The merge is blocked",
"dirty": "merge conflicts detected",
"draft": "PR is in draft mode",
"unknown": "GitHub still calculating mergeability",
}
if mergeable_state in BLOCKED_STATE_REASONS:

if mergeable_state == "blocked":
logger.info(
"Merge blocked because mergeable_state=blocked, checking active suite statuses"
)
check_statuses = [
f"{s['app']['name']}: status={s['status']}, conclusion={s.get('conclusion', 'null')}"
for s in all_suites
for s in active_suites
]
reason = (
state_reasons["blocked"]
BLOCKED_STATE_REASONS["blocked"]
+ ". Check suites: "
+ "; ".join(check_statuses)
)
else:
reason = state_reasons.get(mergeable_state, "unknown reason")
logger.info("Merge blocked because mergeable_state=%s", mergeable_state)
reason = BLOCKED_STATE_REASONS.get(mergeable_state, "unknown reason")

# If any check suite is still in_progress, skip notification - we're just waiting
if any(s["status"] == "in_progress" for s in all_suites):
logger.info("Check suites still in progress, waiting...")
# If any active check suite is still running, skip notification and return
if any(s["status"] == "in_progress" for s in active_suites):
logger.info("Returning because active check suites are still running")
return

msg = f"{BLOCKED}: mergeable_state={mergeable_state} ({reason})"
Expand All @@ -217,6 +268,10 @@ def handle_successful_check_suite(payload: CheckSuiteCompletedPayload):
slack_notify(slack_msg)
return

logger.info(
"PR mergeable_state=%s is acceptable, proceeding to merge", mergeable_state
)

# Get PR files
changed_files = get_pull_request_files(
owner=owner_name, repo=repo_name, pr_number=pr_number, token=token
Expand All @@ -225,6 +280,7 @@ def handle_successful_check_suite(payload: CheckSuiteCompletedPayload):
# Check if only test files restriction is enabled
only_test_files = repo_features.get("auto_merge_only_test_files", False)
if only_test_files:
logger.info("auto_merge_only_test_files enabled, checking changed files")
non_test_files = [
f["filename"]
for f in changed_files
Expand All @@ -246,6 +302,9 @@ def handle_successful_check_suite(payload: CheckSuiteCompletedPayload):
slack_msg = f"`{owner_name}/{repo_name}` PR #{pr_number}: {msg}"
slack_notify(slack_msg)
return
logger.info("All changed files are test/config files, proceeding to merge")
else:
logger.info("auto_merge_only_test_files disabled, skipping file check")

# All conditions met - merge the PR
merge_method = cast(MergeMethod, repo_features.get("merge_method", "merge"))
Expand Down Expand Up @@ -275,3 +334,6 @@ def handle_successful_check_suite(payload: CheckSuiteCompletedPayload):
create_comment(body=msg, base_args=comment_args)
slack_msg = f"`{owner_name}/{repo_name}` PR #{pr_number}: {msg}"
slack_notify(slack_msg)
return

logger.info("Auto-merge succeeded for PR #%s", pr_number)
Loading