chore(api): scoped Stryker.NET mutation testing (#298) - #308
Conversation
Pin dotnet-stryker 4.16.0 as a local tool with three project-scoped configs (Domain/Application/Infrastructure), each an include-only mutate allow-list over the billing/auth/date logic clusters. Add mutation.yml: per-PR (--since:origin/main) + nightly full in-scope, both report-only (break=0). Fix the .gitignore .config/ trap so the tool manifest is tracked; delete the empty LogoutAndRefreshSessionTests stub. Closes #298 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Complete
Scope: PR #308 in thomasluizon/orbit-api — "chore(api): scoped Stryker.NET mutation testing (#298)"
Recommendation: APPROVE
| Severity | Count |
|---|---|
| Critical (incl. |
0 |
| High | 0 |
| Medium | 0 |
| Low / Info | 0 (dropped per signal gate) |
What was checked
This is a pure CI/tooling PR — no src/ application code changed. Diff: .config/dotnet-tools.json (new, pins dotnet-stryker 4.16.0), .github/workflows/mutation.yml (new, per-PR --since matrix + nightly full-scope, both report-only via break: 0), three new stryker-config.*.json allow-lists (Domain/Application/Infrastructure), a .gitignore fix, and deletion of an empty (e69de29b, 0-byte) test stub.
Verified directly against the repo:
- Every file path in all three
mutateglob allow-lists exists exactly where claimed (Domain entities, Application subscription/auth/habit-schedule files, Infrastructure JWT/session/streak/due-date services). - All three referenced
.csproj(project + test-project) paths exist and match. .gitignorefix is correct:.config/→.config/*+!.config/dotnet-tools.jsonis the standard fix for the "can't negate inside an ignored directory" gitignore gotcha, and does make the tool manifest trackable. It also incidentally fixes a real pre-existing bug from the immediately-prior merge (f9f8c2c): a corrupted linecaddy_config/.claude/worktrees/is correctly split back intocaddy_config/(line 56) and the already-present.claude/worktrees/(line 59) — confirmed viagit log -pthat this concatenation was merge damage, not intentional.- Deleted test file was genuinely empty (git blob hash
e69de29b= 0 bytes), so no test coverage was lost. mutation.ymlaction versions (checkout@v7,setup-dotnet@v5) match what's already used intest.yml/sonarcloud.yml/claude-review.yml, so no version drift introduced.- Workflow logic is sound:
fetch-depth: 0+git fetch origin mainsupport--since:origin/main;if: github.event_name == '...'correctly splits PR (since-diff) vs. schedule (full-scope) runs;break: 0thresholds make this genuinely non-blocking as the PR description states.
One non-blocking observation (not a required-fix finding): in stryker-config.infrastructure.json, the five !**/Services/... exclusion lines are inert no-ops — the preceding mutate entries are exact filenames (not wildcards), so none of the excluded files (GooglePlayBillingService.cs, etc.) were ever matched by a positive glob in the first place. Harmless (behavior is correct either way, since this is already an include-only allow-list per the PR's own description), but a future editor could misread it as active filtering. Optional cleanup, not required for merge.
Dimensions 8/9/10/11/14 (DESIGN.md, web↔mobile parity, i18n, contract drift, FEATURES.md) are N/A — no apps/*, DTO, endpoint, or user-facing feature surface touched. Dimension 13 (backend hard rules) is N/A beyond the testing rule, which this PR directly strengthens.
Subagents
| Agent | Verdict |
|---|---|
| security-reviewer | N/A — gate requires src/ changes; none in this diff |
| contract-aligner | N/A — no DTO/Controller/packages/shared surface touched |
Validation
| Check | Result |
|---|---|
| Build (dotnet) | N/A — covered by required CI check |
| Tests (dotnet) | N/A — covered by required CI check |
No prior reviews, review comments, or resolved threads existed on this PR — this is the first automated review pass. No backward-compat concerns: no DTO or packages/shared schema touched, so no risk to shipped mobile clients.
Resolves the CodeQL missing-permissions alert. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Review Complete
Scope: PR #308 in thomasluizon/orbit-api — "chore(api): scoped Stryker.NET mutation testing (#298)"
Diff reviewed: since the last bot review (which approved commit d9d4ce2f), the only new commit is d29f71d4 — "ci: least-privilege GITHUB_TOKEN permissions on Mutation workflow", a 3-line addition to .github/workflows/mutation.yml.
Recommendation: APPROVE
Summary
The incremental diff since the last review adds a single permissions: contents: read block at the workflow root of .github/workflows/mutation.yml (lines 12-13). This scopes the GITHUB_TOKEN down from the (implicit, broader) default permissions to read-only, matching least-privilege CI practice. Verified against the full current file: no step in the mutation job (actions/checkout@v7, actions/setup-dotnet@v5, dotnet restore, dotnet tool restore, dotnet stryker, actions/upload-artifact@v7) requires any permission beyond contents: read — none post PR comments, write checks, or touch other GitHub APIs, and upload-artifact uses the separate Actions runtime token, not GITHUB_TOKEN. Placement is syntactically correct (workflow-level, before jobs:), and there's no conflicting job-level permissions: block. This is a pure security hardening with zero functional risk.
Findings
Critical: None
High: None
Medium: None
Low / Info:
- [Info] Correct, minimal least-privilege hardening. · dimension: Security (#12, config/dependency) · location:
.github/workflows/mutation.yml:12-13· reference: orbit-api hard rules / OWASP CI-hardening best practice.
Rubric dimensions
All other dimensions (Correctness beyond the security note, Dead code, SOLID, Comments, No-workaround, Type safety, console.log, DESIGN.md, Parity, i18n, Contract drift, Backend hard rules #13, FEATURES.md #14) are N/A — this incremental diff touches only a GitHub Actions workflow permissions block, no application/domain code, no DTOs, no UI.
Subagents
| Agent | Verdict |
|---|---|
| security-reviewer | N/A — diff is CI-workflow-only, not src/ code; manually verified permission scoping is sufficient and correctly placed |
| contract-aligner | N/A — no DTO/endpoint/contract surface touched |
Provenance note
Prior context checked: gh pr view --json reviews showed one prior claude review (APPROVED, covering commit d9d4ce2f) and one empty-body github-advanced-security COMMENTED review (no actionable text). GraphQL review-thread resolution status was not independently re-queried this run; no open threads surfaced via the channels that were accessible.
|
2 participants
Scoped mutation testing over the high-stakes modules. API analog of orbit-ui-mobile#417.
dotnet-stryker4.16.0 as a local tool (.config/dotnet-tools.json); fixes the.gitignore.config/trap so the manifest is tracked.stryker-config.json(Domain/Application/Infrastructure), each an include-onlymutateallow-list over billing (Play verify/RTDN), auth/token, and date/streak/overdue logic. I/O adapters (GooglePlayBillingService, GoogleTokenService, etc.) excluded so the score reflects testable logic.mutation.yml: per-PR--since:origin/mainmatrix + nightly full-scope, both report-only (break: 0). A follow-up flipsbreakblocking after this + Testing: property-based tests (FsCheck.Xunit) for domain invariants #299 merge and the baseline is captured. Deletes an empty test stub.Merges after #299. Validated:
dotnet buildgreen; all 3 Stryker configs validated with real--sinceruns (mutate globs resolve to exactly the intended files).🤖 Generated with Claude Code