Skip to content

Commit 9216e31

Browse files
authored
Fix advisor-coverage installation-token mint and harden backfill dispatch (#8585)
**Impact:** pytorch-advisor-coverage lambda only **Risk:** low ## What Replaces the broken scoped installation-token mint with one that actually returns a usable token, re-mints the token ahead of its 60-minute expiry so long backfills don't die mid-run, and raises the inter-dispatch gap floor for backfill mode. ## Why The `actions:write`-scoped mint went through `Auth.AppInstallationAuth`, whose `.token` only resolves once the auth object is handed to a `github.Github(...)` client. Reading `.token` off a standalone instance asserts, so every real dispatch run failed at runtime. It now mints via `GithubIntegration.get_access_token`, which returns the full authorization (token + `expires_at`). Two follow-on gaps this exposes: - A backfill walks months of history off a single startup mint and runs well past the token's 60-minute lifetime. The dispatch path now re-mints when the token nears GitHub's stated `expires_at` (judged against `expires_at`, not elapsed process time, so a suspended-then-resumed run doesn't dispatch with a dead token). - Backfill sustains a dispatch rate the ongoing cron never reaches and is the only mode that can trip GitHub's secondary rate limit on `workflow_dispatch`, so its gap now floors at 5s (vs 1s for ongoing). Env/event may still raise the gap, never lower it below the floor. # Notes - Mint failures now raise a `RuntimeError` naming `GITHUB_APP_ID` / `GITHUB_INSTALLATION_ID`, which disambiguates the common "key registered to a different App" error from a genuine GitHub outage. --------- Signed-off-by: Jean Schmidt <contato@jschmidt.me>
1 parent 08ab86b commit 9216e31

6 files changed

Lines changed: 224 additions & 30 deletions

File tree

aws/lambda/pytorch-advisor-coverage/.env.example

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ AS_OF_STEP_HOURS=24
2929

3030
# Throttle
3131
MAX_DISPATCHES_PER_RUN=10
32+
# Floored to 1s in ongoing mode, 5s in backfill mode (GitHub secondary rate limit).
3233
DISPATCH_GAP_SECONDS=3
3334

3435
# Safety: DRY_RUN=true logs intended dispatches without POSTing. Set to false to

aws/lambda/pytorch-advisor-coverage/README.md

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,11 @@ MODE=backfill AS_OF_START=2026-02-19 AS_OF_END=2026-08-18 \
120120

121121
- `MAX_DISPATCHES_PER_RUN` (default 10) — per invocation, clamped by a compiled
122122
`HARD_CAP` (100) and the Lambda timeout budget; env/event may only LOWER it.
123-
- `DISPATCH_GAP_SECONDS` (default 3) — sleep between dispatches (floored to 1s).
123+
- `DISPATCH_GAP_SECONDS` (default 3) — sleep between dispatches, floored to 1s in
124+
ongoing mode and to 5s in backfill mode. Backfill sustains a dispatch rate the
125+
ongoing cron never reaches, so it is the only mode that can hit GitHub's
126+
secondary rate limit on `workflow_dispatch`. The env may raise the gap past
127+
either floor, never below it.
124128

125129
Cross-invocation duplicates (a red re-dispatched before its verdict lands) are
126130
accepted: safe (prefixed → no reverts) and bounded by the throttle. Intra-run
@@ -155,7 +159,7 @@ dispatched once. This is bounded, safe (non-reverting), and matches
155159
| `AS_OF_START` / `AS_OF_END` || backfill range (UTC) |
156160
| `AS_OF_STEP_HOURS` | `24` | backfill chunk size |
157161
| `MAX_DISPATCHES_PER_RUN` | `10` | see Throttle |
158-
| `DISPATCH_GAP_SECONDS` | `3` | see Throttle |
162+
| `DISPATCH_GAP_SECONDS` | `3` | floored to 5s in backfill mode; see Throttle |
159163
| `DRY_RUN` | `true` | `false` arms real dispatch |
160164
| `LOG_LEVEL` | `INFO` | secret-leaking loggers pinned to WARNING regardless |
161165

aws/lambda/pytorch-advisor-coverage/advisor_coverage/bootstrap.py

Lines changed: 81 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@
1818
import logging
1919
import sys
2020
from dataclasses import dataclass
21+
from datetime import datetime, timedelta, timezone
22+
from typing import Optional
2123

2224
import boto3
2325
import github
@@ -43,6 +45,27 @@
4345
# excludes contents:write, so a leaked coverage token cannot push/revert.
4446
_DISPATCH_TOKEN_PERMISSIONS = {"actions": "write"}
4547

48+
# GitHub installation tokens expire 60 minutes after minting, and the token is a
49+
# plain string handed to GHClientFactory — nothing refreshes it. A backfill
50+
# dispatches for hours off one startup mint, so the dispatch path re-mints ahead
51+
# of expiry. Freshness is judged against GitHub's own `expires_at` rather than
52+
# elapsed local time: a laptop suspended mid-backfill wakes with the token
53+
# already dead while a monotonic counter still reads it as fresh.
54+
_TOKEN_REFRESH_MARGIN = timedelta(minutes=10)
55+
56+
57+
@dataclass
58+
class _DispatchAuth:
59+
"""App credentials retained so the dispatch token can be re-minted."""
60+
61+
app_id: str
62+
pem: str
63+
installation_id: int
64+
expires_at: datetime
65+
66+
67+
_dispatch_auth: Optional[_DispatchAuth] = None
68+
4669

4770
def configure_logging(log_level: str) -> None:
4871
"""Configure logging and pin secret-leaking third-party loggers to WARNING."""
@@ -91,20 +114,35 @@ def _get_secret_from_aws(secret_store_name: str) -> _AWSSecrets:
91114
sys.exit(1)
92115

93116

94-
def _mint_scoped_installation_token(app_id: str, pem: str, installation_id: int) -> str:
95-
"""Mint an installation token scoped to `actions:write` only.
117+
def _mint_scoped_installation_auth(app_id: str, pem: str, installation_id: int):
118+
"""Mint an installation authorization scoped to `actions:write` only.
119+
120+
Returns the whole authorization, not just the token string, because its
121+
`expires_at` is the only trustworthy basis for deciding when to re-mint.
96122
97-
Without token_permissions the mint inherits the App's full permission set
123+
Without explicit permissions the mint inherits the App's full permission set
98124
(incl. contents:write → revert-capable). Scoping to actions:write is the
99125
minimum for workflow_dispatch and removes revert capability entirely.
126+
127+
Minted through GithubIntegration rather than Auth.AppInstallationAuth:
128+
PyGithub (2.6.1) only builds that auth object's internal integration inside
129+
`withRequester`, which nothing calls until the auth is handed to a
130+
`github.Github(...)`, so reading `.token` off a standalone instance always
131+
asserts.
100132
"""
101-
app_auth = github.Auth.AppAuth(app_id, pem)
102-
inst_auth = github.Auth.AppInstallationAuth(
103-
app_auth,
104-
installation_id=installation_id,
105-
token_permissions=_DISPATCH_TOKEN_PERMISSIONS,
106-
)
107-
return inst_auth.token
133+
integration = github.GithubIntegration(auth=github.Auth.AppAuth(app_id, pem))
134+
try:
135+
return integration.get_access_token(
136+
installation_id, permissions=_DISPATCH_TOKEN_PERMISSIONS
137+
)
138+
except github.GithubException as e:
139+
# GitHub answers a key that is validly formed but registered to a
140+
# different App with "A JSON web token could not be decoded" — naming the
141+
# identifiers is what distinguishes that from a genuine outage.
142+
raise RuntimeError(
143+
f"Failed to mint an installation token for GITHUB_APP_ID={app_id}, "
144+
f"GITHUB_INSTALLATION_ID={installation_id}: {e}"
145+
) from e
108146

109147

110148
def setup_clients(config: CoverageConfig) -> None:
@@ -128,10 +166,17 @@ def setup_clients(config: CoverageConfig) -> None:
128166
)
129167

130168
if config.github_app_id and config.github_installation_id and gh_app_secret:
131-
scoped_token = _mint_scoped_installation_token(
169+
global _dispatch_auth
170+
scoped = _mint_scoped_installation_auth(
132171
config.github_app_id, gh_app_secret, config.github_installation_id
133172
)
134-
GHClientFactory.setup_client(token=scoped_token)
173+
GHClientFactory.setup_client(token=scoped.token)
174+
_dispatch_auth = _DispatchAuth(
175+
app_id=config.github_app_id,
176+
pem=gh_app_secret,
177+
installation_id=config.github_installation_id,
178+
expires_at=scoped.expires_at,
179+
)
135180
elif config.github_access_token:
136181
GHClientFactory.setup_client(token=config.github_access_token)
137182
else:
@@ -145,3 +190,27 @@ def setup_clients(config: CoverageConfig) -> None:
145190
raise RuntimeError(
146191
"ClickHouse connection test failed. Please check your configuration."
147192
)
193+
194+
195+
def refresh_dispatch_token_if_stale() -> bool:
196+
"""Re-mint the installation token when it is close to GitHub's stated expiry.
197+
198+
Returns True when a new token was installed. A no-op when the client was
199+
configured from a raw GITHUB_TOKEN — there are no App credentials to re-mint
200+
from, and a PAT does not expire on this timescale.
201+
"""
202+
if _dispatch_auth is None:
203+
return False
204+
if datetime.now(timezone.utc) + _TOKEN_REFRESH_MARGIN < _dispatch_auth.expires_at:
205+
return False
206+
207+
scoped = _mint_scoped_installation_auth(
208+
_dispatch_auth.app_id, _dispatch_auth.pem, _dispatch_auth.installation_id
209+
)
210+
GHClientFactory.setup_client(token=scoped.token)
211+
_dispatch_auth.expires_at = scoped.expires_at
212+
logging.info(
213+
"[coverage] re-minted the installation token, now valid until %s",
214+
scoped.expires_at.isoformat(timespec="seconds"),
215+
)
216+
return True

aws/lambda/pytorch-advisor-coverage/advisor_coverage/config.py

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,10 @@
3333
# Compiled-in safety ceilings. Env/event may only make the throttle SMALLER.
3434
HARD_CAP_DISPATCHES = 100
3535
MIN_DISPATCH_GAP_SECONDS = 1
36+
# Backfill walks months of history in one pass, so it sustains a dispatch rate
37+
# the ongoing cron never approaches and is the only mode that can reach GitHub's
38+
# secondary rate limit on workflow_dispatch. It floors far higher as a result.
39+
BACKFILL_MIN_DISPATCH_GAP_SECONDS = 5
3640
# The Lambda's configured timeout (tf). The throttle is clamped so a run's
3741
# inter-dispatch sleeps can never approach it.
3842
LAMBDA_TIMEOUT_SECONDS = 260
@@ -146,8 +150,17 @@ class CoverageConfig:
146150
log_level: str = "INFO"
147151

148152
def effective_gap_seconds(self) -> int:
149-
"""Gap floored to a positive minimum (never 0 → no dispatch storm)."""
150-
return max(MIN_DISPATCH_GAP_SECONDS, int(self.dispatch_gap_seconds))
153+
"""Gap floored to a positive minimum (never 0 → no dispatch storm).
154+
155+
Backfill floors at its own, much higher minimum; env/event can raise the
156+
gap beyond either floor but never below it.
157+
"""
158+
floor = (
159+
BACKFILL_MIN_DISPATCH_GAP_SECONDS
160+
if self.mode == "backfill"
161+
else MIN_DISPATCH_GAP_SECONDS
162+
)
163+
return max(floor, int(self.dispatch_gap_seconds))
151164

152165
def effective_max_dispatches(self) -> int:
153166
"""Configured cap, clamped by HARD_CAP and the Lambda timeout budget.

aws/lambda/pytorch-advisor-coverage/advisor_coverage/dispatcher.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
from pytorch_auto_revert.github_client_helper import GHClientFactory
2424
from pytorch_auto_revert.utils import proper_workflow_create_dispatch, RetryWithBackoff
2525

26+
from .bootstrap import refresh_dispatch_token_if_stale
2627
from .config import ADVISOR_WORKFLOW_FILE, COVERAGE_SIGNAL_KEY_PREFIX, CoverageConfig
2728
from .enumeration import _naive_utc, RedSignal, UnclassifiedRedEnumerator
2829
from .logfilter import has_readable_log
@@ -287,6 +288,7 @@ def _dispatch_one(
287288
)
288289
return
289290

291+
refresh_dispatch_token_if_stale()
290292
workflow = self._advisor_workflow()
291293
factory = GHClientFactory()
292294
# /dispatches is non-idempotent — single attempt via the retry=0

aws/lambda/pytorch-advisor-coverage/advisor_coverage/tests/test_advisor_coverage.py

Lines changed: 119 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,14 @@
88
import logging
99
import time
1010
import unittest
11-
from datetime import datetime, timedelta
11+
from datetime import datetime, timedelta, timezone
1212
from unittest.mock import MagicMock, patch
1313

1414
from advisor_coverage import config as config_mod
1515
from advisor_coverage.backfill import run_backfill
1616
from advisor_coverage.config import (
1717
_parse_workflows,
18+
BACKFILL_MIN_DISPATCH_GAP_SECONDS,
1819
COVERAGE_SIGNAL_KEY_PREFIX,
1920
CoverageConfig,
2021
HARD_CAP_DISPATCHES,
@@ -97,6 +98,25 @@ def make_red(
9798
)
9899

99100

101+
def _minted(token: str, valid_for: timedelta) -> MagicMock:
102+
"""A stand-in for PyGithub's InstallationAuthorization."""
103+
auth = MagicMock()
104+
auth.token = token
105+
auth.expires_at = datetime.now(timezone.utc) + valid_for
106+
return auth
107+
108+
109+
def _auth_expiring_in(remaining: timedelta):
110+
from advisor_coverage.bootstrap import _DispatchAuth
111+
112+
return _DispatchAuth(
113+
app_id="a",
114+
pem="PEM",
115+
installation_id=1,
116+
expires_at=datetime.now(timezone.utc) + remaining,
117+
)
118+
119+
100120
def _urlopen_cm(status, content_length):
101121
"""A fake urlopen() context manager for logfilter HEAD tests."""
102122
resp = MagicMock()
@@ -557,6 +577,13 @@ def test_timeout_clamp(self):
557577
def test_gap_floor(self):
558578
self.assertEqual(make_config(dispatch_gap_seconds=0).effective_gap_seconds(), 1)
559579

580+
def test_backfill_gap_floor(self):
581+
cfg = make_config(mode="backfill", dispatch_gap_seconds=3)
582+
self.assertEqual(cfg.effective_gap_seconds(), BACKFILL_MIN_DISPATCH_GAP_SECONDS)
583+
raised = make_config(mode="backfill", dispatch_gap_seconds=60)
584+
self.assertEqual(raised.effective_gap_seconds(), 60)
585+
self.assertEqual(make_config(dispatch_gap_seconds=3).effective_gap_seconds(), 3)
586+
560587
def test_repo_pin(self):
561588
with self.assertRaises(ValueError):
562589
CoverageConfig.from_env_and_event({"repo_full_name": "evil/repo"})
@@ -835,18 +862,96 @@ def test_configure_logging_pins_secret_loggers(self):
835862
for name in ("github", "github.Requester", "botocore", "boto3", "urllib3"):
836863
self.assertEqual(logging.getLogger(name).level, logging.WARNING, name)
837864

838-
@patch("advisor_coverage.bootstrap.github")
839-
def test_mint_scopes_token_to_actions_write(self, mock_github):
840-
from advisor_coverage.bootstrap import _mint_scoped_installation_token
865+
# Patches only GithubIntegration, so the surrounding call path runs against
866+
# real PyGithub: a mint that cannot produce a token without a Requester
867+
# fails here instead of passing against an all-mocked github module.
868+
@patch("advisor_coverage.bootstrap.github.GithubIntegration")
869+
def test_mint_scopes_token_to_actions_write(self, mock_integration):
870+
import github
871+
from advisor_coverage.bootstrap import _mint_scoped_installation_auth
872+
873+
get_token = mock_integration.return_value.get_access_token
874+
get_token.return_value.token = "ghs_scoped"
875+
scoped = _mint_scoped_installation_auth("app-id", "PEM", 4242)
876+
self.assertEqual(scoped.token, "ghs_scoped")
877+
self.assertIsInstance(
878+
mock_integration.call_args.kwargs["auth"], github.Auth.AppAuth
879+
)
880+
args, kwargs = get_token.call_args
881+
self.assertEqual(args, (4242,))
882+
self.assertEqual(kwargs["permissions"], {"actions": "write"})
883+
884+
@patch("advisor_coverage.bootstrap.github.GithubIntegration")
885+
def test_mint_failure_names_the_identifiers(self, mock_integration):
886+
import github
887+
from advisor_coverage.bootstrap import _mint_scoped_installation_auth
841888

842-
inst = MagicMock()
843-
inst.token = "ghs_scoped"
844-
mock_github.Auth.AppInstallationAuth.return_value = inst
845-
token = _mint_scoped_installation_token("app-id", "PEM", 4242)
846-
self.assertEqual(token, "ghs_scoped")
847-
kwargs = mock_github.Auth.AppInstallationAuth.call_args.kwargs
848-
self.assertEqual(kwargs["token_permissions"], {"actions": "write"})
849-
self.assertEqual(kwargs["installation_id"], 4242)
889+
mock_integration.return_value.get_access_token.side_effect = (
890+
github.GithubException(401, {"message": "could not be decoded"}, None)
891+
)
892+
with self.assertRaises(RuntimeError) as ctx:
893+
_mint_scoped_installation_auth("app-id", "PEM", 4242)
894+
self.assertIn("app-id", str(ctx.exception))
895+
self.assertIn("4242", str(ctx.exception))
896+
897+
def test_token_refresh_noop_without_app_auth(self):
898+
from advisor_coverage import bootstrap
899+
900+
bootstrap._dispatch_auth = None
901+
self.assertFalse(bootstrap.refresh_dispatch_token_if_stale())
902+
903+
def test_token_refresh_noop_while_fresh(self):
904+
from advisor_coverage import bootstrap
905+
906+
bootstrap._dispatch_auth = _auth_expiring_in(timedelta(minutes=59))
907+
try:
908+
with patch(
909+
"advisor_coverage.bootstrap._mint_scoped_installation_auth"
910+
) as mint:
911+
self.assertFalse(bootstrap.refresh_dispatch_token_if_stale())
912+
mint.assert_not_called()
913+
finally:
914+
bootstrap._dispatch_auth = None
915+
916+
def test_token_reminted_inside_the_expiry_margin(self):
917+
from advisor_coverage import bootstrap
918+
919+
# Still valid, but inside the margin — a dispatch now could outlive it.
920+
bootstrap._dispatch_auth = _auth_expiring_in(timedelta(minutes=5))
921+
try:
922+
with patch(
923+
"advisor_coverage.bootstrap._mint_scoped_installation_auth",
924+
return_value=_minted("ghs_new", timedelta(minutes=60)),
925+
) as mint, patch("advisor_coverage.bootstrap.GHClientFactory") as ghf:
926+
self.assertTrue(bootstrap.refresh_dispatch_token_if_stale())
927+
mint.assert_called_once_with("a", "PEM", 1)
928+
ghf.setup_client.assert_called_once_with(token="ghs_new")
929+
# Expiry advanced, so the next dispatch does not re-mint again.
930+
self.assertFalse(bootstrap.refresh_dispatch_token_if_stale())
931+
finally:
932+
bootstrap._dispatch_auth = None
933+
934+
def test_token_reminted_after_a_suspend(self):
935+
"""An already-expired token re-mints even with no elapsed process time."""
936+
from advisor_coverage import bootstrap
937+
938+
bootstrap._dispatch_auth = _auth_expiring_in(timedelta(minutes=-30))
939+
try:
940+
with patch(
941+
"advisor_coverage.bootstrap._mint_scoped_installation_auth",
942+
return_value=_minted("ghs_new", timedelta(minutes=60)),
943+
), patch("advisor_coverage.bootstrap.GHClientFactory"):
944+
self.assertTrue(bootstrap.refresh_dispatch_token_if_stale())
945+
finally:
946+
bootstrap._dispatch_auth = None
947+
948+
def test_dispatch_checks_token_freshness(self):
949+
reds = [make_red(job_name="j / t", observed="o1")]
950+
with patch(
951+
"advisor_coverage.dispatcher.refresh_dispatch_token_if_stale"
952+
) as refresh, _DispatchHarness(reds, config=make_config(dry_run=False)) as h:
953+
h.dispatcher.dispatch_for_window(T(0), T(59))
954+
refresh.assert_called_once_with()
850955

851956
def test_setup_clients_uses_scoped_token(self):
852957
cfg = make_config(
@@ -855,8 +960,8 @@ def test_setup_clients_uses_scoped_token(self):
855960
github_app_secret=base64.b64encode(b"PEM").decode(),
856961
)
857962
with patch(
858-
"advisor_coverage.bootstrap._mint_scoped_installation_token",
859-
return_value="ghs_scoped",
963+
"advisor_coverage.bootstrap._mint_scoped_installation_auth",
964+
return_value=_minted("ghs_scoped", timedelta(minutes=60)),
860965
) as mint, patch("advisor_coverage.bootstrap.GHClientFactory") as ghf, patch(
861966
"advisor_coverage.bootstrap.CHCliFactory"
862967
) as ch:

0 commit comments

Comments
 (0)