TeamsService, MaintenancePoolService, and UsersService have zero test coverage despite directly controlling money movement and payout-address changes

[chonilius]

Overview

Three services that directly control money movement or a security-sensitive identity field have zero test coverage. Confirm by listing every .spec.ts file in the repo:

src/app.controller.spec.ts
src/bounties/bounties.service.spec.ts
src/bounties/bounty-state-machine.spec.ts
src/common/filters/global-exception.filter.integration.spec.ts
src/common/filters/global-exception.filter.spec.ts
src/common/idempotency/idempotency-cleanup.service.spec.ts
src/common/idempotency/idempotency.interceptor.spec.ts
src/database/escrow-fk-integrity.integration.spec.ts
src/escrow/escrow-response.mapper.spec.ts
src/escrow/escrow.controller.spec.ts
src/escrow/escrow.service.spec.ts
src/github/github-sync.service.spec.ts
src/github/github-webhooks.service.spec.ts
src/github/octokit.provider.spec.ts
src/github/webhook-signature.util.spec.ts
src/milestones/milestones.service.spec.ts
src/sponsors/sponsors.service.spec.ts
src/teams/team-split.util.spec.ts

src/teams/teams.service.ts, src/maintenance-pool/maintenance-pool.service.ts, and src/users/users.service.ts are entirely absent — not one test exists for any of them, despite each owning behavior this exact issue batch has found real, exploitable bugs in:

• TeamsService.assignToBounty (companion issue in this batch: no bounty-status or ownership guard, can redirect a bounty's entire payout to an attacker-controlled team) has zero tests covering it or any other method on the service. team-split.util.spec.ts only covers the pure percentage-math helper functions (validateSplitPercentages, computeSplitShares) — it never touches TeamsService.create, findOne, or assignToBounty themselves.
• MaintenancePoolService.deposit/assignReward (companion issues: creates orphaned, unreachable Escrow rows on repeat deposits; pool.balance has an unguarded lost-update race) has zero tests despite directly locking and releasing real funds, on a service whose own doc comment describes it as letting "maintainers assign rewards out of the running balance... without needing to create a one-off bounty."
• UsersService.setStellarAddress/upsertFromGithub (companion issue: setStellarAddress is authenticated but not authorized, letting any user overwrite another's payout address) has zero tests, despite stellarAddress being the single field that determines where every solo-recipient bounty payout in the entire system physically goes.

This gap is distinct in scope from the existing open "Add comprehensive test coverage for the bounty state machine, escrow release math, and webhook-to-payout integration flow" issue in this repo, which names those three areas explicitly and doesn't mention teams, maintenance pools, or users anywhere in its text — none of the fixes that issue would produce touch these three files. It's also worth noting as a pattern: every file in the "no tests at all" category above (teams.service.ts, maintenance-pool.service.ts, users.service.ts, and — per the companion auth-module testing issue — everything under src/auth) is also a file this batch found a real, shippable bug in. That's not a coincidence worth ignoring: untested code in this codebase has consistently turned out to be where the actual problems live.

Requirements

• Add teams.service.spec.ts: create (including the interaction with validateSplitPercentages), findOne, and — most importantly — assignToBounty, covering both its current unguarded behavior (as a baseline/regression test to make the companion status-guard fix's own tests meaningful against a known-good "before" state) and, once that companion fix lands, the new guard's rejection paths.
• Add maintenance-pool.service.spec.ts: create, deposit (including a repeat-deposit test that would catch the companion "orphaned escrow" bug), assignReward (including a concurrent-call test that would catch the companion "balance lost-update race" bug), and list.
• Add users.service.spec.ts: findById, findByUsername, upsertFromGithub (the account-linking logic the companion OAuth-hardening issue also touches — coordinate rather than duplicate), addRole, and setStellarAddress (including, once the companion IDOR fix lands, a cross-user rejection test).
• Where a companion issue in this batch proposes a specific fix to one of these services, the test added here should be written so it fails against the pre-fix code (proving the bug) and passes post-fix — matching this repo's own established convention (see e.g. the existing escrow-fk-integrity.integration.spec.ts, built specifically to validate the FK-hardening migration) of tests that document a real, previously-unverified behavior rather than only asserting the happy path.

Acceptance Criteria

• teams.service.spec.ts, maintenance-pool.service.spec.ts, and users.service.spec.ts exist with meaningful coverage of every public method on their respective services.
• At least one test per file is written to fail against the current, pre-fix behavior of a real bug identified elsewhere in this batch (the assignToBounty guard gap, the orphaned-escrow deposit bug, the setStellarAddress IDOR) — coordinate sequencing with those issues so this isn't duplicated work, but don't let this issue be satisfied by happy-path-only tests that never would have caught the actual bugs.
• Running npm run test:cov shows non-zero, meaningful coverage for all three files where it previously showed none.

Additional Notes

Precise references: absence of src/teams/teams.service.spec.ts, src/maintenance-pool/maintenance-pool.service.spec.ts, src/users/users.service.spec.ts (confirmed via find src -name '*.spec.ts' — none of the three appear). src/teams/teams.service.ts:52-59 (assignToBounty), src/maintenance-pool/maintenance-pool.service.ts:45-107 (deposit, assignReward), src/users/users.service.ts:92-99 (setStellarAddress). src/teams/team-split.util.spec.ts (existing coverage, confirmed to be scoped to the pure utility functions only, not TeamsService itself — grep its describe blocks to confirm: they reference validateSplitPercentages/computeSplitShares, never TeamsService).

Test/reproduction plan: follow the existing house style established in escrow.service.spec.ts/milestones.service.spec.ts/sponsors.service.spec.ts — a Test.createTestingModule with mocked getRepositoryToken(...) providers and a mocked EscrowService/SorobanClientService where relevant, rather than hitting a real database (leave that tier to the existing .integration.spec.ts pattern if warranted for the FK-sensitive maintenance-pool orphaning scenario specifically, mirroring escrow-fk-integrity.integration.spec.ts).

Cross-references: scoped distinctly from the existing open "Add comprehensive test coverage for the bounty state machine, escrow release math, and webhook-to-payout integration flow" issue (different files entirely) and from the companion "auth module zero test coverage" issue in this batch (different module, though both point at the same underlying pattern of "untested code is where this batch's real bugs were found"). Directly supports and should be sequenced alongside the companion assignToBounty, MaintenancePool orphaned-escrow/balance-race, and setStellarAddress IDOR issues — the tests this issue asks for are the regression proof those fixes need.

[yunus-dev-codecrafter]

Hi! I've carefully reviewed the entire task description and fully understand the requirements and expected outcome.
I have experience working on similar tasks and I'm confident I can complete this within about an hour. I'd be happy to take ownership of this issue and submit a clean, well-tested solution promptly.
If it's available, please assign it to me. Thank you!

[davidishere1]

I’m interested in this issue because services responsible for money movement and security-sensitive identity data should have strong automated test coverage. Missing tests in these areas create unnecessary risk and make future changes harder to validate confidently.

I have 5+ years of programming experience and a strong focus on writing reliable, maintainable software. I’m comfortable analyzing existing codebases, identifying critical paths, and adding focused unit tests that cover both expected behavior and important edge cases. I’d start by reviewing the existing .spec.ts files and the three affected services, then add comprehensive tests while keeping the implementation behavior unchanged.

[davidishere1]

I’m interested in this issue because services responsible for money movement and security-sensitive identity data should have strong automated test coverage. Missing tests in these areas create unnecessary risk and make future changes harder to validate confidently.

I have 5+ years of programming experience and a strong focus on writing reliable, maintainable software. I’m comfortable analyzing existing codebases, identifying critical paths, and adding focused unit tests that cover both expected behavior and important edge cases. I’d start by reviewing the existing .spec.ts files and the three affected services, then add comprehensive tests while keeping the implementation behavior unchanged.

[grantfox-oss]

🦊 GrantFox — @davidishere1 has been assigned to this issue as part of the Third Campaign campaign!

Next steps:

1. Open a Pull Request referencing this issue (e.g., Closes #61)
2. Your PR will be reviewed by the MergeFi maintainers

Good luck! Track your progress on GrantFox.