refactor(server): extract TunnelServiceBase to reduce code duplication

[bianbiandashen]

Summary

Extract common tunnel service functionality into an abstract base class to eliminate code duplication between NgrokService and CloudflareService.

Problem

NgrokService and CloudflareService contained ~80% identical code:

Functionality	Lines (approx)
Binary path discovery	25
Process spawn & setup	40
Output handling	20
Graceful shutdown	25
State management	15
Total duplicated	~125 lines × 2

This duplication meant:

• Bug fixes needed in two places
• Inconsistent behavior crept in over time
• Adding new tunnel providers required copying ~200 lines

Solution

New Abstract Base Class

// tunnel-service-base.ts
export abstract class TunnelServiceBase {
  // Shared implementation
  protected async checkBinary(): Promise<string | null> { ... }
  async start(): Promise<TunnelInfo> { ... }
  async stop(): Promise<void> { ... }
  
  // Subclass responsibilities
  protected abstract getBinaryPaths(): string[];
  protected abstract getBinaryVersionArgs(): string[];
  protected abstract buildStartArgs(): string[];
  protected abstract parseOutput(output: string): string | null;
  protected abstract getServiceName(): string;
}

Refactored NgrokService

// ngrok-service.ts (now ~97 lines, was ~235)
export class NgrokService extends TunnelServiceBase {
  protected getServiceName() { return 'ngrok'; }
  protected getBinaryPaths() { return ['ngrok', '/usr/local/bin/ngrok', ...]; }
  protected getBinaryVersionArgs() { return ['version']; }
  protected buildStartArgs() { return ['http', port, '--log=stdout', ...]; }
  protected parseOutput(output: string) {
    // Parse JSON logs for tunnel URL
    const log = JSON.parse(line);
    return log.msg === 'started tunnel' ? log.url : null;
  }
}

Refactored CloudflareService

// cloudflare-service.ts (now ~64 lines, was ~232)
export class CloudflareService extends TunnelServiceBase {
  protected getServiceName() { return 'Cloudflare'; }
  protected getBinaryPaths() { return ['cloudflared', '/usr/local/bin/cloudflared', ...]; }
  protected getBinaryVersionArgs() { return ['--version']; }
  protected buildStartArgs() { return ['tunnel', '--url', `http://localhost:${port}`]; }
  protected parseOutput(output: string) {
    // Parse URL regex
    const match = output.match(/https:\/\/[a-z0-9-]+\.trycloudflare\.com/);
    return match ? match[0] : null;
  }
}

Code Metrics

File	Before	After	Change
ngrok-service.ts	235 lines	97 lines	-138 lines
cloudflare-service.ts	232 lines	64 lines	-168 lines
tunnel-service-base.ts	N/A	238 lines	+238 lines
Net change			-68 lines

While total lines decreased modestly, the key improvement is single source of truth for tunnel lifecycle logic.

Benefits

1. Maintainability: Fix bugs once in base class
2. Consistency: Same behavior for all tunnel providers
3. Extensibility: Adding new providers (localtunnel, bore, etc.) is trivial
4. Type Safety: TunnelInfo interface ensures consistent return types

Backward Compatibility

• NgrokTunnel type aliased to TunnelInfo
• CloudflareTunnel type aliased to TunnelInfo
• All existing public methods preserved with same signatures

Test Plan

• Verify ngrok tunnel starts correctly
• Verify cloudflared tunnel starts correctly
• Verify graceful shutdown works for both
• Verify binary not found errors work
• Verify startup timeout works

[nikolasdehor]

Found a regression in testability after the refactor.

I checked out this branch and ran:

cd web && pnpm exec vitest run src/server/services/ngrok-service.test.ts src/server/services/cloudflare-service.test.ts

Result: both tests time out.

Reason:

• Tests currently stub legacy private methods (checkNgrokBinary / checkCloudflaredBinary), but start() now calls checkBinary() from TunnelServiceBase.
• checkBinary() does an initial spawn(...version args...) probe, and the test mocks don’t emit close for that probe, so the promise never resolves in these tests.

Impact:

• Existing service tests no longer validate start-path behavior on this branch.

Suggested fix:

• Update tests to mock/spy checkBinary() directly, or
• Mock spawn with two process instances (one for version probe emitting close(0), one for tunnel start output), and
• Keep assertions on start args/output parsing as-is.

Once updated, these tests should pass and keep coverage parity with pre-refactor behavior.