agent_shin: fix bug bundle (gated LLM key, author-filtered marker dedup, dedup gh/grace helpers)

Co-authored-by: Yassin Kortam <yassin@berri.ai>

.github/scripts/agent_shin_shared.py

@@ -27,7 +27,9 @@ enforced it.
 from __future__ import annotations
 
 import datetime as dt
+import os
 import re
+import subprocess
 from typing import Iterable
 
 GREPTILE_BOT_LOGINS = frozenset({"greptile-apps", "greptile-apps[bot]"})
@@ -77,3 +79,67 @@ def extract_greptile_score(comments: Iterable[dict]) -> tuple[int, dict] | None:
 def parse_iso8601(value: str) -> dt.datetime:
     """Parse a GitHub ISO-8601 timestamp into a timezone-aware datetime."""
     return dt.datetime.fromisoformat(value.replace("Z", "+00:00"))
+
+
+def gh(*args: str) -> str:
+    """Run a `gh` CLI command and return stdout. Raises on non-zero exit.
+
+    Shared by both Agent Shin entrypoints so a future change here
+    (timeout handling, logging, retry on transient failures) only needs
+    to be made once.
+    """
+    result = subprocess.run(
+        ["gh", *args],
+        capture_output=True,
+        text=True,
+        check=True,
+    )
+    return result.stdout
+
+
+def seconds_since_latest_marker_comment(
+    comments: Iterable[dict],
+    *,
+    marker: str,
+    bot_login: str | None = None,
+    now: dt.datetime | None = None,
+) -> float | None:
+    """Return seconds since the bot's most recent comment containing ``marker``.
+
+    Filters comments by author so a contributor who quotes the HTML
+    marker (e.g. via GitHub's "Quote reply" feature, which preserves
+    HTML comments in the raw markdown of the quoted text) is not
+    mistaken for a bot warning — that would silently reset cooldown
+    timers and suppress legitimate notifications.
+
+    ``bot_login`` defaults to the `AGENT_SHIN_BOT_LOGIN` env override or
+    ``AGENT_SHIN_DEFAULT_BOT_LOGIN`` so callers normally don't need to
+    pass it. ``now`` is injectable for tests / callers (like the daily
+    sweep) that want every age calculation pinned to one snapshot.
+    """
+    expected_login = (
+        bot_login
+        or os.environ.get("AGENT_SHIN_BOT_LOGIN")
+        or AGENT_SHIN_DEFAULT_BOT_LOGIN
+    ).lower()
+    latest: dt.datetime | None = None
+    for comment in comments:
+        author = ((comment.get("user") or {}).get("login") or "").lower()
+        if author != expected_login:
+            continue
+        body = comment.get("body") or ""
+        if marker not in body:
+            continue
+        created = comment.get("created_at")
+        if not created:
+            continue
+        try:
+            ts = parse_iso8601(created)
+        except ValueError:
+            continue
+        if latest is None or ts > latest:
+            latest = ts
+    if latest is None:
+        return None
+    reference = now if now is not None else dt.datetime.now(dt.timezone.utc)
+    return (reference - latest).total_seconds()

.github/scripts/close_low_quality_prs.py

@@ -49,14 +49,15 @@ from typing import Iterable
 sys.path.insert(0, os.path.dirname(os.path.abspath(__file__)))
 
 from agent_shin_shared import (  # noqa: E402  -- sys.path adjusted above
-    AGENT_SHIN_DEFAULT_BOT_LOGIN,
     GRACE_COMMENT_MARKER,
     GRACE_PERIOD_SECONDS,
     GREPTILE_BOT_LOGINS,
     IMMEDIATE_CLOSE_LOGINS,
     SCORE_PATTERN,
     extract_greptile_score,
+    gh,
     parse_iso8601,
+    seconds_since_latest_marker_comment,
 )
 
 # `GREPTILE_BOT_LOGINS` and `SCORE_PATTERN` (Greptile's GitHub App login
@@ -82,24 +83,13 @@ DEFAULT_OPTOUT_LABELS = ("do not close", "keep open", "wip")
 # imported from `agent_shin_shared` so the Agent Shin LLM judge and
 # this daily Greptile sweep agree on the same marker and duration.
 #
-# `AGENT_SHIN_DEFAULT_BOT_LOGIN` (the GitHub identity that performs
-# Agent Shin's writes; used for matching the author of a grace-warning
-# comment so we don't count somebody quoting the marker; the env
-# override `AGENT_SHIN_BOT_LOGIN` works the same here) and
 # `IMMEDIATE_CLOSE_LOGINS` (case-insensitive logins that bypass BOTH
 # the grace period AND the dry-run gating; useful for dogfooding the
-# bot from external test accounts) are imported too.
-
-
-def gh(*args: str) -> str:
-    """Run a `gh` CLI command and return stdout. Raises on non-zero exit."""
-    result = subprocess.run(
-        ["gh", *args],
-        capture_output=True,
-        text=True,
-        check=True,
-    )
-    return result.stdout
+# bot from external test accounts) is imported too. Author-matching
+# against the bot login (`AGENT_SHIN_DEFAULT_BOT_LOGIN` plus the
+# `AGENT_SHIN_BOT_LOGIN` env override) now happens inside
+# `seconds_since_latest_marker_comment` itself, so this script no
+# longer needs to reach for the constant directly.
 
 
 def fetch_open_prs(repo: str | None) -> list[dict]:
@@ -196,42 +186,19 @@ def seconds_since_last_grace_warning(
     """Return seconds since the bot's most recent grace-period warning, or
     None if no such warning has ever been posted on this PR.
 
-    Detects warnings by matching `GRACE_COMMENT_MARKER` in comments
-    authored by the bot identity. Operates on an already-fetched
-    comments list (avoids a second `gh api` call when the caller has
-    already pulled the page for Greptile-score extraction).
-
-    `now` is injectable so callers (and tests) can pin the reference
-    time. The closer runs everything against a single `now` snapshot
-    captured at the top of `main()` so age calculations stay consistent
-    across many PRs in a single run.
+    Thin wrapper over
+    `agent_shin_shared.seconds_since_latest_marker_comment` — the
+    centralized helper handles the bot-author filter, marker match,
+    timestamp parsing, and `now` injection. Keeping this wrapper
+    preserves the closer's "already-fetched comments + injectable now"
+    interface so callers (and tests) don't need to change.
     """
-    expected_login = (
-        bot_login
-        or os.environ.get("AGENT_SHIN_BOT_LOGIN")
-        or AGENT_SHIN_DEFAULT_BOT_LOGIN
-    ).lower()
-    latest: dt.datetime | None = None
-    for comment in comments:
-        author = ((comment.get("user") or {}).get("login") or "").lower()
-        if author != expected_login:
-            continue
-        body = comment.get("body") or ""
-        if GRACE_COMMENT_MARKER not in body:
-            continue
-        created = comment.get("created_at")
-        if not created:
-            continue
-        try:
-            ts = parse_iso8601(created)
-        except ValueError:
-            continue
-        if latest is None or ts > latest:
-            latest = ts
-    if latest is None:
-        return None
-    reference = now if now is not None else dt.datetime.now(dt.timezone.utc)
-    return (reference - latest).total_seconds()
+    return seconds_since_latest_marker_comment(
+        comments,
+        marker=GRACE_COMMENT_MARKER,
+        bot_login=bot_login,
+        now=now,
+    )
 
 
 def format_grace_warning_comment(score: int, threshold: int) -> str:

.github/scripts/triage_with_llm.py

@@ -55,7 +55,9 @@ from agent_shin_shared import (  # noqa: E402  -- sys.path adjusted above
     IMMEDIATE_CLOSE_LOGINS,
     SCORE_PATTERN,
     extract_greptile_score,
+    gh,
     parse_iso8601,
+    seconds_since_latest_marker_comment,
 )
 
 DEFAULT_MODEL = "gpt-5.4-mini"
@@ -152,17 +154,9 @@ HTML_COMMENT_PATTERN = re.compile(r"<!--.*?-->", re.DOTALL)
 
 # ---------------------------------------------------------------------------
 # gh helpers
-
-
-def gh(*args: str) -> str:
-    """Run a `gh` CLI command and return stdout. Raises on non-zero exit."""
-    result = subprocess.run(
-        ["gh", *args],
-        capture_output=True,
-        text=True,
-        check=True,
-    )
-    return result.stdout
+#
+# `gh` is imported from `agent_shin_shared` so a future change (timeout,
+# logging, retry) only needs to be made once.
 
 
 def fetch_pr(repo: str, number: int) -> dict:
@@ -339,39 +333,22 @@ def _seconds_since_latest_marker_comment(
     marker: str,
     bot_login: str | None = None,
 ) -> float | None:
-    """Shared helper: return seconds since the bot's most recent comment
-    that contains the given HTML marker, or None if no such comment exists.
+    """Return seconds since the bot's most recent comment with ``marker``.
 
-    Used by both the reconsider-verdict cooldown and the grace-period
-    warning detection — keeping the iteration logic centralized stops the
-    two paths from drifting (e.g. one fixing a tz parsing bug and the
-    other forgetting to mirror it).
+    Fetches comments via `_iter_paginated_json` and delegates the
+    iteration / author-filter / timestamp logic to
+    `agent_shin_shared.seconds_since_latest_marker_comment` so the daily
+    Greptile sweep and the LLM judge use one source of truth for the
+    "bot already posted X" detection. The wall-clock `now` is resolved
+    against this module's `dt` so tests that freeze time via
+    `monkeypatch.setattr(triage_module, "dt", ...)` still apply.
     """
-    expected_login = (
-        bot_login
-        or os.environ.get("AGENT_SHIN_BOT_LOGIN")
-        or AGENT_SHIN_DEFAULT_BOT_LOGIN
-    ).lower()
-    latest: dt.datetime | None = None
-    for comment in _iter_paginated_json(f"repos/{repo}/issues/{number}/comments"):
-        author = ((comment.get("user") or {}).get("login") or "").lower()
-        if author != expected_login:
-            continue
-        body = comment.get("body") or ""
-        if marker not in body:
-            continue
-        created = comment.get("created_at")
-        if not created:
-            continue
-        try:
-            ts = dt.datetime.fromisoformat(created.replace("Z", "+00:00"))
-        except ValueError:
-            continue
-        if latest is None or ts > latest:
-            latest = ts
-    if latest is None:
-        return None
-    return (dt.datetime.now(dt.timezone.utc) - latest).total_seconds()
+    return seconds_since_latest_marker_comment(
+        _iter_paginated_json(f"repos/{repo}/issues/{number}/comments"),
+        marker=marker,
+        bot_login=bot_login,
+        now=dt.datetime.now(dt.timezone.utc),
+    )
 
 
 def seconds_since_last_reconsider_verdict(
@@ -861,8 +838,30 @@ def _combine_missing(
     return missing or ["(see explanation below)"]
 
 
-def _has_marker(comments: Iterable[dict], marker: str) -> bool:
-    return any(marker in (comment.get("body") or "") for comment in comments)
+def _has_marker(
+    comments: Iterable[dict], marker: str, *, bot_login: str | None = None
+) -> bool:
+    """Return True iff the bot itself posted a comment containing ``marker``.
+
+    Filters by author so a contributor who quotes the marker (e.g. via
+    GitHub's "Quote reply" feature, which preserves HTML comments in
+    raw markdown) is not mistaken for a bot action — that would
+    silently suppress notifications or change which "recovered" wording
+    is selected. Matches the author-filter pattern used by the sibling
+    `_seconds_since_latest_marker_comment` helper.
+    """
+    expected_login = (
+        bot_login
+        or os.environ.get("AGENT_SHIN_BOT_LOGIN")
+        or AGENT_SHIN_DEFAULT_BOT_LOGIN
+    ).lower()
+    for comment in comments:
+        author = ((comment.get("user") or {}).get("login") or "").lower()
+        if author != expected_login:
+            continue
+        if marker in (comment.get("body") or ""):
+            return True
+    return False
 
 
 def format_ready_for_review_comment(verdict: dict, greptile_score: int | None) -> str:

.github/workflows/triage_issue_with_llm.yml

@@ -49,7 +49,13 @@ jobs:
       - name: Run Agent Shin
         env:
           GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
-          OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }}
+          # Only expose the LLM key when the bot is enabled or a collaborator
+          # triggers it manually, so an external user can't force paid LLM
+          # calls by churning issues while the bot is still in dry-run.
+          # The Python script calls the LLM whenever this var is set
+          # (regardless of `--close`); stripping `--close` doesn't suppress
+          # the API call, only the destructive side effects.
+          OPENAI_API_KEY: ${{ (vars.AGENT_SHIN_ENABLED == 'true' || github.event_name == 'workflow_dispatch') && secrets.OPENAI_API_KEY || '' }}
           OPENAI_BASE_URL: ${{ vars.OPENAI_BASE_URL }}
           TRIAGE_MODEL: ${{ vars.TRIAGE_MODEL }}
           AGENT_SHIN_ENABLED: ${{ vars.AGENT_SHIN_ENABLED }}

.github/workflows/triage_pr_with_llm.yml

@@ -60,7 +60,13 @@ jobs:
       - name: Run Agent Shin
         env:
           GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
-          OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }}
+          # Only expose the LLM key when the bot is enabled or a collaborator
+          # triggers it manually, so an external user can't force paid LLM
+          # calls by churning a fork PR while the bot is still in dry-run.
+          # The Python script calls the LLM whenever this var is set
+          # (regardless of `--close`); stripping `--close` doesn't suppress
+          # the API call, only the destructive side effects.
+          OPENAI_API_KEY: ${{ (vars.AGENT_SHIN_ENABLED == 'true' || github.event_name == 'workflow_dispatch') && secrets.OPENAI_API_KEY || '' }}
           OPENAI_BASE_URL: ${{ vars.OPENAI_BASE_URL }}
           TRIAGE_MODEL: ${{ vars.TRIAGE_MODEL }}
           AGENT_SHIN_ENABLED: ${{ vars.AGENT_SHIN_ENABLED }}

tests/test_litellm/test_github_review_gate.py

@@ -147,7 +147,12 @@ class TestReviewGatePass:
         # A regression marker in history -> this is a recovery, not a first pass.
         monkeypatch.setattr(triage_module, "fetch_pr", lambda repo, n: _make_pr())
         rec = _Recorder(triage_module, monkeypatch)
-        prior = [{"user": {"login": "x"}, "body": triage_module.REGRESSED_MARKER}]
+        prior = [
+            {
+                "user": {"login": "github-actions[bot]"},
+                "body": triage_module.REGRESSED_MARKER,
+            }
+        ]
 
         result = _gate(triage_module, judge=_pass, greptile_score=5, comments=prior)
 
@@ -241,7 +246,12 @@ class TestReviewGateGraceAndClose:
             triage_module, "fetch_pr", lambda repo, n: _make_pr(created_at=JUST_NOW)
         )
         rec = _Recorder(triage_module, monkeypatch)
-        prior = [{"user": {"login": "b"}, "body": triage_module.WITHIN_GRACE_MARKER}]
+        prior = [
+            {
+                "user": {"login": "github-actions[bot]"},
+                "body": triage_module.WITHIN_GRACE_MARKER,
+            }
+        ]
 
         result = _gate(triage_module, judge=_fail, greptile_score=None, comments=prior)
 
@@ -345,7 +355,7 @@ class TestReviewGateGuards:
             triage_module,
             "post_comment",
             lambda repo, n, body: state["comments"].append(
-                {"user": {"login": "agent-shin[bot]"}, "body": body}
+                {"user": {"login": "github-actions[bot]"}, "body": body}
             ),
         )
         monkeypatch.setattr(