From f73d5fa01dbb4ec4bac100c63305d41fdb07cc2b Mon Sep 17 00:00:00 2001 From: Caetano Melone Date: Fri, 21 Aug 2026 14:29:08 -0700 Subject: [PATCH 1/4] validate repo config when when changes are proposed Previously, if a repo's Hubcast config was invalid and a dev opened a PR/branch to fix the issue, we'd read from the default branch config and report the old error. This PR adds the following - if hubcast.yml was changed in the branch/PR, validate that config and report the status back to the user - report the exception text to users when validation fails - add user-friendly parsing to the YAML and pydantic config errors - rename the hubcast-error check to hubcast-config to avoid confusion for successful statuses This change requires an additional API call to check the files changed by a PR. Signed-off-by: Caetano Melone --- src/hubcast/clients/github/client.py | 22 +++- src/hubcast/web/github/messages.py | 5 + src/hubcast/web/github/routes.py | 77 ++++++++++- src/hubcast/web/github/utils.py | 77 ++++++++--- tests/test_github_routes.py | 189 ++++++++++++++++++++++++++- tests/test_repo_config.py | 29 +++- 6 files changed, 366 insertions(+), 33 deletions(-) diff --git a/src/hubcast/clients/github/client.py b/src/hubcast/clients/github/client.py index dd085b6f..785ea8dd 100644 --- a/src/hubcast/clients/github/client.py +++ b/src/hubcast/clients/github/client.py @@ -122,7 +122,12 @@ async def set_check_status( url = f"/repos/{self.repo_owner}/{self.repo_name}/check-runs/{existing_check['id']}" await gh.patch(url, data=payload) - async def get_repo_config(self) -> str | None: + async def get_repo_config(self, ref: str | None = None) -> str | None: + """Get the contents of the repo's hubcast config file. + + Args: + ref: Where to read the file from. Defaults to the repo's default branch. + """ gh_token = await self.auth.authenticate_installation( self.repo_owner, self.repo_name ) @@ -132,6 +137,8 @@ async def get_repo_config(self) -> str | None: # get the contents of the repository hubcast.yml file url = f"/repos/{self.repo_owner}/{self.repo_name}/contents/{self.repo_config_path}" + if ref is not None: + url = f"{url}?ref={ref}" # get raw contents rather than base64 encoded text try: return await gh.getitem(url, accept="application/vnd.github.raw") @@ -142,6 +149,19 @@ async def get_repo_config(self) -> str | None: # all others are unhandled raise + async def get_pr_files(self, pr_number: int) -> list[str]: + """Return the files changed in a PR.""" + gh_token = await self.auth.authenticate_installation( + self.repo_owner, self.repo_name + ) + + async with aiohttp.ClientSession() as session: + gh = gh_aiohttp.GitHubAPI(session, self.requester, oauth_token=gh_token) + + url = f"/repos/{self.repo_owner}/{self.repo_name}/pulls/{pr_number}/files" + files = await gh.getitem(url) + return [f["filename"] for f in files] + async def get_pr(self, id: int) -> dict[str, Any]: """Return individual PR data.""" gh_token = await self.auth.authenticate_installation( diff --git a/src/hubcast/web/github/messages.py b/src/hubcast/web/github/messages.py index 372fff89..35306504 100644 --- a/src/hubcast/web/github/messages.py +++ b/src/hubcast/web/github/messages.py @@ -45,6 +45,11 @@ "Hubcast could not parse `hubcast.yml`. " f"Fix the configuration file and retry. See the [user guide]({CONFIG_DOCS_URL}) for details." ) +CONFIG_VALID_TITLE = "Hubcast config file is valid" +CONFIG_VALID_SUMMARY = ( + "Hubcast has validated `hubcast.yml`. " + "These changes will take effect once merged into the default branch." +) def help_message(bot_caller: str) -> str: diff --git a/src/hubcast/web/github/routes.py b/src/hubcast/web/github/routes.py index f0af2b98..d2122aae 100644 --- a/src/hubcast/web/github/routes.py +++ b/src/hubcast/web/github/routes.py @@ -1,6 +1,6 @@ import logging import re -from collections.abc import Awaitable, Callable +from collections.abc import Awaitable, Callable, Collection from typing import Any from aiohttp.client_exceptions import ClientResponseError @@ -14,6 +14,8 @@ from hubcast.exceptions import HubcastError, RepoConfigError, WebhookPermissionError from hubcast.logging import update_log_context from hubcast.web.github.messages import ( + CONFIG_VALID_SUMMARY, + CONFIG_VALID_TITLE, DEACTIVATED_ACCOUNT_MARKER, DEACTIVATED_ACCOUNT_MSG, HOOK_DECLINED_MSG, @@ -31,7 +33,11 @@ WEBHOOK_PERMISSION_DENIED_TITLE, help_message, ) -from hubcast.web.github.utils import changed_files_from_push, get_repo_config +from hubcast.web.github.utils import ( + changed_files_from_push, + get_repo_config, + parse_repo_config, +) log = logging.getLogger(__name__) @@ -60,7 +66,7 @@ async def dispatch(self, event: sansio.Event, *args: Any, **kwargs: Any) -> None # this avoids overwriting errors if a normal pipeline succeeds, and provides # a default for situations where there is no default check name set # this check won't linger because resolving issues requires a new commit to be pushed -ERROR_CHECK_NAME = "hubcast-error" +ERROR_CHECK_NAME = "hubcast-config" NULL_SHA = "0" * 40 @@ -282,15 +288,25 @@ async def sync_branch( # only refresh config when a default branch push touches .github/hubcast.yml default_branch = event.data["repository"]["default_branch"] is_default_branch = sync_ref == f"refs/heads/{default_branch}" - config_changed = gh.repo_config_path in changed_files_from_push(event.data) + changed_files = changed_files_from_push(event.data) + config_changed = gh.repo_config_path in changed_files try: repo_config = await get_repo_config( gh, src_fullname, refresh=is_default_branch and config_changed ) except RepoConfigError as exc: - await report_config_error(gh, want_sha, exc) + # only report the default branch config's error when this push isn't trying to fix it + if is_default_branch or not config_changed: + await report_config_error(gh, want_sha, exc) + # if the config has changes and not on default branch, validate the new config + if not is_default_branch: + await validate_config_change(gh, changed_files, want_sha) return + # validate the changes when the default branch config doesn't have issues + if not is_default_branch: + await validate_config_change(gh, changed_files, want_sha) + dest_fullname = repo_config.dest_fullname dest_remote_url = f"{gl.instance_url}/{dest_fullname}.git" head_commit = event.data.get("head_commit") @@ -387,6 +403,46 @@ async def remove_branch( # ----------------------------------- +async def validate_config_change( + gh: GitHubClient, changed_files: Collection[str], head_sha: str +) -> None: + """ + Validate the Hubcast repo config at head_sha if changed_files touches it, + reporting feedback via a GH check. + + This is meant to supersede previously reported config errors on the default branch. + """ + if gh.repo_config_path not in changed_files: + return + + # config was deleted in this change + config = await gh.get_repo_config(ref=head_sha) + if config is None: + return + + try: + parse_repo_config(config) + except RepoConfigError as exc: + exc.log(log) + await gh.set_check_status( + head_sha, + ERROR_CHECK_NAME, + "failure", + title=exc.title, + summary=exc.summary, + ) + return + + # report success if validation passes for the PR's config + await gh.set_check_status( + head_sha, + ERROR_CHECK_NAME, + "success", + title=CONFIG_VALID_TITLE, + summary=CONFIG_VALID_SUMMARY, + ) + + async def sync_pr( pull_request: dict[str, Any], gh: GitHubClient, @@ -418,13 +474,22 @@ async def sync_pr( ) return + changed_files = await gh.get_pr_files(pull_request["number"]) + config_changed = gh.repo_config_path in changed_files + # get the repository configuration from .github/hubcast.yml try: repo_config = await get_repo_config(gh, base_fullname) except RepoConfigError as exc: - await report_config_error(gh, want_sha, exc) + # only report the default branch config's error when this push isn't trying to fix it + if not config_changed: + await report_config_error(gh, want_sha, exc) + await validate_config_change(gh, changed_files, want_sha) return + # validate the changes when the default branch config doesn't have issues + await validate_config_change(gh, changed_files, want_sha) + if not repo_config.sync_drafts and pull_request["draft"]: if repo_config.sync_drafts_msg: await gh.set_check_status( diff --git a/src/hubcast/web/github/utils.py b/src/hubcast/web/github/utils.py index 2645b111..e65b3129 100644 --- a/src/hubcast/web/github/utils.py +++ b/src/hubcast/web/github/utils.py @@ -3,6 +3,7 @@ import yaml from cachetools import TTLCache +from pydantic import ValidationError from hubcast.clients.github import GitHubClient from hubcast.exceptions import HubcastError, RepoConfigError @@ -28,6 +29,62 @@ def changed_files_from_push(payload: dict[str, Any]) -> set[str]: } +def _format_yaml_error(exc: yaml.YAMLError) -> str: + """Render YAML parse errors with bullet points rather than pyyaml's default mess.""" + if isinstance(exc, yaml.reader.ReaderError): + # encoding/control-character issues + return f"- {exc.reason} (position {exc.position})" + + # the only other error we can hit via safe_load is MarkedYAMLError + problem = f"{exc.context}; {exc.problem}" if exc.context else exc.problem + mark = exc.problem_mark + line = ( + f"- {problem} (line {mark.line + 1}, column {mark.column + 1})" + if mark + else f"- {problem}" + ) + + snippet = mark.get_snippet() if mark else None + return f"{line}\n\n```\n{snippet}\n```" if snippet else line + + +def _format_validation_error(exc: ValidationError) -> str: + """Render pydantic validation errors as a per-field bullet list.""" + lines = [] + for err in exc.errors(include_url=False, include_input=False): + loc = ".".join(str(p) for p in err["loc"]) + # pydantic prefixes messages from raised errors + msg = err["msg"].removeprefix("Value error, ") + lines.append(f"- `{loc}`: {msg}" if loc else f"- {msg}") + return "\n".join(lines) + + +def parse_repo_config(raw_config: str) -> RepoConfig: + """Parse YAML as a RepoConfig. + + Raises RepoConfigError for invalid YAML or schema validation issues. + """ + try: + config_yaml = yaml.safe_load(raw_config) + except yaml.YAMLError as e: + raise RepoConfigError( + "Invalid YAML in repo config", + title=CONFIG_INVALID_TITLE, + summary=f"{CONFIG_INVALID_SUMMARY}\n\n---\n\n{_format_yaml_error(e)}", + error=str(e), + ) + + try: + return RepoConfig.model_validate(config_yaml) + except ValidationError as e: + raise RepoConfigError( + "Invalid repo config", + title=CONFIG_INVALID_TITLE, + summary=f"{CONFIG_INVALID_SUMMARY}\n\n---\n\n{_format_validation_error(e)}", + error=str(e), + ) + + async def get_repo_config( gh: GitHubClient, fullname: str, refresh: bool = False ) -> RepoConfig: @@ -66,25 +123,7 @@ async def get_repo_config( raise HubcastError("Repo config file not found", log_level="INFO") # parse and validate YAML - try: - config_yaml = yaml.safe_load(fetched_config) - except yaml.YAMLError as e: - raise RepoConfigError( - "Invalid YAML in repo config", - title=CONFIG_INVALID_TITLE, - summary=CONFIG_INVALID_SUMMARY, - error=str(e), - ) - - try: - config = RepoConfig.model_validate(config_yaml) - except ValueError as e: - raise RepoConfigError( - "Invalid repo config", - title=CONFIG_INVALID_TITLE, - summary=CONFIG_INVALID_SUMMARY, - error=str(e), - ) + config = parse_repo_config(fetched_config) config_cache[fullname] = config log.info("Repo config fetched from source forge") diff --git a/tests/test_github_routes.py b/tests/test_github_routes.py index e2e7d24f..f6570103 100644 --- a/tests/test_github_routes.py +++ b/tests/test_github_routes.py @@ -13,6 +13,10 @@ from hubcast.exceptions import HubcastError, RepoConfigError, WebhookPermissionError from hubcast.web.github.messages import ( + CONFIG_INVALID_SUMMARY, + CONFIG_INVALID_TITLE, + CONFIG_VALID_SUMMARY, + CONFIG_VALID_TITLE, DEACTIVATED_ACCOUNT_MARKER, DEACTIVATED_ACCOUNT_MSG, HOOK_DECLINED_MSG, @@ -37,6 +41,7 @@ router, sync_branch, sync_pr_event, + validate_config_change, ) @@ -209,6 +214,8 @@ def mock_gh(): gh.react_to_comment = AsyncMock() gh.auth.authenticate_installation = AsyncMock(return_value="gh-token-123") gh.repo_config_path = ".github/hubcast.yml" + gh.get_pr_files = AsyncMock(return_value=[]) + gh.get_repo_config = AsyncMock(return_value=None) return gh @@ -780,7 +787,7 @@ class SyncCase(NamedTuple): async def test_sync_config_error_sets_error_check( case, request, mock_gh, mock_gl, mock_repligit_ops ): - """An invalid repo config should be reported as a failed hubcast-error check.""" + """An invalid repo config should be reported as a failed hubcast-config check.""" event = request.getfixturevalue(case.event_fixture) mock_repligit_ops["get_repo_config"].side_effect = repo_config_error() @@ -795,6 +802,8 @@ async def test_sync_config_error_sets_error_check( summary="config summary", ) mock_repligit_ops["send_pack"].assert_not_called() + # neither caller should fetch the change's own file when there's nothing new to validate + mock_gh.get_repo_config.assert_not_called() @pytest.mark.asyncio @@ -922,6 +931,182 @@ async def test_sync_other_error_raises( mock_gh.set_check_status.assert_not_called() +# Tests for validate_config_change + +VALID_CONFIG_YAML = "Repo:\n dest_org: owner\n dest_name: repo\n" +INVALID_CONFIG_YAML = "Repo:\n dest_org: owner\n" # missing required dest_name + + +@pytest.mark.asyncio +async def test_validate_config_change_skips_when_config_not_changed(mock_gh): + """Should not fetch or validate config when changed_files doesn't include hubcast.yml.""" + + await validate_config_change(mock_gh, ["src/app.py"], "pr-sha-123") + + mock_gh.get_repo_config.assert_not_called() + mock_gh.set_check_status.assert_not_called() + + +@pytest.mark.asyncio +async def test_validate_config_change_skips_when_config_deleted(mock_gh): + """Should not report a check when hubcast.yml was deleted in this change.""" + + mock_gh.get_repo_config.return_value = None + + await validate_config_change(mock_gh, [".github/hubcast.yml"], "pr-sha-123") + + mock_gh.get_repo_config.assert_awaited_once_with(ref="pr-sha-123") + mock_gh.set_check_status.assert_not_called() + + +@pytest.mark.asyncio +async def test_validate_config_change_valid(mock_gh): + """Should report a success check, on ERROR_CHECK_NAME, when the proposed hubcast.yml is valid.""" + + mock_gh.get_repo_config.return_value = VALID_CONFIG_YAML + + await validate_config_change(mock_gh, [".github/hubcast.yml"], "pr-sha-123") + + mock_gh.set_check_status.assert_awaited_once_with( + "pr-sha-123", + ERROR_CHECK_NAME, + "success", + title=CONFIG_VALID_TITLE, + summary=CONFIG_VALID_SUMMARY, + ) + + +@pytest.mark.asyncio +async def test_validate_config_change_invalid(mock_gh): + """Should report a failure check, on ERROR_CHECK_NAME, when the proposed hubcast.yml fails validation.""" + + mock_gh.get_repo_config.return_value = INVALID_CONFIG_YAML + + await validate_config_change(mock_gh, [".github/hubcast.yml"], "pr-sha-123") + + mock_gh.set_check_status.assert_awaited_once() + args, kwargs = mock_gh.set_check_status.await_args + assert args[:3] == ("pr-sha-123", ERROR_CHECK_NAME, "failure") + assert kwargs["title"] == CONFIG_INVALID_TITLE + assert kwargs["summary"].startswith(CONFIG_INVALID_SUMMARY) + # the specific missing field should be shown + assert "dest_name" in kwargs["summary"] + + +@pytest.mark.asyncio +async def test_sync_pr_config_fix_skips_base_error_report( + mock_pr_event, mock_gh, mock_gl, mock_repligit_ops +): + """When the base branch's config is broken but this PR's own edit to + hubcast.yml fixes it, only the fix's success should be reported. + """ + + mock_repligit_ops["get_repo_config"].side_effect = repo_config_error() + mock_gh.get_pr_files.return_value = [".github/hubcast.yml"] + mock_gh.get_repo_config.return_value = VALID_CONFIG_YAML + + await sync_pr_event(event=mock_pr_event, gh=mock_gh, gl=mock_gl, gl_user="gl-user") + + mock_gh.set_check_status.assert_awaited_once_with( + "pr-sha-123", + ERROR_CHECK_NAME, + "success", + title=CONFIG_VALID_TITLE, + summary=CONFIG_VALID_SUMMARY, + ) + + +@pytest.mark.asyncio +async def test_sync_pr_config_break_fails_even_when_base_config_is_fine( + mock_pr_event, mock_gh, mock_gl, mock_repligit_ops +): + """A PR that breaks hubcast.yml should fail validation even though the base branch's config is fine.""" + + mock_gh.get_pr_files.return_value = [".github/hubcast.yml"] + mock_gh.get_repo_config.return_value = INVALID_CONFIG_YAML + + await sync_pr_event(event=mock_pr_event, gh=mock_gh, gl=mock_gl, gl_user="gl-user") + + failure_calls = [ + call + for call in mock_gh.set_check_status.await_args_list + if call.args[:3] == ("pr-sha-123", ERROR_CHECK_NAME, "failure") + ] + assert len(failure_calls) == 1 + assert failure_calls[0].kwargs["title"] == CONFIG_INVALID_TITLE + assert failure_calls[0].kwargs["summary"].startswith(CONFIG_INVALID_SUMMARY) + assert "dest_name" in failure_calls[0].kwargs["summary"] + + +@pytest.mark.asyncio +async def test_sync_branch_non_default_validates_own_config_on_change( + mock_push_event, mock_gh, mock_gl, mock_repligit_ops +): + """Provide feedback to config changes made to non-default branches.""" + + mock_push_event.data["ref"] = "refs/heads/feature-x" + mock_push_event.data["commits"] = [ + {"added": [], "modified": [".github/hubcast.yml"], "removed": []} + ] + mock_gh.get_repo_config.return_value = VALID_CONFIG_YAML + + await sync_branch(event=mock_push_event, gh=mock_gh, gl=mock_gl, gl_user="gl-user") + + mock_gh.get_repo_config.assert_awaited_once_with(ref="sha-123") + mock_gh.set_check_status.assert_any_await( + "sha-123", + ERROR_CHECK_NAME, + "success", + title=CONFIG_VALID_TITLE, + summary=CONFIG_VALID_SUMMARY, + ) + + +@pytest.mark.asyncio +async def test_sync_branch_non_default_config_fix_skips_base_error_report( + mock_push_event, mock_gh, mock_gl, mock_repligit_ops +): + """A non-default branch push that fixes hubcast.yml should only report success, not the default branch config error.""" + + mock_push_event.data["ref"] = "refs/heads/feature-x" + mock_push_event.data["commits"] = [ + {"added": [], "modified": [".github/hubcast.yml"], "removed": []} + ] + mock_repligit_ops["get_repo_config"].side_effect = repo_config_error() + mock_gh.get_repo_config.return_value = VALID_CONFIG_YAML + + await sync_branch(event=mock_push_event, gh=mock_gh, gl=mock_gl, gl_user="gl-user") + + mock_gh.set_check_status.assert_awaited_once_with( + "sha-123", + ERROR_CHECK_NAME, + "success", + title=CONFIG_VALID_TITLE, + summary=CONFIG_VALID_SUMMARY, + ) + + +@pytest.mark.asyncio +async def test_sync_branch_non_default_config_unrelated_still_reports_base_error( + mock_push_event, mock_gh, mock_gl, mock_repligit_ops +): + """A non-default branch push that doesn't touch hubcast.yml should still report the default branches config error.""" + + mock_push_event.data["ref"] = "refs/heads/feature-x" + mock_repligit_ops["get_repo_config"].side_effect = repo_config_error() + + await sync_branch(event=mock_push_event, gh=mock_gh, gl=mock_gl, gl_user="gl-user") + + mock_gh.set_check_status.assert_awaited_once_with( + "sha-123", + ERROR_CHECK_NAME, + "failure", + title="config title", + summary="config summary", + ) + mock_gh.get_repo_config.assert_not_called() + + # Tests for remove_pr @@ -1535,7 +1720,7 @@ async def test_rerun_check_unrecognized_check_skipped( async def test_rerun_check_config_error_sets_error_check( mock_check_run_event, mock_gh, mock_gl, mock_repligit_ops ): - """An invalid repo config should be reported as a failed hubcast-error check.""" + """An invalid repo config should be reported as a failed hubcast-config check.""" mock_repligit_ops["get_repo_config"].side_effect = repo_config_error() diff --git a/tests/test_repo_config.py b/tests/test_repo_config.py index f569f417..b0bd29e1 100644 --- a/tests/test_repo_config.py +++ b/tests/test_repo_config.py @@ -156,10 +156,25 @@ async def test_get_repo_config_refreshes(mock_github_client): @pytest.mark.asyncio -async def test_get_repo_config_invalid_yaml(): +@pytest.mark.parametrize( + "raw_config,expected_detail", + [ + pytest.param( + "invalid: yaml: :", + "mapping values are not allowed here", + id="scanner_error", + ), + pytest.param( + "Repo:\n dest_org: owner\n dest_name: \x01repo\n", + "special characters are not allowed", + id="reader_error", + ), + ], +) +async def test_get_repo_config_invalid_yaml(raw_config, expected_detail): """Test handling of invalid YAML in repo config.""" gh = AsyncMock() - gh.get_repo_config = AsyncMock(return_value="invalid: yaml: :") + gh.get_repo_config = AsyncMock(return_value=raw_config) gh.repo_owner = "owner" gh.repo_name = "repo" @@ -170,7 +185,8 @@ async def test_get_repo_config_invalid_yaml(): # route handlers report these to the user via a failed check assert exc_info.value.title == CONFIG_INVALID_TITLE - assert exc_info.value.summary == CONFIG_INVALID_SUMMARY + assert exc_info.value.summary.startswith(CONFIG_INVALID_SUMMARY) + assert expected_detail in exc_info.value.summary @pytest.mark.asyncio @@ -187,7 +203,9 @@ async def test_get_repo_config_missing_repo_key(): assert "top-level 'Repo' section" in exc_info.value.context["error"] assert exc_info.value.title == CONFIG_INVALID_TITLE - assert exc_info.value.summary == CONFIG_INVALID_SUMMARY + assert exc_info.value.summary.startswith(CONFIG_INVALID_SUMMARY) + # the specific validation issue should beq in the summary shown to users + assert "top-level 'Repo' section" in exc_info.value.summary @pytest.mark.asyncio @@ -202,7 +220,8 @@ async def test_get_repo_config_missing_required_fields(): with pytest.raises(RepoConfigError, match="Invalid repo config") as exc_info: await get_repo_config(gh, "owner/repo") - assert "Field required" in exc_info.value.context["error"] + assert "dest_name" in exc_info.value.summary + assert "Field required" in exc_info.value.summary @pytest.mark.asyncio From 1523bc2809817ccf8ee52239bc4dc84439e6abcc Mon Sep 17 00:00:00 2001 From: Caetano Melone Date: Fri, 21 Aug 2026 15:11:58 -0700 Subject: [PATCH 2/4] fix ty Signed-off-by: Caetano Melone --- src/hubcast/web/github/utils.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/hubcast/web/github/utils.py b/src/hubcast/web/github/utils.py index e65b3129..57fd0a33 100644 --- a/src/hubcast/web/github/utils.py +++ b/src/hubcast/web/github/utils.py @@ -2,6 +2,7 @@ from typing import Any import yaml +import yaml.reader from cachetools import TTLCache from pydantic import ValidationError From 74183dc5eaa4f2278a5ad612e371f6c0bc25521f Mon Sep 17 00:00:00 2001 From: Caetano Melone Date: Fri, 21 Aug 2026 15:17:36 -0700 Subject: [PATCH 3/4] fix ty Signed-off-by: Caetano Melone --- src/hubcast/web/github/utils.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/hubcast/web/github/utils.py b/src/hubcast/web/github/utils.py index 57fd0a33..f4eaca34 100644 --- a/src/hubcast/web/github/utils.py +++ b/src/hubcast/web/github/utils.py @@ -36,7 +36,9 @@ def _format_yaml_error(exc: yaml.YAMLError) -> str: # encoding/control-character issues return f"- {exc.reason} (position {exc.position})" - # the only other error we can hit via safe_load is MarkedYAMLError + assert isinstance( + exc, yaml.MarkedYAMLError + ) # the only other error safe_load raises (needed to resolve type issues) problem = f"{exc.context}; {exc.problem}" if exc.context else exc.problem mark = exc.problem_mark line = ( From 85ec138ef979cdd0f18ea9b9bc4a4c0039f1cb22 Mon Sep 17 00:00:00 2001 From: Caetano Melone Date: Fri, 21 Aug 2026 16:32:26 -0700 Subject: [PATCH 4/4] don't use asserts, raise Signed-off-by: Caetano Melone --- src/hubcast/web/github/utils.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/hubcast/web/github/utils.py b/src/hubcast/web/github/utils.py index f4eaca34..16b80620 100644 --- a/src/hubcast/web/github/utils.py +++ b/src/hubcast/web/github/utils.py @@ -36,9 +36,10 @@ def _format_yaml_error(exc: yaml.YAMLError) -> str: # encoding/control-character issues return f"- {exc.reason} (position {exc.position})" - assert isinstance( - exc, yaml.MarkedYAMLError - ) # the only other error safe_load raises (needed to resolve type issues) + if not isinstance(exc, yaml.MarkedYAMLError): + # the only other error safe_load raises (needed to resolve type issues) + raise TypeError(f"Unexpected YAML error type: {type(exc)!r}") + problem = f"{exc.context}; {exc.problem}" if exc.context else exc.problem mark = exc.problem_mark line = (