From 70f8e13c4c28af9d5e9b4db3ed3925315bc53f4a Mon Sep 17 00:00:00 2001 From: GISCE Bot Date: Tue, 26 May 2026 19:57:30 +0000 Subject: [PATCH 1/3] fix: support forwarded github notifications --- src/github_agent_bridge/cli.py | 4 +-- src/github_agent_bridge/parser.py | 46 +++++++++++++++++++++++++------ src/github_agent_bridge/policy.py | 11 ++++++-- src/github_agent_bridge/queue.py | 4 +-- src/github_agent_bridge/reader.py | 4 +-- tests/test_manual_enqueue_cli.py | 28 ++++++++++++++++++- tests/test_parser.py | 16 +++++------ tests/test_policy.py | 25 +++++++++++++++++ tests/test_queue.py | 2 +- 9 files changed, 112 insertions(+), 28 deletions(-) diff --git a/src/github_agent_bridge/cli.py b/src/github_agent_bridge/cli.py index e4511f2..fb084ff 100644 --- a/src/github_agent_bridge/cli.py +++ b/src/github_agent_bridge/cli.py @@ -18,7 +18,7 @@ from .models import Notification, utc_now from .monitor import MonitorThresholds, monitor, report_json from .observability import DEFAULT_PROCESS_SAMPLE_RETENTION_SECONDS -from .parser import decode_header_value, extract_body_text, parse_auth_results +from .parser import decode_header_value, extract_body_text, is_github_notification_message, parse_auth_results from .policy import Policy from .queue import JobQueue from .reader import ImapConfig, ImapReader, imap_mailbox_arg @@ -34,7 +34,7 @@ def load_policy(path: str | None) -> Policy: def msg_to_notification(msg, uid: int | None = None) -> Notification | None: from_addr = decode_header_value(msg.get("From", "")) - if "notifications@github.com" not in from_addr.lower(): + if not is_github_notification_message(msg, from_addr): return None return Notification( uid=uid, diff --git a/src/github_agent_bridge/parser.py b/src/github_agent_bridge/parser.py index 19a3b20..0b035de 100644 --- a/src/github_agent_bridge/parser.py +++ b/src/github_agent_bridge/parser.py @@ -8,9 +8,9 @@ REVIEW_ONLY_PATTERNS = ("fes-ne una review", "fes una review", "fes review", "fer una review", "fes-ne una revisio", "fes-ne una revisió", "fes una revisio", "fes una revisió", "fer una revisio", "fer una revisió", "review de la pr", "revisió de la pr", "revisio de la pr", "revisa aquesta pr", "revisa els canvis", "revisar els canvis", "com veus els canvis", "què et semblen els canvis", "que et semblen els canvis", "what do you think of these changes", "please review", "can you review") IMPLEMENTATION_PATTERNS = ("fes els canvis", "fes-ho", "implementa", "modifica", "canvia", "arregla", "corregeix", "fix", "push", "commit", "aplica", "resol", "resolve") -BOT_MENTION_PATTERNS = ("@pilipilisbot", "pilipilisbot", "you are receiving this because you were mentioned") -ASSIGNMENT_PATTERNS = ("assigned you", "assigned to you", "you were assigned", "you are assigned", "assigned pilipilisbot", "assigned @pilipilisbot") -REVIEW_REQUEST_PATTERNS = ("requested your review", "requested a review from you", "you were requested for review", "review requested", "requested review from pilipilisbot", "requested review from @pilipilisbot", "requested @pilipilisbot") +BOT_MENTION_PATTERNS = ("you are receiving this because you were mentioned",) +ASSIGNMENT_PATTERNS = ("assigned you", "assigned to you", "you were assigned", "you are assigned") +REVIEW_REQUEST_PATTERNS = ("requested your review", "requested a review from you", "you were requested for review", "review requested") COPILOT_REVIEW_PATTERNS = ("copilot-pull-request-reviewer", "github-copilot", "github copilot", "copilot reviewed", "copilot commented", "copilot left a comment", "copilot suggested", "copilot requested changes") WORKFLOW_RUN_FAILED_PATTERNS = ("run failed", "workflow run failed", "workflow failed", "job failed", "failing after") @@ -37,13 +37,41 @@ def parse_auth_results(msg: Message) -> dict[str, bool]: return {"spf": "spf=pass" in raw, "dkim": "dkim=pass" in raw, "dmarc": "dmarc=pass" in raw} +def is_github_notification_message(msg: Message, from_addr: str | None = None) -> bool: + """Return True for direct GitHub notifications and Google Groups rewrites. + + GISCE routes GitHub mail through a Google Group, so incoming notifications can + arrive as `From: ... via GISCE Bot ` while retaining the + GitHub reply address, message id and X-GitHub headers. + """ + sender = (from_addr or decode_header_value(msg.get("From", ""))).lower() + if "notifications@github.com" in sender: + return True + reply_to = decode_header_value(msg.get("Reply-To", "")).lower() + message_id = decode_header_value(msg.get("Message-ID", "")).lower() + return ( + bool(msg.get("X-GitHub-Recipient")) + and bool(msg.get("X-GitHub-Reason")) + and "@reply.github.com" in reply_to + and "github.com" in message_id + ) + + def _contains_any(text: str, patterns: tuple[str, ...]) -> bool: return any(p in text for p in patterns) -def github_event_flags(subject: str, body: str) -> dict[str, bool]: +def _bot_patterns(bot_logins: set[str] | None) -> tuple[str, ...]: + names = sorted({login.lower().lstrip("@") for login in (bot_logins or set()) if login.strip()}) + return tuple(pattern for name in names for pattern in (f"@{name}", name)) + + +def github_event_flags(subject: str, body: str, bot_logins: set[str] | None = None) -> dict[str, bool]: text = f"{subject}\n{body}".lower() - return {"bot_mentioned": _contains_any(text, BOT_MENTION_PATTERNS), "assigned": _contains_any(text, ASSIGNMENT_PATTERNS), "review_requested": _contains_any(text, REVIEW_REQUEST_PATTERNS), "copilot_review": _contains_any(text, COPILOT_REVIEW_PATTERNS)} + bot_patterns = _bot_patterns(bot_logins) + assignment_patterns = ASSIGNMENT_PATTERNS + tuple(f"assigned {p}" for p in bot_patterns) + review_patterns = REVIEW_REQUEST_PATTERNS + tuple(f"requested review from {p}" for p in bot_patterns) + tuple(f"requested {p}" for p in bot_patterns) + return {"bot_mentioned": _contains_any(text, BOT_MENTION_PATTERNS + bot_patterns), "assigned": _contains_any(text, assignment_patterns), "review_requested": _contains_any(text, review_patterns), "copilot_review": _contains_any(text, COPILOT_REVIEW_PATTERNS)} def _looks_like_pr_thread(subject: str, body: str) -> bool: @@ -51,9 +79,9 @@ def _looks_like_pr_thread(subject: str, body: str) -> bool: return bool(re.search(r"\bpr #\d+\b|\bpull request #\d+\b", text) or re.search(r"github\.com/[^/]+/[^/]+/pull/\d+", text)) -def classify_work_intent(subject: str, body: str) -> str: +def classify_work_intent(subject: str, body: str, bot_logins: set[str] | None = None) -> str: text = f"{subject}\n{body}".lower() - flags = github_event_flags(subject, body) + flags = github_event_flags(subject, body, bot_logins) asks_review = flags["review_requested"] or _contains_any(text, REVIEW_ONLY_PATTERNS) asks_implementation = flags["assigned"] or _contains_any(text, IMPLEMENTATION_PATTERNS) if asks_review and not asks_implementation: @@ -66,9 +94,9 @@ def classify_work_intent(subject: str, body: str) -> str: return "work_allowed" -def classify_github_action(subject: str, body: str) -> str: +def classify_github_action(subject: str, body: str, bot_logins: set[str] | None = None) -> str: text = f"{subject}\n{body}".lower() - flags = github_event_flags(subject, body) + flags = github_event_flags(subject, body, bot_logins) if re.search(r"github\.com/[^/]+/[^/]+/actions/runs/\d+", text) and _contains_any(text, WORKFLOW_RUN_FAILED_PATTERNS): return "workflow_run_failed" if "merged" in text: diff --git a/src/github_agent_bridge/policy.py b/src/github_agent_bridge/policy.py index 350a71f..c65c4e6 100644 --- a/src/github_agent_bridge/policy.py +++ b/src/github_agent_bridge/policy.py @@ -60,7 +60,7 @@ class FeedbackLearning: @dataclass(frozen=True) class Policy: - source_from: str = "notifications@github.com" + source_from: str | tuple[str, ...] = "notifications@github.com" required_url_prefix: str = "https://github.com/" message_id_domain: str = "github.com" trusted_repos: set[str] = field(default_factory=set) @@ -73,6 +73,7 @@ class Policy: org_routes: dict[str, Route] = field(default_factory=dict) repo_roles: dict[str, str] = field(default_factory=dict) org_roles: dict[str, str] = field(default_factory=dict) + bot_logins: set[str] = field(default_factory=set) prompt_overrides: PromptOverrides = field(default_factory=PromptOverrides) feedback_learning: FeedbackLearning = field(default_factory=FeedbackLearning) @@ -150,7 +151,7 @@ def feedback_learning(raw: dict) -> FeedbackLearning: ) return cls( - source_from=source.get("from", cls.source_from), + source_from=tuple(source.get("from")) if isinstance(source.get("from"), list) else source.get("from", cls.source_from), required_url_prefix=source.get("requiredUrlPrefix", cls.required_url_prefix), message_id_domain=source.get("messageIdDomain", cls.message_id_domain), trusted_repos={r.lower() for r in data.get("trustedRepos", [])}, @@ -161,13 +162,17 @@ def feedback_learning(raw: dict) -> FeedbackLearning: trusted_auto_actions=set(actions.get("trustedAuto", ["reply_comment", "open_issue", "submit_review", "sync_after_merge", "workflow_run_failed"])), repo_routes=routes(data.get("repoRoutes", {})), org_routes=routes(data.get("orgRoutes", {})), repo_roles=roles(data.get("repoRoles", {})), org_roles=roles(data.get("orgRoles", {})), + bot_logins={str(login).lower().lstrip("@") for login in data.get("botLogins", []) if str(login).strip()}, prompt_overrides=prompt_overrides(data.get("promptOverrides", {})), feedback_learning=feedback_learning(data.get("feedbackLearning", {})), ) def trusted_source(self, n: Notification, ctx: GitHubContext) -> bool: auth_ok = all(bool(n.auth.get(k)) for k in ("spf", "dkim", "dmarc")) if n.auth else True - return self.source_from in n.from_addr and auth_ok and any(u.startswith(self.required_url_prefix) for u in ctx.urls) and self.message_id_domain in n.message_id + sources = (self.source_from,) if isinstance(self.source_from, str) else self.source_from + from_addr = n.from_addr.lower() + source_ok = any(str(source).lower() in from_addr for source in sources) + return source_ok and auth_ok and any(u.startswith(self.required_url_prefix) for u in ctx.urls) and self.message_id_domain in n.message_id def repo_trusted(self, repo: str | None) -> bool: if not repo: diff --git a/src/github_agent_bridge/queue.py b/src/github_agent_bridge/queue.py index 5d0903c..ae57d89 100644 --- a/src/github_agent_bridge/queue.py +++ b/src/github_agent_bridge/queue.py @@ -44,8 +44,8 @@ def init(self) -> None: def enqueue(self, n: Notification, policy: Policy) -> tuple[Job | None, str]: ctx = extract_github_context(n.body) - action = classify_github_action(n.subject, n.body) - intent = classify_work_intent(n.subject, n.body) + action = classify_github_action(n.subject, n.body, policy.bot_logins) + intent = classify_work_intent(n.subject, n.body, policy.bot_logins) decision = policy.decision(n, ctx, action) status = {"auto": "done", "ask": "waiting_approval", "deny": "denied"}.get(decision, "pending") now = utc_now() diff --git a/src/github_agent_bridge/reader.py b/src/github_agent_bridge/reader.py index 6b267f1..d5ab2b0 100644 --- a/src/github_agent_bridge/reader.py +++ b/src/github_agent_bridge/reader.py @@ -5,7 +5,7 @@ from dataclasses import dataclass from .models import Notification -from .parser import decode_header_value, extract_body_text, parse_auth_results +from .parser import decode_header_value, extract_body_text, is_github_notification_message, parse_auth_results from .policy import Policy from .queue import JobQueue @@ -57,7 +57,7 @@ def fetch_once(self) -> int: from_addr = decode_header_value(msg.get("From", "")) subject = decode_header_value(msg.get("Subject", "")) message_id = decode_header_value(msg.get("Message-ID", "")) - if "notifications@github.com" in from_addr.lower(): + if is_github_notification_message(msg, from_addr): n = Notification(uid=uid, message_id=message_id, subject=subject, from_addr=from_addr, body=extract_body_text(msg), auth=parse_auth_results(msg)) self.queue.enqueue(n, self.policy) # Only GitHub notifications belong to this bounded context. diff --git a/tests/test_manual_enqueue_cli.py b/tests/test_manual_enqueue_cli.py index 313b062..2ec4a1f 100644 --- a/tests/test_manual_enqueue_cli.py +++ b/tests/test_manual_enqueue_cli.py @@ -1,6 +1,7 @@ import json +from email.message import EmailMessage -from github_agent_bridge.cli import _parse_github_comment_url, notification_from_comment_url +from github_agent_bridge.cli import _parse_github_comment_url, msg_to_notification, notification_from_comment_url from github_agent_bridge.parser import extract_github_context @@ -32,3 +33,28 @@ def fake_gh(args, gh_bin="gh"): assert ctx.repo == "gisce/erp" assert ctx.issue_number == 27675 assert ctx.comment_id == 4419572864 + + +def test_giscebot_mention_classifies_as_reply_comment(): + from github_agent_bridge.parser import classify_github_action + + body = "@giscebot pots mirar això?\nhttps://github.com/gisce/erp/pull/27675#issuecomment-4419572864" + + assert classify_github_action("Re: [gisce/erp] Example (PR #27675)", body, {"giscebot"}) == "reply_comment" + + +def test_msg_to_notification_accepts_google_group_rewritten_github_mail(): + msg = EmailMessage() + msg["From"] = "'Eduard Carreras' via GISCE Bot " + msg["Reply-To"] = "gisce/erp " + msg["Message-ID"] = "" + msg["Subject"] = "Re: [gisce/erp] Example (PR #27853)" + msg["X-GitHub-Recipient"] = "giscebot" + msg["X-GitHub-Reason"] = "mention" + msg.set_content("https://github.com/gisce/erp/pull/27853#issuecomment-4547966148") + + n = msg_to_notification(msg, uid=6) + + assert n is not None + assert n.uid == 6 + assert n.from_addr == "'Eduard Carreras' via GISCE Bot " diff --git a/tests/test_parser.py b/tests/test_parser.py index 0695a70..5312235 100644 --- a/tests/test_parser.py +++ b/tests/test_parser.py @@ -46,7 +46,7 @@ def test_extract_pr_comment_context_before_workflow_run_link(): def test_mentions_are_actionable(): - assert classify_github_action("Re: [x] PR", "@pilipilisbot fes-ho") == "reply_comment" + assert classify_github_action("Re: [x] PR", "@pilipilisbot fes-ho", {"pilipilisbot"}) == "reply_comment" assert classify_github_action("Re: [x] PR", "You are receiving this because you were mentioned.") == "reply_comment" @@ -71,23 +71,23 @@ def test_review_request_uses_formal_review_flow(): subject = "Re: [gisce/erp] Permitir caller en los dominios (PR #27315)" body = "ecarreras requested review from @pilipilisbot on this pull request." - assert classify_github_action(subject, body) == "submit_review" - assert classify_work_intent(subject, body) == "review_only" + assert classify_github_action(subject, body, {"pilipilisbot"}) == "submit_review" + assert classify_work_intent(subject, body, {"pilipilisbot"}) == "review_only" def test_pr_review_followup_is_read_only_without_explicit_implementation(): subject = "Re: [gisce/erp] Permitir caller en los dominios (PR #27315)" body = "@pilipilisbot però la transacció en què s'executa que entra per eval_domain és amb una transacció readonly" - assert classify_github_action(subject, body) == "reply_comment" - assert classify_work_intent(subject, body) == "review_only" + assert classify_github_action(subject, body, {"pilipilisbot"}) == "reply_comment" + assert classify_work_intent(subject, body, {"pilipilisbot"}) == "review_only" def test_pr_followup_can_still_request_explicit_implementation(): subject = "Re: [gisce/erp] Permitir caller en los dominios (PR #27315)" body = "@pilipilisbot aplica el canvi i fes push" - assert classify_github_action(subject, body) == "reply_comment" + assert classify_github_action(subject, body, {"pilipilisbot"}) == "reply_comment" assert classify_work_intent(subject, body) == "work_allowed" @@ -95,5 +95,5 @@ def test_pr_assignment_allows_work(): subject = "Re: [gisce/erp] Permitir caller en los dominios (PR #27315)" body = "ecarreras assigned @pilipilisbot to this pull request." - assert classify_github_action(subject, body) == "open_issue" - assert classify_work_intent(subject, body) == "work_allowed" + assert classify_github_action(subject, body, {"pilipilisbot"}) == "open_issue" + assert classify_work_intent(subject, body, {"pilipilisbot"}) == "work_allowed" diff --git a/tests/test_policy.py b/tests/test_policy.py index 1ba4a94..afd80cd 100644 --- a/tests/test_policy.py +++ b/tests/test_policy.py @@ -10,6 +10,22 @@ def test_trusted_org_auto_trusted(): assert Policy(trusted_orgs={"gisce"}).decision(n, ctx, "reply_comment") == "auto_trusted" +def test_trusted_source_accepts_configured_forwarder(): + body = "@giscebot https://github.com/gisce/erp/pull/27853#issuecomment-4547966148" + n = Notification( + 1, + "", + "subj", + "'Eduard Carreras' via GISCE Bot ", + body, + auth={"spf": True, "dkim": True, "dmarc": True}, + ) + ctx = extract_github_context(body) + policy = Policy(source_from=("notifications@github.com", "giscebot@gisce.net"), trusted_orgs={"gisce"}) + + assert policy.decision(n, ctx, "reply_comment") == "auto_trusted" + + def test_enabled_repos_restricts_canary_scope(): n = Notification(1, "", "subj", "notifications@github.com", "", auth={"spf": True, "dkim": True, "dmarc": True}) erp = extract_github_context("@pilipilisbot https://github.com/gisce/erp/issues/1#issuecomment-1") @@ -46,6 +62,15 @@ def test_policy_from_file_loads_roles_and_rejects_unknown(tmp_path): raise AssertionError("expected ValueError for unknown repo role") +def test_policy_from_file_loads_bot_logins(tmp_path): + policy_file = tmp_path / "policy.json" + policy_file.write_text('{"botLogins": ["@GISCEBot", "pilipilisbot"]}') + + policy = Policy.from_file(policy_file) + + assert policy.bot_logins == {"giscebot", "pilipilisbot"} + + def test_policy_from_file_loads_feedback_learning(tmp_path): policy_file = tmp_path / "policy.json" policy_file.write_text('{"feedbackLearning": {"enabled": false, "minConfidence": 0.7, "autoApproveConfidence": 0.9, "maxEventsPerRun": 3, "model": "test-model", "thinking": "medium", "sessionId": "feedback-test"}}') diff --git a/tests/test_queue.py b/tests/test_queue.py index 584d07c..1a653fa 100644 --- a/tests/test_queue.py +++ b/tests/test_queue.py @@ -12,7 +12,7 @@ def notif(uid, mid, body): def policy(): - return Policy(trusted_orgs={"gisce"}) + return Policy(trusted_orgs={"gisce"}, bot_logins={"pilipilisbot"}) def test_enqueue_and_coalesce_same_work_key(tmp_path, monkeypatch): From 80dfee9481dc8cdc011bc144ee50552ea3e21e43 Mon Sep 17 00:00:00 2001 From: GISCE Bot Date: Tue, 26 May 2026 20:42:24 +0000 Subject: [PATCH 2/3] fix: preserve default bot login policy --- README.md | 4 +++ docs/policy-reference.md | 46 +++++++++++++++++++++++++++++-- policy.example.json | 8 +++++- src/github_agent_bridge/policy.py | 9 ++++-- tests/test_policy.py | 15 ++++++++++ 5 files changed, 76 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 5a7fbf5..2a245b2 100644 --- a/README.md +++ b/README.md @@ -97,6 +97,10 @@ The bridge is conservative by default. `policy.json` decides what is trusted, wh ```json { + "source": { + "from": ["notifications@github.com", "giscebot@gisce.net"] + }, + "botLogins": ["pilipilisbot"], "trustedOrgs": ["your-org"], "enabledRepos": ["your-org/your-repo"], "orgRoutes": { diff --git a/docs/policy-reference.md b/docs/policy-reference.md index 9e92fa9..3353656 100644 --- a/docs/policy-reference.md +++ b/docs/policy-reference.md @@ -44,11 +44,12 @@ gab --policy ~/.config/github-agent-bridge/policy.json enqueue-comment-url ... ```json { "source": { - "from": "notifications@github.com", + "from": ["notifications@github.com", "giscebot@gisce.net"], "requiredAuth": ["spf=pass", "dkim=pass", "dmarc=pass"], "requiredUrlPrefix": "https://github.com/", "messageIdDomain": "github.com" }, + "botLogins": ["pilipilisbot"], "trustedRepos": ["your-org/your-repo"], "trustedOrgs": ["your-org"], "enabledRepos": ["your-org/your-repo"], @@ -97,6 +98,7 @@ gab --policy ~/.config/github-agent-bridge/policy.json enqueue-comment-url ... | `orgRoutes` | object | `{}` | Per-owner delivery routes used when no `repoRoutes` entry matches. | | `repoRoles` | object | `{}` | Exact per-repo operating role. Takes precedence over `orgRoles`. | | `orgRoles` | object | `{}` | Per-owner operating role used when no `repoRoles` entry matches. | +| `botLogins` | array of strings | `["pilipilisbot"]` | GitHub login names that should count as addressed bots when classifying mentions, assignments, and review requests. | | `actions` | object | built-in action defaults | Maps classified notification actions to policy decisions. | | `promptOverrides` | object | `{}` | Optional Markdown files that replace selected packaged prompt resources. | | `feedbackLearning` | object | `{ "enabled": true, "minConfidence": 0.5, "autoApproveConfidence": 0.8 }` | Controls candidate capture, autonomous learning, and prompt threshold for feedback rules. | @@ -109,7 +111,7 @@ Unknown top-level keys are ignored by the current implementation. | Key | Type | Default | Meaning | | --- | --- | --- | --- | -| `from` | string | `notifications@github.com` | Required substring in the decoded email `From` header. | +| `from` | string or array of strings | `notifications@github.com` | Required substring in the decoded email `From` header. Use an array when GitHub notifications are forwarded or rewritten by a trusted mail gateway while GitHub reply headers and message ids are preserved. | | `requiredUrlPrefix` | string | `https://github.com/` | At least one extracted URL must start with this prefix. | | `messageIdDomain` | string | `github.com` | Required substring in the email `Message-ID`. | | `requiredAuth` | array of strings | currently documented only | Intended SPF/DKIM/DMARC requirements. See note below. | @@ -123,7 +125,7 @@ Current auth behavior: Source trust fails when any of these are false: ```text -source.from is in From header +any configured source.from value is in From header AND auth is OK AND at least one GitHub URL has source.requiredUrlPrefix AND Message-ID contains source.messageIdDomain @@ -131,6 +133,44 @@ AND Message-ID contains source.messageIdDomain If source trust fails, the decision is always `deny`. +Example with Google Groups or similar forwarded GitHub notifications: + +```json +{ + "source": { + "from": ["notifications@github.com", "giscebot@gisce.net"], + "requiredUrlPrefix": "https://github.com/", + "messageIdDomain": "github.com" + } +} +``` + +The parser still requires GitHub-specific headers, a GitHub reply address, GitHub message id content, and normal source trust before forwarded messages are accepted. + +## `botLogins` + +`botLogins` defines the GitHub accounts that count as the addressed bot for mention, assignment, and review-request classification. + +Default: + +```json +{ + "botLogins": ["pilipilisbot"] +} +``` + +Configured names are case-insensitive and may include or omit the leading `@`. + +Example: + +```json +{ + "botLogins": ["pilipilisbot", "giscebot"] +} +``` + +With this policy, comments that mention `@giscebot`, assignments to `@giscebot`, and review requests from `@giscebot` are classified the same way as the default `@pilipilisbot` notifications. Set an explicit empty array only if the deployment should rely on GitHub footer text such as “You are receiving this because you were mentioned” instead of login matching. + ## `trustedRepos` Exact repositories trusted for `trustedAuto` actions. diff --git a/policy.example.json b/policy.example.json index 24940cd..5f7edf0 100644 --- a/policy.example.json +++ b/policy.example.json @@ -1,6 +1,9 @@ { "source": { - "from": "notifications@github.com", + "from": [ + "notifications@github.com", + "giscebot@gisce.net" + ], "requiredAuth": [ "spf=pass", "dkim=pass", @@ -10,6 +13,9 @@ "messageIdDomain": "github.com" }, "trustedRepos": [], + "botLogins": [ + "pilipilisbot" + ], "trustedOrgs": [ "your-org" ], diff --git a/src/github_agent_bridge/policy.py b/src/github_agent_bridge/policy.py index c65c4e6..20c2ea1 100644 --- a/src/github_agent_bridge/policy.py +++ b/src/github_agent_bridge/policy.py @@ -21,6 +21,7 @@ "worktree", } DEFAULT_REPO_ROLE = "contributor" +DEFAULT_BOT_LOGINS = frozenset({"pilipilisbot"}) @dataclass(frozen=True) @@ -73,7 +74,7 @@ class Policy: org_routes: dict[str, Route] = field(default_factory=dict) repo_roles: dict[str, str] = field(default_factory=dict) org_roles: dict[str, str] = field(default_factory=dict) - bot_logins: set[str] = field(default_factory=set) + bot_logins: set[str] = field(default_factory=lambda: set(DEFAULT_BOT_LOGINS)) prompt_overrides: PromptOverrides = field(default_factory=PromptOverrides) feedback_learning: FeedbackLearning = field(default_factory=FeedbackLearning) @@ -162,7 +163,11 @@ def feedback_learning(raw: dict) -> FeedbackLearning: trusted_auto_actions=set(actions.get("trustedAuto", ["reply_comment", "open_issue", "submit_review", "sync_after_merge", "workflow_run_failed"])), repo_routes=routes(data.get("repoRoutes", {})), org_routes=routes(data.get("orgRoutes", {})), repo_roles=roles(data.get("repoRoles", {})), org_roles=roles(data.get("orgRoles", {})), - bot_logins={str(login).lower().lstrip("@") for login in data.get("botLogins", []) if str(login).strip()}, + bot_logins=( + {str(login).lower().lstrip("@") for login in data.get("botLogins", []) if str(login).strip()} + if "botLogins" in data + else set(DEFAULT_BOT_LOGINS) + ), prompt_overrides=prompt_overrides(data.get("promptOverrides", {})), feedback_learning=feedback_learning(data.get("feedbackLearning", {})), ) diff --git a/tests/test_policy.py b/tests/test_policy.py index afd80cd..e792025 100644 --- a/tests/test_policy.py +++ b/tests/test_policy.py @@ -71,6 +71,21 @@ def test_policy_from_file_loads_bot_logins(tmp_path): assert policy.bot_logins == {"giscebot", "pilipilisbot"} +def test_policy_keeps_default_bot_login_when_not_configured(tmp_path): + policy_file = tmp_path / "policy.json" + policy_file.write_text("{}") + + assert Policy().bot_logins == {"pilipilisbot"} + assert Policy.from_file(policy_file).bot_logins == {"pilipilisbot"} + + +def test_policy_allows_explicit_empty_bot_logins(tmp_path): + policy_file = tmp_path / "policy.json" + policy_file.write_text('{"botLogins": []}') + + assert Policy.from_file(policy_file).bot_logins == set() + + def test_policy_from_file_loads_feedback_learning(tmp_path): policy_file = tmp_path / "policy.json" policy_file.write_text('{"feedbackLearning": {"enabled": false, "minConfidence": 0.7, "autoApproveConfidence": 0.9, "maxEventsPerRun": 3, "model": "test-model", "thinking": "medium", "sessionId": "feedback-test"}}') From e26c41b78a0e7f3d0a3452b8d7e91cc56ab81bb2 Mon Sep 17 00:00:00 2001 From: GISCE Bot Date: Tue, 26 May 2026 21:27:25 +0000 Subject: [PATCH 3/3] fix: treat approved reviews as non-actionable --- src/github_agent_bridge/dispatch.py | 3 +++ tests/test_github_followup_detection.py | 22 ++++++++++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/src/github_agent_bridge/dispatch.py b/src/github_agent_bridge/dispatch.py index 666a5cb..b1eaeff 100644 --- a/src/github_agent_bridge/dispatch.py +++ b/src/github_agent_bridge/dispatch.py @@ -131,6 +131,9 @@ def is_non_actionable_review(self, ctx: GitHubContext) -> bool: review = self.pull_request_review(ctx) if not review: return False + state = (review.get("state") or "").upper() + if state == "APPROVED": + return True body = (review.get("body") or "").lower() non_actionable_markers = ( "generated no new comments", diff --git a/tests/test_github_followup_detection.py b/tests/test_github_followup_detection.py index 48eb7ca..780cf79 100644 --- a/tests/test_github_followup_detection.py +++ b/tests/test_github_followup_detection.py @@ -65,6 +65,28 @@ def test_visible_followup_ignores_review_comment_before_trigger(): assert github.visible_followup_after_trigger(ctx) is None +def test_approved_review_is_non_actionable(): + ctx = GitHubContext( + urls=["https://github.com/gisce/erp/pull/27805#pullrequestreview-4325056741"], + repo="gisce/erp", + issue_number=27805, + review_id=4325056741, + ) + github = RecordingGitHubClient( + { + ("api", "repos/gisce/erp/pulls/27805/reviews/4325056741"): json.dumps( + { + "state": "APPROVED", + "body": "Looks good after the follow-up commit.", + "submitted_at": "2026-05-20T03:59:00Z", + } + ), + } + ) + + assert github.is_non_actionable_review(ctx) is True + + def test_visible_followup_for_issue_comment_returns_newest_bot_comment_after_trigger(): ctx = GitHubContext( urls=["https://github.com/pilipilisbot/github-agent-bridge/pull/13#issuecomment-4524715895"],