[Regression on closed #102 / #367] markMergedAndRelease marks a team bounty PAID when the team has no splits or no longer exists, skipping the payout entirely

Abierto
  • ¿Algo no está bien?
#746 0 comentarios 0 reacciones 0 asignados Ver en GitHub

Los mantenedores suelen responder en 1 día

Nadie ha tomado este issue todavía.

Evaluación

Dificultad
3/5
Tiempo estimado
1-2 días
Aptitud para principiantes
72/100
Tipo de issue
Error
Claridad
Bien especificado
Estado de actividad
Activo
Stack tecnológico
node.js, typescript
Área
backend-api-design, payments, testing

Línea de trabajo

Start with BountiesService.markMergedAndRelease in src/bounties/bounties.service.ts and the existing cases in src/bounties/bounties.service.spec.ts. Check how skipped payouts are handled by the RELEASE_FAILED transition in src/bounties/bounty-state-machine.ts. Add coverage for empty team splits, a missing team, and a bounty with no recipient; done when each case leaves the bounty retryable and does not mark it PAID.

Escrito por el modelo de indexación a partir del texto del issue.

Descripción

bug

Location

src/bounties/bounties.service.ts:191-238 — BountiesService.markMergedAndRelease

try {
  if (bounty.teamId) {
    const team = await this.teamRepo.findOne({
      where: { id: bounty.teamId },
      relations: { splits: true },
    });
    if (team && team.splits.length > 0) {
      ...
      await this.escrowService.splitRelease(bounty.escrowId, recipients);
    }
  } else if (bounty.claimedById) {
    ...
    await this.escrowService.release(bounty.escrowId, contributor?.stellarAddress ?? '', bounty.claimedById);
  }
} catch (err) { ... }

assertTransition(bounty.status, BountyStatus.PAID);
bounty.status = BountyStatus.PAID;
bounty.paidAt = new Date();

Problem

The if (team && team.splits.length > 0) guard has no else. When it is false, the whole payout is skipped, no exception is thrown, control falls through to line 233, and the bounty is written as PAID with paidAt set — while the escrow is still LOCKED and no Payment row exists.

Three distinct inputs reach that branch:

  1. team.splits is empty — a team created or edited into a zero-split state.
  2. team is null — bounty.teamId points at a row that no longer exists. Bounty.teamId is a nullable FK with no cascade back to the bounty, so a deleted team leaves the id dangling.
  3. The else if (bounty.claimedById) arm is likewise silently skipped when a bounty has neither teamId nor claimedById — reachable because IN_REVIEW is driven by webhook from PR linkage, not by a claim (see #385).

This is the defect closed as #102 and re-filed and closed as #367. The fix is not present on main.

Why it matters

PAID is terminal in the state machine — BOUNTY_TRANSITIONS[BountyStatus.PAID] = [] (src/bounties/bounty-state-machine.ts:38). So the bounty is unrecoverable through the API:

  • it cannot be retried (PAID -> MERGED is not a legal transition, and markMergedAndRelease would reject at its own assertTransition(PAID, MERGED) on line 179),
  • it cannot be refunded (PAID -> REFUNDED is not legal either),
  • the escrow stays LOCKED indefinitely, so the sponsor's funds are stranded on-chain with no route that can move them.

Meanwhile every downstream consumer reads the bounty as settled: PAID + paidAt is what the sponsor dashboard's spend aggregate, the analytics payout figures, and reputation all key off. So the platform reports the money as paid out to the team while the contributors received nothing and the sponsor cannot get it back. The contributor has done the work, the PR is merged on GitHub, and there is no state left from which to pay them.

The RELEASE_FAILED escape hatch that exists precisely for this situation (line 227-230, and RELEASE_FAILED -> MERGED retry on bounty-state-machine.ts:37) is never reached, because nothing threw — the payout was skipped rather than attempted.

There is no test coverage for the branch: src/bounties/bounties.service.spec.ts exercises teamId: null (line 185) and teamId: 'team-1' with populated splits (line 208), but nothing with an empty splits array or a missing team.

Reproduction / failure path

Empty splits

  1. POST /teams creating team T, then remove its splits (or create it in a state where splits is empty — TeamsService.updateSplits' delete-then-insert window also exposes this, cf. #375/#536).
  2. POST /bounties → B; POST /bounties/B/fund → escrow E LOCKED; assign T to B so B.teamId = T.
  3. Claim, open a PR, merge it. The webhook drives markMergedAndRelease(B).
  4. team.splits.length === 0 → splitRelease is never called.
  5. Result: B.status = 'paid', B.paidAt set, E.status = 'LOCKED', zero Payment rows for E. GET /bounties/B reports a paid bounty.

Missing team

  1. Same setup, then delete team T from the teams table (directly, or via any future delete endpoint).
  2. Merge the PR → teamRepo.findOne returns null → same outcome.

Suggested fix

Make the unpayable case an explicit failure rather than a fall-through, so the existing RELEASE_FAILED retry path handles it:

if (bounty.teamId) {
  const team = await this.teamRepo.findOne({
    where: { id: bounty.teamId },
    relations: { splits: true },
  });
  if (!team) {
    throw new BadRequestException(
      `Bounty ${bounty.id} references team ${bounty.teamId}, which no longer exists`,
    );
  }
  if (team.splits.length === 0) {
    throw new BadRequestException(
      `Team ${team.id} has no configured splits — cannot release bounty ${bounty.id}`,
    );
  }
  ...
  await this.escrowService.splitRelease(bounty.escrowId, recipients);
} else if (bounty.claimedById) {
  ...
} else {
  throw new BadRequestException(
    `Bounty ${bounty.id} has neither a team nor a claimant — no payout recipient`,
  );
}

Because these throws happen inside the existing try, the bounty lands in RELEASE_FAILED (not PAID), stays refundable and retryable, and the escrow is still releasable once the team's splits are repaired.

Additionally, guard at the source so the bad state is harder to create: reject assigning a team with zero splits to a bounty in TeamsService.assignToBounty, and require splits.length > 0 whenever a team's splits are rewritten.

Tests to add in src/bounties/bounties.service.spec.ts: merge a team bounty whose splits is [], merge one whose teamRepo.findOne resolves null, and merge one with neither teamId nor claimedById — each asserting the bounty ends in RELEASE_FAILED, that escrowService.splitRelease/release was not called, and that PAID was never written.

Lenguaje dominante
TypeScript
Estrellas
2
Forks
90
Merge medio
31 min
PR fusionados (30 d)
20

Preparar el entorno

  • Incluye un Dockerfile o un archivo de Docker Compose
  • Sin plantilla de pull request
  • Sin guía de contribución

Primeros pasos

  1. Lee el issue completo y luego la guía de contribución del proyecto.
  2. Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
  3. Haz un fork del repositorio y trabaja en una rama.
  4. Abre un pull request que haga referencia al número del issue.

Más de MergeFi/backend

Todos los issues de MergeFi/backend

Issues similares

Más issues de TypeScript

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.