From 107e69c0204e6a630ebc3c29f9963cf96a04f617 Mon Sep 17 00:00:00 2001 From: rnetser Date: Sun, 14 Jun 2026 18:29:19 +0300 Subject: [PATCH 01/13] fix: use get_app().slug for app_bot_login instead of get_user().login MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit get_user() always returns 403 for GitHub App installation tokens — it's a platform limitation, not a permissions issue. get_app() is the correct endpoint for app tokens and returns app metadata including slug. Bot login format is always {slug}[bot]. Closes #1113 Signed-off-by: rnetser Generated-by: Claude --- webhook_server/libs/github_api.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/webhook_server/libs/github_api.py b/webhook_server/libs/github_api.py index 64f87853e..a3d830cfe 100644 --- a/webhook_server/libs/github_api.py +++ b/webhook_server/libs/github_api.py @@ -555,11 +555,12 @@ async def process(self) -> Any: ) if _github_app_api: try: - self.app_bot_login = await github_api_call( - lambda: _github_app_api.get_user().login, + _app = await github_api_call( + _github_app_api.get_app, logger=self.logger, log_prefix=self.log_prefix, ) + self.app_bot_login = f"{_app.slug}[bot]" except asyncio.CancelledError: raise except Exception: From 9ea505eb3a3090b818d779ee85a1606fcaba7b0a Mon Sep 17 00:00:00 2001 From: rnetser Date: Sun, 14 Jun 2026 18:56:26 +0300 Subject: [PATCH 02/13] fix: wrap slug access inside github_api_call for retry protection Moves .slug property access inside the github_api_call lambda to keep it off the event loop and get retry protection, per AGENTS.md guidelines. Signed-off-by: rnetser Assisted-by: Claude --- webhook_server/libs/github_api.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/webhook_server/libs/github_api.py b/webhook_server/libs/github_api.py index a3d830cfe..4cb28d3f2 100644 --- a/webhook_server/libs/github_api.py +++ b/webhook_server/libs/github_api.py @@ -555,12 +555,11 @@ async def process(self) -> Any: ) if _github_app_api: try: - _app = await github_api_call( - _github_app_api.get_app, + self.app_bot_login = await github_api_call( + lambda: f"{_github_app_api.get_app().slug}[bot]", logger=self.logger, log_prefix=self.log_prefix, ) - self.app_bot_login = f"{_app.slug}[bot]" except asyncio.CancelledError: raise except Exception: From 94b1ef544d080ed24fbc6f534201f5b301c15534 Mon Sep 17 00:00:00 2001 From: rnetser Date: Sun, 14 Jun 2026 19:19:20 +0300 Subject: [PATCH 03/13] fix: use AppAuth JWT for get_app() instead of installation token get_app() requires AppAuth (JWT), not AppInstallationAuth. Added get_github_app_slug() helper that creates its own GithubIntegration with proper AppAuth to call get_app().slug. Removed the intermediate _github_app_api variable since we no longer need the installation token for bot login detection. Signed-off-by: rnetser Assisted-by: Claude --- webhook_server/libs/github_api.py | 40 +++++++++---------- .../utils/github_repository_settings.py | 19 +++++++++ 2 files changed, 38 insertions(+), 21 deletions(-) diff --git a/webhook_server/libs/github_api.py b/webhook_server/libs/github_api.py index 4cb28d3f2..f1460fd23 100644 --- a/webhook_server/libs/github_api.py +++ b/webhook_server/libs/github_api.py @@ -47,6 +47,7 @@ from webhook_server.utils.context import WebhookContext, get_context from webhook_server.utils.github_repository_settings import ( DEFAULT_BRANCH_PROTECTION, + get_github_app_slug, get_repository_github_app_api, ) from webhook_server.utils.github_retry import github_api_call @@ -546,28 +547,25 @@ async def process(self) -> Any: # Initialize app bot login for bot-PR identification (async) if not self.app_bot_login: - _github_app_api = await github_api_call( - get_repository_github_app_api, - config_=self.config, - repository_name=self.repository_full_name, - logger=self.logger, - log_prefix=self.log_prefix, - ) - if _github_app_api: - try: - self.app_bot_login = await github_api_call( - lambda: f"{_github_app_api.get_app().slug}[bot]", - logger=self.logger, - log_prefix=self.log_prefix, - ) - except asyncio.CancelledError: - raise - except Exception: - self.logger.exception( - f"{self.log_prefix} Failed to get app bot login — bot-PR detection may not work" + try: + _app_slug = await github_api_call( + get_github_app_slug, + self.config, + logger=self.logger, + log_prefix=self.log_prefix, + ) + if _app_slug: + self.app_bot_login = f"{_app_slug}[bot]" + else: + self.logger.error( + f"{self.log_prefix} Failed to get app slug — bot-PR detection may not work" ) - else: - self.logger.debug(f"{self.log_prefix} No GitHub App API available — app_bot_login not set") + except asyncio.CancelledError: + raise + except Exception: + self.logger.exception( + f"{self.log_prefix} Failed to get app bot login — bot-PR detection may not work" + ) event_log: str = f"Event type: {self.github_event}. event ID: {self.x_github_delivery}" diff --git a/webhook_server/utils/github_repository_settings.py b/webhook_server/utils/github_repository_settings.py index 5628c9fe1..45b5b745f 100644 --- a/webhook_server/utils/github_repository_settings.py +++ b/webhook_server/utils/github_repository_settings.py @@ -432,6 +432,25 @@ def get_repository_github_app_api(config_: Config, repository_name: str) -> Gith return None +def get_github_app_slug(config_: Config) -> str | None: + """Get the GitHub App slug using App JWT authentication. + + Returns the app slug (e.g., 'manage-repositories-app') or None if unavailable. + """ + try: + with open(os.path.join(config_.data_dir, "webhook-server.private-key.pem")) as fd: + private_key = fd.read() + + github_app_id: int = config_.root_data["github-app-id"] + auth: AppAuth = Auth.AppAuth(app_id=github_app_id, private_key=private_key) + app_instance: GithubIntegration = GithubIntegration(auth=auth) + return app_instance.get_app().slug + + except Exception: + LOGGER.exception("Failed to get GitHub App slug") + return None + + def get_repository_github_app_token(config_: Config, repository_name: str) -> str | None: """Get a raw GitHub App installation token string for use with CLI tools. From e5e2b56d4c3e13ca35e404ecb3e1028e38eb6de5 Mon Sep 17 00:00:00 2001 From: rnetser Date: Sun, 14 Jun 2026 19:25:11 +0300 Subject: [PATCH 04/13] fix: let exceptions propagate from get_github_app_slug for retry Removed try/except from get_github_app_slug() so exceptions propagate through github_api_call() for retry/backoff on transient failures. The outer try/except in process() handles permanent failures. Signed-off-by: rnetser Assisted-by: Claude --- webhook_server/libs/github_api.py | 7 +----- .../utils/github_repository_settings.py | 22 ++++++++----------- 2 files changed, 10 insertions(+), 19 deletions(-) diff --git a/webhook_server/libs/github_api.py b/webhook_server/libs/github_api.py index f1460fd23..8de85e12f 100644 --- a/webhook_server/libs/github_api.py +++ b/webhook_server/libs/github_api.py @@ -554,12 +554,7 @@ async def process(self) -> Any: logger=self.logger, log_prefix=self.log_prefix, ) - if _app_slug: - self.app_bot_login = f"{_app_slug}[bot]" - else: - self.logger.error( - f"{self.log_prefix} Failed to get app slug — bot-PR detection may not work" - ) + self.app_bot_login = f"{_app_slug}[bot]" except asyncio.CancelledError: raise except Exception: diff --git a/webhook_server/utils/github_repository_settings.py b/webhook_server/utils/github_repository_settings.py index 45b5b745f..5c38f6a71 100644 --- a/webhook_server/utils/github_repository_settings.py +++ b/webhook_server/utils/github_repository_settings.py @@ -432,23 +432,19 @@ def get_repository_github_app_api(config_: Config, repository_name: str) -> Gith return None -def get_github_app_slug(config_: Config) -> str | None: +def get_github_app_slug(config_: Config) -> str: """Get the GitHub App slug using App JWT authentication. - Returns the app slug (e.g., 'manage-repositories-app') or None if unavailable. + Returns the app slug (e.g., 'manage-repositories-app'). + Raises on failure so github_api_call() can apply retry/backoff. """ - try: - with open(os.path.join(config_.data_dir, "webhook-server.private-key.pem")) as fd: - private_key = fd.read() - - github_app_id: int = config_.root_data["github-app-id"] - auth: AppAuth = Auth.AppAuth(app_id=github_app_id, private_key=private_key) - app_instance: GithubIntegration = GithubIntegration(auth=auth) - return app_instance.get_app().slug + with open(os.path.join(config_.data_dir, "webhook-server.private-key.pem")) as fd: + private_key = fd.read() - except Exception: - LOGGER.exception("Failed to get GitHub App slug") - return None + github_app_id: int = config_.root_data["github-app-id"] + auth: AppAuth = Auth.AppAuth(app_id=github_app_id, private_key=private_key) + app_instance: GithubIntegration = GithubIntegration(auth=auth) + return app_instance.get_app().slug def get_repository_github_app_token(config_: Config, repository_name: str) -> str | None: From ba5b0bc6b3a48833d49e760fe3986726287dc273 Mon Sep 17 00:00:00 2001 From: rnetser Date: Sun, 14 Jun 2026 19:36:52 +0300 Subject: [PATCH 05/13] style: fix ruff formatting Signed-off-by: rnetser Assisted-by: Claude --- webhook_server/libs/github_api.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/webhook_server/libs/github_api.py b/webhook_server/libs/github_api.py index 8de85e12f..31f06523a 100644 --- a/webhook_server/libs/github_api.py +++ b/webhook_server/libs/github_api.py @@ -558,9 +558,7 @@ async def process(self) -> Any: except asyncio.CancelledError: raise except Exception: - self.logger.exception( - f"{self.log_prefix} Failed to get app bot login — bot-PR detection may not work" - ) + self.logger.exception(f"{self.log_prefix} Failed to get app bot login — bot-PR detection may not work") event_log: str = f"Event type: {self.github_event}. event ID: {self.x_github_delivery}" From 013e19c01d5d921defe159a4f0b9742919ef9a71 Mon Sep 17 00:00:00 2001 From: rnetser Date: Sun, 14 Jun 2026 19:44:47 +0300 Subject: [PATCH 06/13] perf: cache GitHub App slug at module level MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Slug is immutable — no need to read PEM + call GET /app on every webhook event. First call fetches and caches, subsequent calls return cached value. Assisted-by: Claude Signed-off-by: rnetser --- webhook_server/utils/github_repository_settings.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/webhook_server/utils/github_repository_settings.py b/webhook_server/utils/github_repository_settings.py index 5c38f6a71..041de31d8 100644 --- a/webhook_server/utils/github_repository_settings.py +++ b/webhook_server/utils/github_repository_settings.py @@ -43,6 +43,7 @@ } LOGGER = get_logger_with_params() +_github_app_slug_cache: str | None = None def _get_github_repo_api(github_api: github.Github, repository: int | str) -> Repository | None: @@ -436,15 +437,22 @@ def get_github_app_slug(config_: Config) -> str: """Get the GitHub App slug using App JWT authentication. Returns the app slug (e.g., 'manage-repositories-app'). + Caches the result at module level since the slug is immutable. Raises on failure so github_api_call() can apply retry/backoff. """ + global _github_app_slug_cache + + if _github_app_slug_cache is not None: + return _github_app_slug_cache + with open(os.path.join(config_.data_dir, "webhook-server.private-key.pem")) as fd: private_key = fd.read() github_app_id: int = config_.root_data["github-app-id"] auth: AppAuth = Auth.AppAuth(app_id=github_app_id, private_key=private_key) app_instance: GithubIntegration = GithubIntegration(auth=auth) - return app_instance.get_app().slug + _github_app_slug_cache = app_instance.get_app().slug + return _github_app_slug_cache def get_repository_github_app_token(config_: Config, repository_name: str) -> str | None: From 130bf09a55e4ea5d6aa539cac4aa4df3df8e6ca2 Mon Sep 17 00:00:00 2001 From: rnetser Date: Mon, 15 Jun 2026 18:18:49 +0300 Subject: [PATCH 07/13] refactor: extract _create_github_integration helper and harden slug cache Addresses human review findings: (1) extracted shared _create_github_integration() helper to eliminate DRY violation across 3 functions, (2) added threading.Lock with double-checked locking for slug cache, (3) added empty slug fail-fast guard, (4) added debug logging. Signed-off-by: rnetser Assisted-by: Claude --- .../utils/github_repository_settings.py | 44 +++++++++++-------- 1 file changed, 26 insertions(+), 18 deletions(-) diff --git a/webhook_server/utils/github_repository_settings.py b/webhook_server/utils/github_repository_settings.py index 041de31d8..a66ccb655 100644 --- a/webhook_server/utils/github_repository_settings.py +++ b/webhook_server/utils/github_repository_settings.py @@ -1,5 +1,6 @@ import copy import os +import threading from collections.abc import Callable from concurrent.futures import Future, ThreadPoolExecutor, as_completed from copy import deepcopy @@ -44,6 +45,7 @@ LOGGER = get_logger_with_params() _github_app_slug_cache: str | None = None +_github_app_slug_lock = threading.Lock() def _get_github_repo_api(github_api: github.Github, repository: int | str) -> Repository | None: @@ -408,15 +410,23 @@ def _set_checkrun_queued(_api: Repository, _pull_request: PullRequest) -> None: return True, f"[API user {api_user}] - {repository}: Set check run status to {QUEUED_STR} is done", LOGGER.debug -def get_repository_github_app_api(config_: Config, repository_name: str) -> Github | None: - LOGGER.debug("Getting repositories GitHub app API") +def _create_github_integration(config_: Config) -> GithubIntegration: + """Create a GithubIntegration instance with App JWT authentication. + Reads the private key and app ID from config to create an authenticated + GithubIntegration. This is the shared setup for all GitHub App API operations. + """ with open(os.path.join(config_.data_dir, "webhook-server.private-key.pem")) as fd: private_key = fd.read() github_app_id: int = config_.root_data["github-app-id"] auth: AppAuth = Auth.AppAuth(app_id=github_app_id, private_key=private_key) - app_instance: GithubIntegration = GithubIntegration(auth=auth) + return GithubIntegration(auth=auth) + + +def get_repository_github_app_api(config_: Config, repository_name: str) -> Github | None: + LOGGER.debug("Getting repositories GitHub app API") + app_instance = _create_github_integration(config_) owner: str repo: str owner, repo = repository_name.split("/") @@ -445,14 +455,18 @@ def get_github_app_slug(config_: Config) -> str: if _github_app_slug_cache is not None: return _github_app_slug_cache - with open(os.path.join(config_.data_dir, "webhook-server.private-key.pem")) as fd: - private_key = fd.read() - - github_app_id: int = config_.root_data["github-app-id"] - auth: AppAuth = Auth.AppAuth(app_id=github_app_id, private_key=private_key) - app_instance: GithubIntegration = GithubIntegration(auth=auth) - _github_app_slug_cache = app_instance.get_app().slug - return _github_app_slug_cache + with _github_app_slug_lock: + # Double-checked locking + if _github_app_slug_cache is not None: + return _github_app_slug_cache + + LOGGER.debug("Getting GitHub App slug") + app_instance = _create_github_integration(config_) + slug = app_instance.get_app().slug + if not slug: + raise ValueError("GitHub App returned empty slug") + _github_app_slug_cache = slug + return _github_app_slug_cache def get_repository_github_app_token(config_: Config, repository_name: str) -> str | None: @@ -461,13 +475,7 @@ def get_repository_github_app_token(config_: Config, repository_name: str) -> st Returns the token string or None if the app is not configured/installed. """ LOGGER.debug(f"Getting GitHub App installation token for {repository_name}") - - with open(os.path.join(config_.data_dir, "webhook-server.private-key.pem")) as fd: - private_key = fd.read() - - github_app_id: int = config_.root_data["github-app-id"] - auth: AppAuth = Auth.AppAuth(app_id=github_app_id, private_key=private_key) - app_instance: GithubIntegration = GithubIntegration(auth=auth) + app_instance = _create_github_integration(config_) owner, repo = repository_name.split("/", maxsplit=1) try: From 8e6bb6c71397b701e1cb9f49a334281a05fea77e Mon Sep 17 00:00:00 2001 From: rnetser Date: Tue, 16 Jun 2026 19:07:07 +0300 Subject: [PATCH 08/13] test: add tests for app_bot_login initialization via get_github_app_slug Tests cover slug initialization, skip-when-set, and failure handling paths. Assisted-by: Claude Signed-off-by: rnetser --- webhook_server/tests/test_github_api.py | 120 ++++++++++++++++++++++++ 1 file changed, 120 insertions(+) diff --git a/webhook_server/tests/test_github_api.py b/webhook_server/tests/test_github_api.py index e1357983a..4cbb2af61 100644 --- a/webhook_server/tests/test_github_api.py +++ b/webhook_server/tests/test_github_api.py @@ -561,6 +561,126 @@ async def test_process_unsupported_event( # Should not raise an exception, just skip processing await webhook.process() + @patch.dict(os.environ, {"WEBHOOK_SERVER_DATA_DIR": "webhook_server/tests/manifests"}) + @patch("webhook_server.libs.github_api.get_github_repo_api") + @patch("webhook_server.libs.github_api.get_repository_github_app_api") + @patch("webhook_server.libs.github_api.get_api_with_highest_rate_limit") + @patch("webhook_server.utils.helpers.get_apis_and_tokes_from_config") + @patch("webhook_server.libs.config.Config.repository_local_data") + @patch("webhook_server.libs.github_api.GithubWebhook.add_api_users_to_auto_verified_and_merged_users") + @patch("webhook_server.libs.github_api.get_github_app_slug", return_value="manage-repositories-app") + async def test_process_initializes_app_bot_login_from_slug( + self, + mock_get_slug: Mock, + mock_auto_verified_prop: Mock, + mock_repo_local_data: Mock, + mock_get_apis: Mock, + mock_api_rate_limit: Mock, + mock_repo_api: Mock, + mock_repo_github_api: Mock, + pull_request_payload: dict[str, Any], + ) -> None: + """Test app_bot_login is initialized from get_github_app_slug.""" + mock_api = Mock() + mock_api.rate_limiting = [100, 5000] + mock_user = Mock() + mock_user.login = "test-user" + mock_api.get_user.return_value = mock_user + + mock_api_rate_limit.return_value = (mock_api, "TOKEN", "USER") + mock_repo_api.return_value = Mock() + mock_repo_github_api.return_value = Mock() + mock_get_apis.return_value = [] + mock_repo_local_data.return_value = {} + + headers = Headers({"X-GitHub-Event": "unsupported_event"}) + webhook = GithubWebhook(hook_data=pull_request_payload, headers=headers, logger=Mock()) + webhook.app_bot_login = "" + + await webhook.process() + assert webhook.app_bot_login == "manage-repositories-app[bot]" + + @patch.dict(os.environ, {"WEBHOOK_SERVER_DATA_DIR": "webhook_server/tests/manifests"}) + @patch("webhook_server.libs.github_api.get_github_repo_api") + @patch("webhook_server.libs.github_api.get_repository_github_app_api") + @patch("webhook_server.libs.github_api.get_api_with_highest_rate_limit") + @patch("webhook_server.utils.helpers.get_apis_and_tokes_from_config") + @patch("webhook_server.libs.config.Config.repository_local_data") + @patch("webhook_server.libs.github_api.GithubWebhook.add_api_users_to_auto_verified_and_merged_users") + @patch("webhook_server.libs.github_api.get_github_app_slug", return_value="other-app") + async def test_process_app_bot_login_skips_when_already_set( + self, + mock_get_slug: Mock, + mock_auto_verified_prop: Mock, + mock_repo_local_data: Mock, + mock_get_apis: Mock, + mock_api_rate_limit: Mock, + mock_repo_api: Mock, + mock_repo_github_api: Mock, + pull_request_payload: dict[str, Any], + ) -> None: + """Test app_bot_login is not overwritten when already set.""" + mock_api = Mock() + mock_api.rate_limiting = [100, 5000] + mock_user = Mock() + mock_user.login = "test-user" + mock_api.get_user.return_value = mock_user + + mock_api_rate_limit.return_value = (mock_api, "TOKEN", "USER") + mock_repo_api.return_value = Mock() + mock_repo_github_api.return_value = Mock() + mock_get_apis.return_value = [] + mock_repo_local_data.return_value = {} + + headers = Headers({"X-GitHub-Event": "unsupported_event"}) + webhook = GithubWebhook(hook_data=pull_request_payload, headers=headers, logger=Mock()) + webhook.app_bot_login = "existing-app[bot]" + + await webhook.process() + assert webhook.app_bot_login == "existing-app[bot]" + mock_get_slug.assert_not_called() + + @patch.dict(os.environ, {"WEBHOOK_SERVER_DATA_DIR": "webhook_server/tests/manifests"}) + @patch("webhook_server.libs.github_api.get_github_repo_api") + @patch("webhook_server.libs.github_api.get_repository_github_app_api") + @patch("webhook_server.libs.github_api.get_api_with_highest_rate_limit") + @patch("webhook_server.utils.helpers.get_apis_and_tokes_from_config") + @patch("webhook_server.libs.config.Config.repository_local_data") + @patch("webhook_server.libs.github_api.GithubWebhook.add_api_users_to_auto_verified_and_merged_users") + @patch("webhook_server.libs.github_api.get_github_app_slug", side_effect=Exception("PEM not found")) + async def test_process_app_bot_login_handles_slug_failure( + self, + mock_get_slug: Mock, + mock_auto_verified_prop: Mock, + mock_repo_local_data: Mock, + mock_get_apis: Mock, + mock_api_rate_limit: Mock, + mock_repo_api: Mock, + mock_repo_github_api: Mock, + pull_request_payload: dict[str, Any], + ) -> None: + """Test app_bot_login handles slug retrieval failure gracefully.""" + mock_api = Mock() + mock_api.rate_limiting = [100, 5000] + mock_user = Mock() + mock_user.login = "test-user" + mock_api.get_user.return_value = mock_user + + mock_api_rate_limit.return_value = (mock_api, "TOKEN", "USER") + mock_repo_api.return_value = Mock() + mock_repo_github_api.return_value = Mock() + mock_get_apis.return_value = [] + mock_repo_local_data.return_value = {} + + headers = Headers({"X-GitHub-Event": "unsupported_event"}) + mock_logger = Mock() + webhook = GithubWebhook(hook_data=pull_request_payload, headers=headers, logger=mock_logger) + webhook.app_bot_login = "" + + await webhook.process() + assert webhook.app_bot_login == "" + mock_logger.exception.assert_called_once() + @patch("webhook_server.libs.github_api.get_repository_github_app_api") @patch("webhook_server.libs.github_api.get_api_with_highest_rate_limit") @patch("webhook_server.libs.github_api.get_github_repo_api") From 16541240318bf8b07385dcaedc827bbd3fdf9409 Mon Sep 17 00:00:00 2001 From: rnetser Date: Tue, 16 Jun 2026 20:42:42 +0300 Subject: [PATCH 09/13] test: assert get_user not called in slug path, fix mock param order Add mock_api.get_user.assert_not_called() to all app_bot_login tests to verify slug-based initialization doesn't call GET /user. Fix parameter names to match @patch decorator order. Assisted-by: Claude Signed-off-by: rnetser --- webhook_server/tests/test_github_api.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/webhook_server/tests/test_github_api.py b/webhook_server/tests/test_github_api.py index 4cbb2af61..9be28d2f5 100644 --- a/webhook_server/tests/test_github_api.py +++ b/webhook_server/tests/test_github_api.py @@ -576,8 +576,8 @@ async def test_process_initializes_app_bot_login_from_slug( mock_repo_local_data: Mock, mock_get_apis: Mock, mock_api_rate_limit: Mock, + mock_repo_github_app_api: Mock, mock_repo_api: Mock, - mock_repo_github_api: Mock, pull_request_payload: dict[str, Any], ) -> None: """Test app_bot_login is initialized from get_github_app_slug.""" @@ -589,7 +589,7 @@ async def test_process_initializes_app_bot_login_from_slug( mock_api_rate_limit.return_value = (mock_api, "TOKEN", "USER") mock_repo_api.return_value = Mock() - mock_repo_github_api.return_value = Mock() + mock_repo_github_app_api.return_value = Mock() mock_get_apis.return_value = [] mock_repo_local_data.return_value = {} @@ -599,6 +599,7 @@ async def test_process_initializes_app_bot_login_from_slug( await webhook.process() assert webhook.app_bot_login == "manage-repositories-app[bot]" + mock_api.get_user.assert_not_called() @patch.dict(os.environ, {"WEBHOOK_SERVER_DATA_DIR": "webhook_server/tests/manifests"}) @patch("webhook_server.libs.github_api.get_github_repo_api") @@ -615,8 +616,8 @@ async def test_process_app_bot_login_skips_when_already_set( mock_repo_local_data: Mock, mock_get_apis: Mock, mock_api_rate_limit: Mock, + mock_repo_github_app_api: Mock, mock_repo_api: Mock, - mock_repo_github_api: Mock, pull_request_payload: dict[str, Any], ) -> None: """Test app_bot_login is not overwritten when already set.""" @@ -628,7 +629,7 @@ async def test_process_app_bot_login_skips_when_already_set( mock_api_rate_limit.return_value = (mock_api, "TOKEN", "USER") mock_repo_api.return_value = Mock() - mock_repo_github_api.return_value = Mock() + mock_repo_github_app_api.return_value = Mock() mock_get_apis.return_value = [] mock_repo_local_data.return_value = {} @@ -639,6 +640,7 @@ async def test_process_app_bot_login_skips_when_already_set( await webhook.process() assert webhook.app_bot_login == "existing-app[bot]" mock_get_slug.assert_not_called() + mock_api.get_user.assert_not_called() @patch.dict(os.environ, {"WEBHOOK_SERVER_DATA_DIR": "webhook_server/tests/manifests"}) @patch("webhook_server.libs.github_api.get_github_repo_api") @@ -655,8 +657,8 @@ async def test_process_app_bot_login_handles_slug_failure( mock_repo_local_data: Mock, mock_get_apis: Mock, mock_api_rate_limit: Mock, + mock_repo_github_app_api: Mock, mock_repo_api: Mock, - mock_repo_github_api: Mock, pull_request_payload: dict[str, Any], ) -> None: """Test app_bot_login handles slug retrieval failure gracefully.""" @@ -668,7 +670,7 @@ async def test_process_app_bot_login_handles_slug_failure( mock_api_rate_limit.return_value = (mock_api, "TOKEN", "USER") mock_repo_api.return_value = Mock() - mock_repo_github_api.return_value = Mock() + mock_repo_github_app_api.return_value = Mock() mock_get_apis.return_value = [] mock_repo_local_data.return_value = {} @@ -680,6 +682,7 @@ async def test_process_app_bot_login_handles_slug_failure( await webhook.process() assert webhook.app_bot_login == "" mock_logger.exception.assert_called_once() + mock_api.get_user.assert_not_called() @patch("webhook_server.libs.github_api.get_repository_github_app_api") @patch("webhook_server.libs.github_api.get_api_with_highest_rate_limit") From 318ee8a6f93a05295e149f2971e6083daddd38ba Mon Sep 17 00:00:00 2001 From: rnetser Date: Tue, 16 Jun 2026 21:43:06 +0300 Subject: [PATCH 10/13] fix: move network I/O outside lock, scope slug cache by app-id Perform get_app().slug outside the threading lock to avoid blocking threadpool workers. Change module-level slug cache from single value to dict keyed by github-app-id for proper scoping. Assisted-by: Claude Signed-off-by: rnetser --- .../utils/github_repository_settings.py | 33 ++++++++++--------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/webhook_server/utils/github_repository_settings.py b/webhook_server/utils/github_repository_settings.py index a66ccb655..bfe2a586b 100644 --- a/webhook_server/utils/github_repository_settings.py +++ b/webhook_server/utils/github_repository_settings.py @@ -44,7 +44,7 @@ } LOGGER = get_logger_with_params() -_github_app_slug_cache: str | None = None +_github_app_slug_cache: dict[int, str] = {} _github_app_slug_lock = threading.Lock() @@ -447,26 +447,27 @@ def get_github_app_slug(config_: Config) -> str: """Get the GitHub App slug using App JWT authentication. Returns the app slug (e.g., 'manage-repositories-app'). - Caches the result at module level since the slug is immutable. + Caches the result keyed by github-app-id since the slug is immutable per app. Raises on failure so github_api_call() can apply retry/backoff. """ - global _github_app_slug_cache + github_app_id: int = config_.root_data["github-app-id"] + + if github_app_id in _github_app_slug_cache: + return _github_app_slug_cache[github_app_id] - if _github_app_slug_cache is not None: - return _github_app_slug_cache + # Perform network I/O outside the lock — concurrent calls may + # duplicate work but won't block each other. + LOGGER.debug("Getting GitHub App slug") + app_instance = _create_github_integration(config_) + slug = app_instance.get_app().slug + if not slug: + raise ValueError("GitHub App returned empty slug") with _github_app_slug_lock: - # Double-checked locking - if _github_app_slug_cache is not None: - return _github_app_slug_cache - - LOGGER.debug("Getting GitHub App slug") - app_instance = _create_github_integration(config_) - slug = app_instance.get_app().slug - if not slug: - raise ValueError("GitHub App returned empty slug") - _github_app_slug_cache = slug - return _github_app_slug_cache + # Another thread may have populated it while we were fetching + if github_app_id not in _github_app_slug_cache: + _github_app_slug_cache[github_app_id] = slug + return _github_app_slug_cache[github_app_id] def get_repository_github_app_token(config_: Config, repository_name: str) -> str | None: From f44aee13d330932392b84171c65b5d114088fce4 Mon Sep 17 00:00:00 2001 From: rnetser Date: Tue, 16 Jun 2026 21:55:08 +0300 Subject: [PATCH 11/13] fix: use Config.get_value for github-app-id instead of root_data Use Config.get_value() for github-app-id access in _create_github_integration() and get_github_app_slug() for consistency with the rest of the codebase. Assisted-by: Claude Signed-off-by: rnetser --- webhook_server/tests/test_github_repository_settings.py | 4 ++++ webhook_server/utils/github_repository_settings.py | 4 ++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/webhook_server/tests/test_github_repository_settings.py b/webhook_server/tests/test_github_repository_settings.py index 216c9fde7..a28f1f657 100644 --- a/webhook_server/tests/test_github_repository_settings.py +++ b/webhook_server/tests/test_github_repository_settings.py @@ -726,6 +726,7 @@ def test_get_repository_github_app_api_success(self, mock_logger: Mock, mock_ope mock_config = Mock() mock_config.data_dir = "/test/dir" mock_config.root_data = {"github-app-id": 12345} + mock_config.get_value.return_value = 12345 mock_file = Mock() mock_file.read.return_value = "test-private-key" @@ -758,6 +759,7 @@ def test_get_repository_github_app_api_exception(self, mock_logger: Mock, mock_o mock_config = Mock() mock_config.data_dir = "/test/dir" mock_config.root_data = {"github-app-id": 12345} + mock_config.get_value.return_value = 12345 mock_file = Mock() mock_file.read.return_value = "test-private-key" @@ -786,6 +788,7 @@ def test_get_repository_github_app_token_success(self, _mock_logger: Mock, mock_ mock_config = Mock() mock_config.data_dir = "/test/dir" mock_config.root_data = {"github-app-id": 12345} + mock_config.get_value.return_value = 12345 mock_file = Mock() mock_file.read.return_value = "test-private-key" @@ -821,6 +824,7 @@ def test_get_repository_github_app_token_failure(self, mock_logger: Mock, mock_o mock_config = Mock() mock_config.data_dir = "/test/dir" mock_config.root_data = {"github-app-id": 12345} + mock_config.get_value.return_value = 12345 mock_file = Mock() mock_file.read.return_value = "test-private-key" diff --git a/webhook_server/utils/github_repository_settings.py b/webhook_server/utils/github_repository_settings.py index bfe2a586b..83af0909f 100644 --- a/webhook_server/utils/github_repository_settings.py +++ b/webhook_server/utils/github_repository_settings.py @@ -419,7 +419,7 @@ def _create_github_integration(config_: Config) -> GithubIntegration: with open(os.path.join(config_.data_dir, "webhook-server.private-key.pem")) as fd: private_key = fd.read() - github_app_id: int = config_.root_data["github-app-id"] + github_app_id: int = config_.get_value("github-app-id") auth: AppAuth = Auth.AppAuth(app_id=github_app_id, private_key=private_key) return GithubIntegration(auth=auth) @@ -450,7 +450,7 @@ def get_github_app_slug(config_: Config) -> str: Caches the result keyed by github-app-id since the slug is immutable per app. Raises on failure so github_api_call() can apply retry/backoff. """ - github_app_id: int = config_.root_data["github-app-id"] + github_app_id: int = config_.get_value("github-app-id") if github_app_id in _github_app_slug_cache: return _github_app_slug_cache[github_app_id] From f7d6a0a0c31fd04a6afa954cea3291e0d96ec859 Mon Sep 17 00:00:00 2001 From: rnetser Date: Tue, 16 Jun 2026 22:05:38 +0300 Subject: [PATCH 12/13] fix: fail-fast on missing github-app-id, assert get_value in tests Add ValueError for missing github-app-id in _create_github_integration() and get_github_app_slug(). Assert mock_config.get_value.assert_called_with ("github-app-id") in all GitHub App tests. Signed-off-by: rnetser Assisted-by: Claude --- webhook_server/tests/test_github_repository_settings.py | 4 ++++ webhook_server/utils/github_repository_settings.py | 8 ++++++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/webhook_server/tests/test_github_repository_settings.py b/webhook_server/tests/test_github_repository_settings.py index a28f1f657..b21872852 100644 --- a/webhook_server/tests/test_github_repository_settings.py +++ b/webhook_server/tests/test_github_repository_settings.py @@ -749,6 +749,7 @@ def test_get_repository_github_app_api_success(self, mock_logger: Mock, mock_ope result = get_repository_github_app_api(mock_config, "owner/repo") assert result == mock_github + mock_config.get_value.assert_called_with("github-app-id") mock_auth.AppAuth.assert_called_once_with(app_id=12345, private_key="test-private-key") mock_app_instance.get_repo_installation.assert_called_once_with(owner="owner", repo="repo") @@ -778,6 +779,7 @@ def test_get_repository_github_app_api_exception(self, mock_logger: Mock, mock_o result = get_repository_github_app_api(mock_config, "owner/repo") assert result is None + mock_config.get_value.assert_called_with("github-app-id") mock_logger.error.assert_called_once() assert "Repository owner/repo not found by manage-repositories-app" in mock_logger.error.call_args[0][0] @@ -813,6 +815,7 @@ def test_get_repository_github_app_token_success(self, _mock_logger: Mock, mock_ result = get_repository_github_app_token(mock_config, "owner/repo") assert result == "fake-installation-token" # pragma: allowlist secret + mock_config.get_value.assert_called_with("github-app-id") mock_auth.AppAuth.assert_called_once_with(app_id=12345, private_key="test-private-key") mock_app_instance.get_repo_installation.assert_called_once_with(owner="owner", repo="repo") mock_app_instance.get_access_token.assert_called_once_with(67890) @@ -842,6 +845,7 @@ def test_get_repository_github_app_token_failure(self, mock_logger: Mock, mock_o result = get_repository_github_app_token(mock_config, "owner/repo") assert result is None + mock_config.get_value.assert_called_with("github-app-id") mock_logger.exception.assert_called_once() assert ( "Failed to get GitHub App installation token for owner/repo" diff --git a/webhook_server/utils/github_repository_settings.py b/webhook_server/utils/github_repository_settings.py index 83af0909f..9155be1a9 100644 --- a/webhook_server/utils/github_repository_settings.py +++ b/webhook_server/utils/github_repository_settings.py @@ -419,7 +419,9 @@ def _create_github_integration(config_: Config) -> GithubIntegration: with open(os.path.join(config_.data_dir, "webhook-server.private-key.pem")) as fd: private_key = fd.read() - github_app_id: int = config_.get_value("github-app-id") + github_app_id: int | None = config_.get_value("github-app-id") + if not github_app_id: + raise ValueError("github-app-id not configured — required for GitHub App authentication") auth: AppAuth = Auth.AppAuth(app_id=github_app_id, private_key=private_key) return GithubIntegration(auth=auth) @@ -450,7 +452,9 @@ def get_github_app_slug(config_: Config) -> str: Caches the result keyed by github-app-id since the slug is immutable per app. Raises on failure so github_api_call() can apply retry/backoff. """ - github_app_id: int = config_.get_value("github-app-id") + github_app_id: int | None = config_.get_value("github-app-id") + if not github_app_id: + raise ValueError("github-app-id not configured — required for GitHub App slug lookup") if github_app_id in _github_app_slug_cache: return _github_app_slug_cache[github_app_id] From 79a9bdc4a516f0d681dc64d8927f5d50f7d19cb5 Mon Sep 17 00:00:00 2001 From: rnetser Date: Tue, 16 Jun 2026 22:16:36 +0300 Subject: [PATCH 13/13] refactor: pass pre-fetched github-app-id to avoid duplicate config lookup _create_github_integration() now accepts optional github_app_id parameter. get_github_app_slug() passes the pre-fetched ID to avoid calling config_.get_value() twice. Assisted-by: Claude Signed-off-by: rnetser --- webhook_server/utils/github_repository_settings.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/webhook_server/utils/github_repository_settings.py b/webhook_server/utils/github_repository_settings.py index 9155be1a9..e6d853666 100644 --- a/webhook_server/utils/github_repository_settings.py +++ b/webhook_server/utils/github_repository_settings.py @@ -410,8 +410,8 @@ def _set_checkrun_queued(_api: Repository, _pull_request: PullRequest) -> None: return True, f"[API user {api_user}] - {repository}: Set check run status to {QUEUED_STR} is done", LOGGER.debug -def _create_github_integration(config_: Config) -> GithubIntegration: - """Create a GithubIntegration instance with App JWT authentication. +def _create_github_integration(config_: Config, github_app_id: int | None = None) -> GithubIntegration: + """Create an authenticated GithubIntegration instance using App JWT. Reads the private key and app ID from config to create an authenticated GithubIntegration. This is the shared setup for all GitHub App API operations. @@ -419,9 +419,11 @@ def _create_github_integration(config_: Config) -> GithubIntegration: with open(os.path.join(config_.data_dir, "webhook-server.private-key.pem")) as fd: private_key = fd.read() - github_app_id: int | None = config_.get_value("github-app-id") if not github_app_id: - raise ValueError("github-app-id not configured — required for GitHub App authentication") + github_app_id = config_.get_value("github-app-id") + if not github_app_id: + raise ValueError("github-app-id not configured — required for GitHub App authentication") + auth: AppAuth = Auth.AppAuth(app_id=github_app_id, private_key=private_key) return GithubIntegration(auth=auth) @@ -462,7 +464,7 @@ def get_github_app_slug(config_: Config) -> str: # Perform network I/O outside the lock — concurrent calls may # duplicate work but won't block each other. LOGGER.debug("Getting GitHub App slug") - app_instance = _create_github_integration(config_) + app_instance = _create_github_integration(config_, github_app_id=github_app_id) slug = app_instance.get_app().slug if not slug: raise ValueError("GitHub App returned empty slug")