Develop - #92
Closed
MarTrepodi wants to merge 7 commits into
Closed
Conversation
## 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) <noreply@anthropic.com>
## 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 <feature-branch>` — 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](#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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
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](#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) <noreply@anthropic.com>
MarTrepodi
enabled auto-merge (squash)
May 15, 2026 13:28
auto-merge was automatically disabled
May 15, 2026 13:30
Pull request was closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Related Issues
Type of Change
Checklist
feat:,fix:,refactor:, etc.)python -m pytest tests/ -v)ruff check src/ tests/)Testing