From 523a2ca2354b1282afc07ee7c2e6b6c9d4a1d8b0 Mon Sep 17 00:00:00 2001 From: MarTrepodi Date: Sat, 9 May 2026 19:36:11 -0400 Subject: [PATCH 1/6] ci: align workflow triggers with develop branch model (#85) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary - Replace stale branch references (`1.0-maintenance`, `2.0-development`) with `develop` in `ci.yml`, `commitlint.yml`, and `integration.yml`. - Add `develop` to push triggers in `ci.yml` and `integration.yml` so post-merge signal is visible on the integration branch. - Drop `async` and `statcalc` from `commitlint.yml` `scope-enum` (the underlying feature branches are deleted). - Delete `test.yml` since it is a strict subset of the `test` and `docs` jobs in `ci.yml` at lower fidelity (single Python version, no coverage). ## Why The repository moved to a `main` (release-only) + `develop` (integration) branch model. CI workflows still triggered on the previous `2.0-development` and `1.0-maintenance` names and never ran on `develop` itself — so PRs targeting `develop` got no checks and merges to `develop` had no post-merge verification. This is PR 1 of a two-PR plan from `workflow-review-plan.md`. PR 2 hardens `release.yml` with a branch guard. ## Test plan - [ ] `Lint`, `Type Check`, `Test (Python 3.10..3.13)`, `Docs`, `Build`, `Validate Commit Messages` all run on this PR (proves the new `develop` triggers fire). - [ ] No remaining stale branch references: `grep -rn "1.0-maintenance\|2.0-development\|2.0-auth\|feature/async\|feature/StatCalc" .github/` returns empty. - [ ] After merge, `Lint` etc. run on the develop branch tip (proves the push trigger fires). - [ ] Once merged and job names are stable, set required status checks on `main` per `workflow-review-plan.md`. ## Notes for reviewers - `test.yml` deletion is safe — branch protection on `main` currently has `contexts: []`, so it is not a required check anywhere. - `labeler.yml` was intentionally not touched: it triggers on PR open/sync with no branch list, which is correct. - `release.yml` branch guard is deliberately deferred to a follow-up PR (different risk profile, needs manual dispatch test). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.7 (1M context) --- .github/workflows/ci.yml | 4 ++-- .github/workflows/commitlint.yml | 7 +++---- .github/workflows/integration.yml | 4 ++-- .github/workflows/test.yml | 35 ------------------------------- 4 files changed, 7 insertions(+), 43 deletions(-) delete mode 100644 .github/workflows/test.yml diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6a22df1..f70f28f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -2,9 +2,9 @@ name: CI on: pull_request: - branches: [main, 1.0-maintenance, 2.0-development] + branches: [main, develop] push: - branches: [main] + branches: [main, develop] permissions: contents: read diff --git a/.github/workflows/commitlint.yml b/.github/workflows/commitlint.yml index 0d46996..28bb56b 100644 --- a/.github/workflows/commitlint.yml +++ b/.github/workflows/commitlint.yml @@ -2,7 +2,7 @@ name: Commit Lint on: pull_request: - branches: [main, 1.0-maintenance] + branches: [main, develop] permissions: contents: read @@ -30,9 +30,8 @@ jobs: "chore", "docs", "test", "style", "ci", "perf" ]], "scope-enum": [1, "always", [ - "core", "helpers", "deps", "release", "ci", - "version", "hmac", "docs", "tests", "examples", - "async", "statcalc" + "core", "helpers", "deps", "release", "ci", + "version", "hmac", "docs", "tests", "examples" ]], "subject-max-length": [1, "always", 100], "header-max-length": [0, "always"], diff --git a/.github/workflows/integration.yml b/.github/workflows/integration.yml index a767e88..91d0463 100644 --- a/.github/workflows/integration.yml +++ b/.github/workflows/integration.yml @@ -2,9 +2,9 @@ name: Integration Tests on: pull_request: - branches: [main, 1.0-maintenance] + branches: [main, develop] push: - branches: [main] + branches: [main, develop] workflow_dispatch: permissions: diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml deleted file mode 100644 index 36aee02..0000000 --- a/.github/workflows/test.yml +++ /dev/null @@ -1,35 +0,0 @@ -name: Test - -on: - pull_request: - push: - branches: - - main - -permissions: - contents: read - -jobs: - test: - runs-on: ubuntu-latest - - steps: - - name: Checkout - uses: actions/checkout@v6 - - - name: Setup Python - uses: actions/setup-python@v6 - with: - python-version: "3.12" - - - name: Setup uv - uses: astral-sh/setup-uv@v7 - - - name: Sync dependencies - run: uv sync --group dev --group docs - - - name: Run unit tests - run: uv run pytest -q - - - name: Build docs - run: uv run mkdocs build From 8c0aa3a932c8636e6f855d2e4f6de396d0821cda Mon Sep 17 00:00:00 2001 From: MarTrepodi Date: Sun, 10 May 2026 08:02:29 -0400 Subject: [PATCH 2/6] ci(release): guard release workflow against non-main dispatch (#86) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Add `if: github.ref == 'refs/heads/main'` to both jobs in `release.yml` so that an accidental `workflow_dispatch` from `develop` or a feature branch exits without bumping the version, tagging, or publishing to PyPI. ## Why `release.yml` currently only has `on: workflow_dispatch:` with no branch restriction. With the new branch model (`develop` for integration, `main` for releases only), it would be easy to dispatch the release workflow from `develop` and accidentally cut a release from un-merged integration work. The guard is applied per-job because workflow-level `if:` is not valid for `workflow_dispatch`. ## Why not migrate to tag-based triggers The original review document suggested replacing `workflow_dispatch` with `on: push: tags: ['v*']`. That is correct for workflows that publish in *response* to tags created elsewhere, but this workflow *creates* the tag itself (line 54: `git tag -a "v$VERSION"` and `git push --follow-tags`). Reacting to its own tag would be circular, or require splitting the tag-creation step out into a separate workflow without meaningful benefit. The dispatch-with-guard pattern is the right fit. ## Test plan - [ ] After merge, dispatch from a non-main branch: `gh workflow run release.yml --ref ` — every job should be skipped (`if` evaluates false). - [ ] Dispatch from `main` should run normally (do not test the publish path until ready for an actual release; consider commenting out the `pypi-publish` step in a fork first if a smoke test is desired). - [ ] No regression to the `on: workflow_dispatch:` trigger itself — the workflow still appears in the GitHub Actions UI and accepts manual dispatch. ## Notes - Action `uses:` lines are intentionally not pinned to SHA in this PR. The repository already configures Dependabot for `github-actions` with `target-branch: develop`, which will pin to SHA organically over the next several update cycles. - Independent of [#85](https://github.com/swgoh-utils/comlink-python/pull/85) (the trigger-alignment PR). Either order of merge works. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.7 (1M context) --- .github/workflows/release.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index a6c9478..0db7e3f 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -7,6 +7,7 @@ jobs: release: name: Semantic Release runs-on: ubuntu-latest + if: github.ref == 'refs/heads/main' concurrency: group: ${{ github.workflow }}-release-${{ github.ref_name }} @@ -70,6 +71,7 @@ jobs: # Publish the package to PyPI pypi-publish: runs-on: ubuntu-latest + if: github.ref == 'refs/heads/main' needs: - release permissions: From 51235c1c6bfaf54ef8521f44529a10c6ac5e3db1 Mon Sep 17 00:00:00 2001 From: MarTrepodi Date: Sat, 9 May 2026 16:48:25 -0400 Subject: [PATCH 3/6] chore(dependabot): set target branch to `develop`, limit open PRs to 10 --- .github/dependabot.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/dependabot.yml b/.github/dependabot.yml index bfc4b4c..9f48cb9 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -4,6 +4,7 @@ updates: directory: "/" schedule: interval: "weekly" + target-branch: "develop" labels: - "dependencies" - "ci" @@ -12,5 +13,7 @@ updates: directory: "/" schedule: interval: "weekly" + target-branch: "develop" + open-pull-requests-limit: 10 labels: - "dependencies" From e8a6e05d997559c23734dda5fee2c44cd172d2e8 Mon Sep 17 00:00:00 2001 From: MarTrepodi Date: Sat, 9 May 2026 17:45:04 -0400 Subject: [PATCH 4/6] fix(helpers): handle multi-day offsets in get_arena_payout Replace the single if-check with a while-loop so that payouts shifted into the past by offsets of 24+ hours are advanced day-by-day until they land in the future. Before this fix, calling get_arena_payout(offset=1440) when the wall clock was past the configured payout hour would return today at 18:00 (squad) or 19:00 (fleet), a time already in the past, violating the documented contract that the result is always in the future. The TestGetArenaPayoutEdge.test_payout_already_passed_adds_day test covers this case and now passes regardless of time of day. Fixes #87 Co-Authored-By: Claude Opus 4.7 (1M context) --- src/swgoh_comlink/helpers/_arena.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/swgoh_comlink/helpers/_arena.py b/src/swgoh_comlink/helpers/_arena.py index ac11b3b..8ea8f45 100644 --- a/src/swgoh_comlink/helpers/_arena.py +++ b/src/swgoh_comlink/helpers/_arena.py @@ -52,6 +52,6 @@ def get_arena_payout(offset: int, fleet: bool = False) -> datetime: else: payout = payout.replace(hour=18, minute=0, second=0, microsecond=0) payout = payout - timedelta(minutes=(offset + local_offset)) - if payout < datetime.now(): + while payout < datetime.now(): payout = payout + timedelta(days=1) return payout From c9e3f59b91a51137867f766f448d7deda07e90b0 Mon Sep 17 00:00:00 2001 From: MarTrepodi Date: Sun, 10 May 2026 08:06:17 -0400 Subject: [PATCH 5/6] fix(helpers): ensure arena payout time adjusts correctly when shifted to past --- src/swgoh_comlink/helpers/_arena.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/swgoh_comlink/helpers/_arena.py b/src/swgoh_comlink/helpers/_arena.py index 8ea8f45..e2e52d4 100644 --- a/src/swgoh_comlink/helpers/_arena.py +++ b/src/swgoh_comlink/helpers/_arena.py @@ -52,6 +52,7 @@ def get_arena_payout(offset: int, fleet: bool = False) -> datetime: else: payout = payout.replace(hour=18, minute=0, second=0, microsecond=0) payout = payout - timedelta(minutes=(offset + local_offset)) + # Loop until payout time is in the future in case payout time is adjusted to a past time while payout < datetime.now(): payout = payout + timedelta(days=1) return payout From 75feb6701648cbaf986624bb90532b8fe24d443a Mon Sep 17 00:00:00 2001 From: MarTrepodi Date: Sun, 10 May 2026 09:07:48 -0400 Subject: [PATCH 6/6] fix(tests): switch HMAC rejection tests from GET to POST endpoint (#89) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #88 ## Summary The four HMAC rejection tests in `tests/integration/test_hmac.py` (`test_hmac_no_key_rejected`, `test_hmac_wrong_key_rejected`, and their async variants) were calling `client.get_enums()`, which is a **GET** request to `/enums`. The Comlink HMAC service does not enforce HMAC on GET endpoints — only on POST — so the server returned 200 OK regardless of authentication and the expected `SwgohComlinkException` was never raised. All four tests failed with `Failed: DID NOT RAISE`. This PR switches those tests to call `client.get_player_arena(allycode=TEST_ALLYCODE, player_details_only=True)`, a POST request to `/playerArena` (HMAC-protected). Uses the shared `TEST_ALLYCODE` constant from `conftest.py` so the rejection tests align with the existing `get_player` and `get_player_arena` success tests. ## Why this only surfaced now Integration tests never ran on PRs targeting `develop` until [#85](https://github.com/swgoh-utils/comlink-python/pull/85) landed and `integration.yml` started triggering on `develop` PRs. PRs to `main` had been skipping these tests because `develop` was not in the trigger list, so the latent bug went unnoticed. ## Verification Probed a live HMAC-protected Comlink container directly to confirm the diagnosis and exercised the new test code end-to-end: | Scenario | Endpoint | Result | |---|---|---| | no auth | `GET /enums` | 200 OK *(server doesn't protect GET — root cause)* | | no auth | `POST /playerArena` | 403 `HMACValidationError` (Authorization header missing) | | wrong secret | `POST /playerArena` | 403 `HMACValidationError` (HMAC validation failed) | | valid `test_key`/`test_secret` | `POST /playerArena` | 200 OK with real `playerDetailsOnly` response | All four updated tests `PASS` against the live container. ## Test plan - [ ] CI Integration Tests check passes on this PR (was failing on every PR since #85 merged due to this bug). - [ ] Other CI checks (`Lint`, `Type Check`, `Test (Python 3.10..3.13)`, `Docs`, `Build`, `Validate Commit Messages`) remain green. - [ ] No changes outside `tests/integration/test_hmac.py`; client and server code untouched. ## Notes - Production HMAC client behavior is correct and unchanged — verified by the existing `test_hmac_*_request_succeeds` tests (which continue to pass) and by the positive-control valid-auth probe in this PR's verification. - Bug was purely in test scaffolding. Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.7 (1M context) --- tests/integration/test_hmac.py | 37 ++++++++++++++++++++++++++-------- 1 file changed, 29 insertions(+), 8 deletions(-) diff --git a/tests/integration/test_hmac.py b/tests/integration/test_hmac.py index edb8b78..8d14154 100644 --- a/tests/integration/test_hmac.py +++ b/tests/integration/test_hmac.py @@ -14,6 +14,7 @@ COMLINK_HMAC_URL, HMAC_ACCESS_KEY, HMAC_SECRET_KEY, + TEST_ALLYCODE, ) pytestmark = pytest.mark.integration @@ -48,14 +49,24 @@ def test_hmac_sync_player_request_succeeds(comlink_hmac): @hmac_configured def test_hmac_no_key_rejected(): - """Sync client without HMAC keys is rejected by the protected endpoint.""" + """Sync client without HMAC keys is rejected by the protected endpoint. + + Uses a POST endpoint (`playerArena`) because the Comlink HMAC service + only enforces HMAC on POST; GET endpoints like `/enums` are + unauthenticated. + """ with SwgohComlink(url=COMLINK_HMAC_URL) as client, pytest.raises(SwgohComlinkException): - client.get_enums() + client.get_player_arena(allycode=TEST_ALLYCODE, player_details_only=True) @hmac_configured def test_hmac_wrong_key_rejected(): - """Sync client with wrong secret key is rejected by the protected endpoint.""" + """Sync client with wrong secret key is rejected by the protected endpoint. + + Uses a POST endpoint (`playerArena`) because the Comlink HMAC service + only enforces HMAC on POST; GET endpoints like `/enums` are + unauthenticated. + """ with ( SwgohComlink( url=COMLINK_HMAC_URL, @@ -64,7 +75,7 @@ def test_hmac_wrong_key_rejected(): ) as client, pytest.raises(SwgohComlinkException), ): - client.get_enums() + client.get_player_arena(allycode=TEST_ALLYCODE, player_details_only=True) # ── Async: valid HMAC ─────────────────────────────────────────────────── @@ -94,23 +105,33 @@ async def test_hmac_async_player_request_succeeds(async_comlink_hmac): @hmac_configured @pytest.mark.asyncio async def test_hmac_no_key_async_rejected(): - """Async client without HMAC keys is rejected by the protected endpoint.""" + """Async client without HMAC keys is rejected by the protected endpoint. + + Uses a POST endpoint (`playerArena`) because the Comlink HMAC service + only enforces HMAC on POST; GET endpoints like `/enums` are + unauthenticated. + """ async with SwgohComlinkAsync(url=COMLINK_HMAC_URL) as client: with pytest.raises(SwgohComlinkException): - await client.get_enums() + await client.get_player_arena(allycode=TEST_ALLYCODE, player_details_only=True) @hmac_configured @pytest.mark.asyncio async def test_hmac_wrong_key_async_rejected(): - """Async client with wrong secret key is rejected by the protected endpoint.""" + """Async client with wrong secret key is rejected by the protected endpoint. + + Uses a POST endpoint (`playerArena`) because the Comlink HMAC service + only enforces HMAC on POST; GET endpoints like `/enums` are + unauthenticated. + """ async with SwgohComlinkAsync( url=COMLINK_HMAC_URL, access_key=HMAC_ACCESS_KEY, secret_key="wrong_secret_key", ) as client: with pytest.raises(SwgohComlinkException): - await client.get_enums() + await client.get_player_arena(allycode=TEST_ALLYCODE, player_details_only=True) # ── HMAC header verification ────────────────────────────────────────────