feat(milestones): POST /:id/close — close + refund the unspent balance

src/escrow/escrow.service.ts

@@ -584,20 +584,14 @@ export class EscrowService {
     recipients: Array<[string, number]>,
     manager?: EntityManager,
   ): Promise<ContractInvocationResult> {
-    return this.invokeOnLockedEscrow(escrow, operation, () =>
-      this.soroban.invoke(
-        'release',
-        // `release(issue_id: u64, recipients)` — u64-typed on-chain (#301).
-        [u64(this.onChainKeyFor(escrow)), recipients],
-        this.contractOpts(escrow),
-      ),
     return this.invokeOnLockedEscrow(
       escrow,
       operation,
       () =>
         this.soroban.invoke(
           'release',
-          [this.onChainKeyFor(escrow), recipients],
+          // `release(issue_id: u64, recipients)` — u64-typed on-chain (#301).
+          [u64(this.onChainKeyFor(escrow)), recipients],
           this.contractOpts(escrow),
         ),
       manager,

src/github/github-webhooks.service.spec.ts

@@ -869,6 +869,9 @@ describe('GithubWebhooksService', () => {
 
       expect(bountiesService.markInReview).not.toHaveBeenCalled();
       expect(bountiesService.markMergedAndRelease).not.toHaveBeenCalled();
+    });
+  });
+
   // #308: a redelivered X-GitHub-Delivery used to hit the unique constraint
   // on webhook_events.deliveryId and escape handleEvent as a 500, so every
   // redelivery of that event failed forever.

src/milestones/milestones.controller.spec.ts

@@ -1,12 +1,15 @@
+import type { Request } from 'express';
 import { Test, TestingModule } from '@nestjs/testing';
 import { plainToInstance } from 'class-transformer';
 import { validate } from 'class-validator';
 import { MilestonesController } from './milestones.controller';
 import { MilestonesService } from './milestones.service';
 import { CreateMilestoneDto } from './dto/create-milestone.dto';
 import { AssetType, MilestoneStatus } from '../common/enums';
+import { getRepositoryToken } from '@nestjs/typeorm';
 import { JwtAuthGuard } from '../auth/guards/jwt-auth.guard';
 import { RolesGuard } from '../auth/guards/roles.guard';
+import { IdempotencyKey } from '../common/entities/idempotency-key.entity';
 
 describe('MilestonesController', () => {
   let controller: MilestonesController;
@@ -33,7 +36,13 @@ describe('MilestonesController', () => {
 
     const module: TestingModule = await Test.createTestingModule({
       controllers: [MilestonesController],
-      providers: [{ provide: MilestonesService, useValue: milestonesService }],
+      providers: [
+        { provide: MilestonesService, useValue: milestonesService },
+        {
+          provide: getRepositoryToken(IdempotencyKey),
+          useValue: { findOne: jest.fn(), save: jest.fn(), create: jest.fn() },
+        },
+      ],
     })
       .overrideGuard(JwtAuthGuard)
       .useValue({ canActivate: () => true })
@@ -53,9 +62,14 @@ describe('MilestonesController', () => {
         asset: AssetType.USDC,
       };
 
-      await controller.create(dto);
+      await controller.create(dto, {
+        user: { userId: 'sponsor-1' },
+      } as unknown as Request);
 
-      expect(milestonesService.create).toHaveBeenCalledWith(dto);
+      expect(milestonesService.create).toHaveBeenCalledWith(
+        dto,
+        'sponsor-1',
+      );
     });
   });
 
@@ -77,9 +91,15 @@ describe('MilestonesController', () => {
 
   describe('fund', () => {
     it('calls milestonesService.fund with id and funderAddress', async () => {
-      await controller.fund('m1', { funderAddress: 'GFUNDER' });
-
-      expect(milestonesService.fund).toHaveBeenCalledWith('m1', 'GFUNDER');
+      await controller.fund('m1', { funderAddress: 'GFUNDER' }, {
+        user: { userId: 'sponsor-1' },
+      } as unknown as Request);
+
+      expect(milestonesService.fund).toHaveBeenCalledWith(
+        'm1',
+        'GFUNDER',
+        'sponsor-1',
+      );
     });
   });
 

src/milestones/milestones.controller.ts

@@ -143,4 +143,19 @@ export class MilestonesController {
   allocateBudget(@Param('id', new ParseUUIDPipe()) id: string) {
     return this.milestonesService.allocateBudget(id);
   }
+
+  @ApiOperation({
+    summary: 'Close a milestone and refund its unspent balance',
+  })
+  @ApiBearerAuth()
+  @ApiStandardErrorResponses()
+  @UseGuards(JwtAuthGuard)
+  @Post(':id/close')
+  close(@Param('id', new ParseUUIDPipe()) id: string, @Req() req: Request) {
+    // There was no way to reach MilestoneStatus.CLOSED or refund the unspent
+    // escrow — a sponsor couldn't reclaim an unspent budget (#587). Sponsor-
+    // only; the service enforces it.
+    const userId = (req.user as { userId?: string }).userId;
+    return this.milestonesService.close(id, userId);
+  }
 }

src/milestones/milestones.service.spec.ts

@@ -1,11 +1,12 @@
 import { Test, TestingModule } from '@nestjs/testing';
-import { BadRequestException } from '@nestjs/common';
+import { BadRequestException, ForbiddenException } from '@nestjs/common';
 import { getRepositoryToken } from '@nestjs/typeorm';
 import { DataSource } from 'typeorm';
 import { MilestonesService } from './milestones.service';
 import { EscrowService } from '../escrow/escrow.service';
 import { Issue, Milestone } from '../common/entities';
 import { AssetType, MilestoneStatus } from '../common/enums';
+import { InvalidMilestoneTransitionError } from './milestone-state-machine';
 
 describe('MilestonesService', () => {
   let service: MilestonesService;
@@ -21,6 +22,7 @@ describe('MilestonesService', () => {
   let escrowService: {
     fund: jest.Mock;
     releasePartial: jest.Mock;
+    refund: jest.Mock;
   };
   let dataSource: {
     transaction: jest.Mock;
@@ -43,6 +45,7 @@ describe('MilestonesService', () => {
         amount: '100',
         status: 'confirmed',
       }),
+      refund: jest.fn().mockResolvedValue(undefined),
     };
     dataSource = {
       transaction: jest.fn().mockImplementation((fn: Function) =>
@@ -75,7 +78,7 @@ describe('MilestonesService', () => {
         sponsorId: 'sponsor-1',
       });
 
-      const milestone = await service.fund('m1', 'GFUNDER');
+      const milestone = await service.fund('m1', 'GFUNDER', 'sponsor-1');
 
       expect(milestone.status).toBe(MilestoneStatus.FUNDED);
       expect(milestone.escrowId).toBe('escrow-1');
@@ -90,7 +93,7 @@ describe('MilestonesService', () => {
         sponsorId: 'sponsor-1',
       });
 
-      await service.fund('m1', 'GFUNDER');
+      await service.fund('m1', 'GFUNDER', 'sponsor-1');
 
       expect(escrowService.fund).toHaveBeenCalledWith(
         expect.objectContaining({
@@ -110,7 +113,7 @@ describe('MilestonesService', () => {
         sponsorId: null,
       });
 
-      await service.fund('m2', 'GFUNDER');
+      await service.fund('m2', 'GFUNDER', 'sponsor-1');
 
       expect(escrowService.fund).toHaveBeenCalledWith(
         expect.objectContaining({ sponsorId: null }),
@@ -123,7 +126,7 @@ describe('MilestonesService', () => {
         status: MilestoneStatus.FUNDED,
       });
 
-      await expect(service.fund('m1', 'GFUNDER')).rejects.toThrow(
+      await expect(service.fund('m1', 'GFUNDER', 'sponsor-1')).rejects.toThrow(
         'Milestone m1 is not OPEN (current: funded)',
       );
     });
@@ -263,6 +266,7 @@ describe('MilestonesService', () => {
         '250.0000000',
         'RECIPIENT_ADDR',
         'user-1',
+        expect.anything(),
       );
       expect(payment.id).toBe('payment-1');
       expect(dataSource.transaction).toHaveBeenCalled();
@@ -341,7 +345,7 @@ describe('MilestonesService', () => {
 
       await expect(
         service.resolveIssue('m1', 'i1', 'RECIPIENT_ADDR'),
-      ).rejects.toThrow('Milestone m1 is not accepting distributions');
+      ).rejects.toThrow(InvalidMilestoneTransitionError);
     });
 
     it('rejects when the issue does not belong to the milestone (#114)', async () => {
@@ -406,7 +410,53 @@ describe('MilestonesService', () => {
         '333.3333333',
         'RECIPIENT_ADDR',
         undefined,
+        expect.anything(),
       );
     });
   });
+
+  describe('close', () => {
+    it('refunds the escrow and moves the milestone to CLOSED', async () => {
+      milestoneRepo.findOne.mockResolvedValue({
+        id: 'm1',
+        status: MilestoneStatus.FUNDED,
+        escrowId: 'escrow-1',
+        sponsorId: 'sponsor-1',
+      });
+
+      const closed = await service.close('m1', 'sponsor-1');
+
+      expect(escrowService.refund).toHaveBeenCalledWith('escrow-1');
+      expect(closed.status).toBe(MilestoneStatus.CLOSED);
+      expect(milestoneRepo.save).toHaveBeenCalled();
+    });
+
+    it('rejects a caller that is not the milestone sponsor (#587)', async () => {
+      milestoneRepo.findOne.mockResolvedValue({
+        id: 'm1',
+        status: MilestoneStatus.FUNDED,
+        escrowId: 'escrow-1',
+        sponsorId: 'sponsor-1',
+      });
+
+      await expect(service.close('m1', 'mallory')).rejects.toThrow(
+        ForbiddenException,
+      );
+      expect(escrowService.refund).not.toHaveBeenCalled();
+    });
+
+    it('skips the escrow refund when nothing was funded', async () => {
+      milestoneRepo.findOne.mockResolvedValue({
+        id: 'm1',
+        status: MilestoneStatus.OPEN,
+        escrowId: null,
+        sponsorId: 'sponsor-1',
+      });
+
+      const closed = await service.close('m1', 'sponsor-1');
+
+      expect(escrowService.refund).not.toHaveBeenCalled();
+      expect(closed.status).toBe(MilestoneStatus.CLOSED);
+    });
+  });
 });

src/milestones/milestones.service.ts

@@ -130,6 +130,30 @@ export class MilestonesService {
     return this.issueRepo.save(issue);
   }
 
+  /**
+   * Sponsor-only close — transitions the milestone to CLOSED and refunds the
+   * escrow's remaining unspent balance. MilestoneStatus.CLOSED existed and the
+   * state machine allowed (OPEN|FUNDED|IN_PROGRESS) -> CLOSED, but no service/
+   * controller path reached it (#587) — a sponsor had no way to reclaim the
+   * unspent part of a milestone budget.
+   */
+  async close(milestoneId: string, callerUserId: string): Promise<Milestone> {
+    const milestone = await this.findOne(milestoneId);
+    if (milestone.sponsorId && milestone.sponsorId !== callerUserId) {
+      throw new ForbiddenException('Only the milestone sponsor can close it');
+    }
+    assertTransition(milestone.status, MilestoneStatus.CLOSED);
+
+    // Refund the remaining escrow balance — the sponsor keeps what wasn't
+    // distributed. releasePartial paid the contributors; refund returns the
+    // rest (#587).
+    if (milestone.escrowId) {
+      await this.escrowService.refund(milestone.escrowId);
+    }
+    milestone.status = MilestoneStatus.CLOSED;
+    return this.milestoneRepo.save(milestone);
+  }
+
   /**
    * Distributes this milestone's budget proportionally as its issues resolve.
    * TODO: support per-issue weighted budgets; currently splits the remaining

src/sponsors/sponsors.controller.ts

@@ -26,25 +26,6 @@ function validatePage(limit?: number, offset?: number) {
     throw new BadRequestException('offset must be >= 0');
   }
   return { limit, offset };
-/**
- * Parse optional limit/offset query params, rejecting NaN, negatives,
- * non-integers, and limits above MAX_PAGE_LIMIT with a 400 (#280).
- */
-function parsePagination(limit?: string, offset?: string) {
-  const parse = (name: string, raw: string | undefined, min: number, max: number) => {
-    if (raw === undefined || raw === '') return undefined;
-    const n = Number(raw);
-    if (!Number.isInteger(n) || n < min || n > max) {
-      throw new BadRequestException(
-        `${name} must be an integer between ${min} and ${max}`,
-      );
-    }
-    return n;
-  };
-  return {
-    limit: parse('limit', limit, 1, MAX_PAGE_LIMIT),
-    offset: parse('offset', offset, 0, Number.MAX_SAFE_INTEGER),
-  };
 }
 
 @ApiTags('sponsors')
@@ -73,7 +54,6 @@ export class SponsorsController {
   ) {
     this.assertOwnsSponsor(user, id);
     return this.sponsorsService.dashboard(id, validatePage(limit, offset));
-    return this.sponsorsService.dashboard(id, parsePagination(limit, offset));
   }
 
   @ApiBearerAuth()
@@ -91,6 +71,5 @@ export class SponsorsController {
       id,
       validatePage(limit, offset),
     );
-    return this.sponsorsService.milestoneProgress(id, parsePagination(limit, offset));
   }
 }