adam/orchestrator
Archived
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Enumerate draft PRs via the GitHub App, not gh

Browse source 
The R720 box has no gh installed, so the draft-PR monitor's `gh pr list`
shell-out raised FileNotFoundError on every 30s tick and the monitor was
permanently blind to open draft PRs (no runaway/stale detection). The
dispatcher already authenticates with a short-lived App installation
token over REST; reuse that path.

Add app_draft_pr_lister (GET /repos/{owner}/{repo}/pulls, client-side
filter to draft PRs under agent-team/apply/) and prefer it in the monitor
provider when AGENT_TEAM_GH_APP_* are set, falling back to gh otherwise.
Fails soft to an empty snapshot so a sweep never crashes the tick loop.
This commit is contained in:
Adam Moussa 2026-06-25 13:37:39 -04:00
parent dff64161de
commit 6f0fc675ed
3 changed files with 229 additions and 14 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

91
agent-team/agent_team/dispatcher.py
View file

@ -26,19 +26,24 @@ production.
from __future__ import annotations
import base64
import logging
import re
from collections.abc import Callable
from dataclasses import dataclass
from datetime import datetime, timezone
from typing import Any, Protocol
from agent_team.state_store import compute_content_hash
_LOG = logging.getLogger(__name__)
__all__ = [
"DispatchInputs",
"DispatchResult",
"DispatcherError",
"RunLocator",
"app_branch_pusher",
"app_draft_pr_lister",
"app_run_locator",
"app_workflow_dispatcher",
"build_dispatch_inputs",
@ -748,6 +753,92 @@ def app_workflow_dispatcher(
return _fire
def app_draft_pr_lister(
token_provider: Any, *, _http: Any = None
) -> "Callable[..., list[dict[str, Any]]]":
"""App-token draft-PR lister for the A4 monitor — REST, never ``gh``.
Enumerates the currently-open *agent-team* draft PRs via
``GET /repos/{owner}/{repo}/pulls?state=open`` with an installation token from
``token_provider.token()``, replacing the ``gh pr list`` shell-out (the box
has no ``gh`` installed, so that path raised ``FileNotFoundError`` every tick
and the monitor was permanently blind). The rows are filtered client-side to
``draft is True`` and a head ref under ``head_prefix`` (the dispatcher's own
``agent-team/apply/`` namespace) so an unrelated human draft PR is never
counted, and mapped to the three fields the monitor keys off
(``number`` / ``createdAt`` / ``updatedAt``).
Fail-SOFT, mirroring the ``gh`` provider it replaces and the sibling sweeps:
any transport, auth (non-200), or parse error yields an EMPTY list (the
monitor no-ops this pass) rather than raising — a maintenance sweep must never
crash the tick loop. The Bearer token is NEVER logged or surfaced in an error.
``_http`` injects a ``requests``-like client for tests (``.get(url, *,
params=..., headers=..., timeout=...)`` -> response exposing ``.status_code``
and ``.json()``); the default lazily imports ``requests``.
"""
def _list(*, owner: str, repo: str, head_prefix: str) -> list[dict[str, Any]]:
headers = {
"Accept": "application/vnd.github+json",
"X-GitHub-Api-Version": "2022-11-28",
"Authorization": f"Bearer {token_provider.token()}",
}
url = f"{GITHUB_API_ROOT}/repos/{owner}/{repo}/pulls"
params = {"state": "open", "per_page": 100}
try:
if _http is not None:
resp = _http.get(url, params=params, headers=headers, timeout=15.0)
else:
import requests # deferred: optional dependency
resp = requests.get(url, params=params, headers=headers, timeout=15.0)
except Exception as exc: # noqa: BLE001 - never surface a token-bearing error
_LOG.warning(
"draft-pr-monitor: REST pulls enumeration transport error (%s); "
"treating as no open draft PRs this pass",
type(exc).__name__,
)
return []
status = getattr(resp, "status_code", None)
if status != 200:
_LOG.warning(
"draft-pr-monitor: REST pulls enumeration failed (status=%s); "
"treating as no open draft PRs this pass",
status,
)
return []
try:
rows = resp.json()
except Exception: # noqa: BLE001 - malformed body -> fail soft
_LOG.warning(
"draft-pr-monitor: REST pulls returned unparseable JSON; "
"treating as no open draft PRs this pass"
)
return []
out: list[dict[str, Any]] = []
for row in rows or []:
if not isinstance(row, dict) or not row.get("draft"):
continue
head_ref = ((row.get("head") or {}).get("ref")) or ""
if not head_ref.startswith(head_prefix):
continue
number = row.get("number")
if number is None:
continue
out.append(
{
"number": number,
"createdAt": row.get("created_at"),
"updatedAt": row.get("updated_at"),
}
)
return out
return _list
def app_run_locator(
token_provider: Any,
*,

88
agent-team/run-team.py
View file

@ -581,28 +581,88 @@ _DRAFT_PR_HEAD_PREFIX = "agent-team/apply/"
_DRAFT_PR_LIST_TIMEOUT_S = 30
def _app_token_provider_from_env() -> "Any | None":
"""Build a :class:`agent_team.github_app.TokenProvider` from env, or ``None``.
Mirrors :func:`agent_team.coordinator._app_dispatch_seams`' resolution: all
three of ``AGENT_TEAM_GH_APP_ID`` / ``_INSTALLATION_ID`` / ``_PRIVATE_KEY``
(a path to the App ``.pem``) must be set and the key file readable, else this
returns ``None`` and the caller falls back to the ``gh`` path. Never raises
and NEVER logs the key path's contents — a half-configured box still serves.
"""
import os # noqa: PLC0415
from pathlib import Path # noqa: PLC0415
app_id = os.environ.get("AGENT_TEAM_GH_APP_ID", "").strip()
inst = os.environ.get("AGENT_TEAM_GH_APP_INSTALLATION_ID", "").strip()
key_path = os.environ.get("AGENT_TEAM_GH_APP_PRIVATE_KEY", "").strip()
if not (app_id and inst and key_path):
return None
try:
pem = Path(key_path).read_text(encoding="utf-8")
except Exception: # noqa: BLE001 - unreadable key -> gh fallback, never crash
_LOG.warning(
"draft-pr-monitor: AGENT_TEAM_GH_APP_PRIVATE_KEY path could not be "
"read; falling back to the gh draft-PR provider."
)
return None
from agent_team.github_app import TokenProvider # noqa: PLC0415
return TokenProvider(app_id=app_id, private_key_pem=pem, installation_id=inst)
def _default_draft_pr_provider(*, owner: str, repo: str) -> "Callable[[], list[Any]]":
"""Build the production READ-ONLY draft-PR provider for the A4 monitor.
Returns a zero-arg callable that enumerates the currently-open *agent-team*
draft PRs via a single read-only ``gh pr list`` (one GET; it NEVER writes,
closes, or dispatches anything) and maps each into a
:class:`agent_team.draft_pr_monitor.DraftPr` snapshot the monitor consumes.
draft PRs and maps each into a :class:`agent_team.draft_pr_monitor.DraftPr`
snapshot the monitor consumes. It NEVER writes, closes, or dispatches
anything.
**Auth path (GitHub App vs gh-default).** When the three ``AGENT_TEAM_GH_APP_*``
env vars resolve a token provider, enumeration goes through the REST
``GET /repos/{owner}/{repo}/pulls`` with a short-lived installation token
(:func:`agent_team.dispatcher.app_draft_pr_lister`) — the SAME App auth the
dispatcher already uses, and the path that does not need ``gh`` on the box.
This is the live R720 path: the box has no ``gh`` installed, so the old
``gh pr list`` shell-out raised ``FileNotFoundError`` every tick and the
monitor was permanently blind. Absent the App env (or an unreadable key) it
falls back to the ``gh`` CLI for parity with environments that have it.
The query is scoped to the ``agent-team/apply/`` head namespace
(:data:`_DRAFT_PR_HEAD_PREFIX`) so it only ever sees the dispatcher's own
apply/verify draft PRs — never an unrelated human draft PR. ``--json`` pulls
exactly the three fields the monitor keys off (``number`` / ``createdAt`` ->
``opened_at`` for the runaway window, ``updatedAt`` -> ``updated_at`` for
staleness).
Mirrors :func:`agent_team.ci_watcher.default_ci_poller`: a thin closure over
``owner`` / ``repo`` that fails closed — a non-zero ``gh`` exit, a timeout, or
unparseable JSON yields an empty snapshot (the monitor then no-ops this pass)
rather than raising, so a transient gh hiccup never breaks the tick loop. (The
coordinator's ``_draft_pr_monitor_sweep`` ALSO swallows provider errors, so
this is belt-and-suspenders.)
apply/verify draft PRs — never an unrelated human draft PR. Both paths fail
closed — any transport, auth, or parse error yields an empty snapshot (the
monitor no-ops this pass) rather than raising, so a transient hiccup never
breaks the tick loop. (The coordinator's ``_draft_pr_monitor_sweep`` ALSO
swallows provider errors, so this is belt-and-suspenders.)
"""
from agent_team.draft_pr_monitor import DraftPr # noqa: PLC0415
token_provider = _app_token_provider_from_env()
if token_provider is not None:
from agent_team.dispatcher import app_draft_pr_lister # noqa: PLC0415
lister = app_draft_pr_lister(token_provider)
def app_provider() -> list[Any]:
rows = lister(owner=owner, repo=repo, head_prefix=_DRAFT_PR_HEAD_PREFIX)
snapshots: list[Any] = []
for row in rows:
number = row.get("number")
if number is None:
continue
snapshots.append(
DraftPr(
number=int(number),
opened_at=row.get("createdAt"),
updated_at=row.get("updatedAt"),
)
)
return snapshots
return app_provider
repo_slug = f"{owner}/{repo}"
def provider() -> list[Any]:

64
agent-team/tests/test_dispatcher.py
View file

@ -17,6 +17,7 @@ from agent_team.dispatcher import (
DispatchInputs,
DispatchResult,
app_branch_pusher,
app_draft_pr_lister,
app_run_locator,
app_workflow_dispatcher,
build_dispatch_inputs,
@ -595,3 +596,66 @@ def test_app_run_locator_scrubs_token_from_transport_error() -> None:
)
assert "ghs_TESTTOKEN" not in str(excinfo.value)
assert excinfo.value.__cause__ is None
# --------------------------------------------------------------------------- #
# app_draft_pr_lister (App-token REST seam; replaces the `gh pr list` shell-out)
# --------------------------------------------------------------------------- #
_PULLS_BODY = [
# In scope + draft -> included.
{
"number": 72,
"draft": True,
"head": {"ref": "agent-team/apply/abc123"},
"created_at": "2026-06-25T16:00:00Z",
"updated_at": "2026-06-25T16:30:00Z",
},
# Draft but NOT in the agent-team/apply namespace -> excluded.
{
"number": 50,
"draft": True,
"head": {"ref": "feature/human-thing"},
"created_at": "2026-06-24T10:00:00Z",
"updated_at": "2026-06-24T11:00:00Z",
},
# In namespace but NOT a draft (a ready PR) -> excluded.
{
"number": 60,
"draft": False,
"head": {"ref": "agent-team/apply/def456"},
"created_at": "2026-06-25T12:00:00Z",
"updated_at": "2026-06-25T12:30:00Z",
},
]
def test_app_draft_pr_lister_filters_to_agent_team_drafts() -> None:
http = _FakeHttp(get_body=_PULLS_BODY, get_status=200)
lister = app_draft_pr_lister(_StubTokenProvider(), _http=http)
rows = lister(owner="owner", repo="repo", head_prefix="agent-team/apply/")
assert [r["number"] for r in rows] == [72]
row = rows[0]
assert row["createdAt"] == "2026-06-25T16:00:00Z"
assert row["updatedAt"] == "2026-06-25T16:30:00Z"
call = http.get_calls[0]
assert call["url"].endswith("/repos/owner/repo/pulls")
assert call["params"]["state"] == "open"
assert call["headers"]["Authorization"] == "Bearer ghs_TESTTOKEN"
def test_app_draft_pr_lister_fails_soft_on_auth_error() -> None:
# A non-200 (e.g. 401/403) yields an EMPTY list (monitor no-ops) — it must
# NOT raise, unlike the dispatch seams, because a sweep cannot crash the tick.
http = _FakeHttp(get_body={}, get_status=403)
lister = app_draft_pr_lister(_StubTokenProvider(), _http=http)
assert lister(owner="o", repo="r", head_prefix="agent-team/apply/") == []
def test_app_draft_pr_lister_fails_soft_and_scrubs_token_on_transport_error() -> None:
lister = app_draft_pr_lister(_StubTokenProvider(), _http=_RaisingHttp())
# Returns [] (fail-soft); the raising transport embedded the token in its
# message, but the lister swallows it and never re-raises it.
assert lister(owner="o", repo="r", head_prefix="agent-team/apply/") == []