Merge pull request #42 from DGouron/feat/followup-importants

DGouron · web-flow · commit 7fe00b1eba4a · 2026-02-12T22:43:08.000+01:00
feat(followup): re-verify Important issues on pending-approval MRs
diff --git a/src/cli/parseCliArgs.ts b/src/cli/parseCliArgs.ts
@@ -39,13 +39,19 @@ interface VersionArgs {
   command: 'version';
 }
 
+interface FollowupImportantsArgs {
+  command: 'followup-importants';
+  project: string | undefined;
+  yes: boolean;
+}
+
 interface HelpArgs {
   command: 'help';
 }
 
-export type CliArgs = StartArgs | StopArgs | StatusArgs | LogsArgs | InitArgs | ValidateArgs | VersionArgs | HelpArgs;
+export type CliArgs = StartArgs | StopArgs | StatusArgs | LogsArgs | InitArgs | ValidateArgs | FollowupImportantsArgs | VersionArgs | HelpArgs;
 
-const KNOWN_COMMANDS = ['start', 'stop', 'status', 'logs', 'init', 'validate'] as const;
+const KNOWN_COMMANDS = ['start', 'stop', 'status', 'logs', 'init', 'validate', 'followup-importants'] as const;
 type KnownCommand = (typeof KNOWN_COMMANDS)[number];
 
 function hasFlag(args: string[], long: string, short?: string): boolean {
@@ -94,6 +100,14 @@ function parseStatusArgs(args: string[]): StatusArgs {
   };
 }
 
+function parseFollowupImportantsArgs(args: string[]): FollowupImportantsArgs {
+  return {
+    command: 'followup-importants',
+    project: getFlagValue(args, '--project', '-p'),
+    yes: hasFlag(args, '--yes', '-y'),
+  };
+}
+
 function parseLogsArgs(args: string[]): LogsArgs {
   const linesValue = getFlagValue(args, '--lines', '-n');
   return {
@@ -154,5 +168,7 @@ export function parseCliArgs(args: string[]): CliArgs {
       return parseInitArgs(args);
     case 'validate':
       return parseValidateArgs(args);
+    case 'followup-importants':
+      return parseFollowupImportantsArgs(args);
   }
 }
diff --git a/src/interface-adapters/controllers/http/mrTrackingAdvanced.routes.ts b/src/interface-adapters/controllers/http/mrTrackingAdvanced.routes.ts
@@ -285,6 +285,71 @@ export const mrTrackingAdvancedRoutes: FastifyPluginAsync<MrTrackingAdvancedRout
     return { success: true, mr: result };
   });
 
+  fastify.post('/api/mr-tracking/followup-importants', async (request, reply) => {
+    const body = request.body as { projectPath?: string };
+
+    let repos: RepositoryConfig[];
+    if (body.projectPath) {
+      const validation = validateProjectPath(body.projectPath);
+      if (!validation.valid) {
+        reply.code(400);
+        return { success: false, error: validation.error };
+      }
+      const repo = getRepositories().find(r => r.localPath === validation.path && r.enabled);
+      if (!repo) {
+        reply.code(404);
+        return { success: false, error: 'Repository not configured' };
+      }
+      repos = [repo];
+    } else {
+      repos = getRepositories().filter(r => r.enabled);
+    }
+
+    const candidates: Array<{ mr: import('../../../entities/tracking/trackedMr.js').TrackedMr; projectPath: string }> = [];
+    for (const repo of repos) {
+      const mrs = reviewRequestTrackingGateway.getByState(repo.localPath, 'pending-approval');
+      for (const mr of mrs) {
+        if (mr.totalWarnings > 0) {
+          candidates.push({ mr, projectPath: repo.localPath });
+        }
+      }
+    }
+
+    if (candidates.length === 0) {
+      return { success: true, triggered: 0, candidates: [], failed: [] };
+    }
+
+    const failed: Array<{ mrId: string; error: string }> = [];
+    let triggered = 0;
+
+    for (const { mr, projectPath } of candidates) {
+      try {
+        const internalResponse = await fastify.inject({
+          method: 'POST',
+          url: '/api/mr-tracking/followup',
+          payload: { mrId: mr.id, projectPath },
+        });
+        const data = JSON.parse(internalResponse.body);
+        if (data.success) {
+          triggered++;
+        } else {
+          failed.push({ mrId: mr.id, error: data.error });
+        }
+      } catch (error) {
+        failed.push({ mrId: mr.id, error: error instanceof Error ? error.message : String(error) });
+      }
+    }
+
+    logInfo('Followup importants batch triggered', { triggered, failed: failed.length, total: candidates.length });
+
+    return {
+      success: true,
+      triggered,
+      candidates: candidates.map(c => ({ mrId: c.mr.id, mrNumber: c.mr.mrNumber, title: c.mr.title })),
+      failed,
+    };
+  });
+
   fastify.post('/api/mr-tracking/sync', async (request, reply) => {
     const body = request.body as { projectPath?: string; mrId?: string };
     const { projectPath, mrId } = body;
diff --git a/src/interface-adapters/views/dashboard/index.html b/src/interface-adapters/views/dashboard/index.html
@@ -822,11 +822,16 @@ <h1>Reviewflow</h1>
       const mrPrefix = mr.platform === 'github' ? '#' : '!';
       const threadInfo = type === 'pending-fix'
         ? `<span class="mr-threads has-open"><i data-lucide="message-circle"></i> ${mr.openThreads} ouvert${mr.openThreads > 1 ? 's' : ''}</span>`
-        : `<span class="mr-threads all-resolved"><i data-lucide="check-circle"></i> Résolus</span>`;
+        : (type === 'pending-approval' && mr.totalWarnings > 0)
+          ? `<span class="mr-threads has-warnings"><i data-lucide="alert-triangle"></i> ${mr.totalWarnings} important${mr.totalWarnings > 1 ? 's' : ''}</span>`
+          : `<span class="mr-threads all-resolved"><i data-lucide="check-circle"></i> Résolus</span>`;
 
       const openBtn = `<a href="${mr.url}" target="_blank" class="btn-action open"><i data-lucide="external-link"></i> Ouvrir</a>`;
       const autoFollowupChecked = mr.autoFollowup !== false ? 'checked' : '';
-      const actions = type === 'pending-fix'
+      const showFollowupActions = type === 'pending-fix' ||
+        (type === 'pending-approval' && mr.totalWarnings > 0);
+
+      const actions = showFollowupActions
         ? `<label class="auto-followup-toggle" onclick="event.stopPropagation()" title="Active/désactive le followup automatique après un push">
              <input type="checkbox" ${autoFollowupChecked} onchange="toggleAutoFollowup('${mr.id}', this.checked)">
              Auto follow-up
@@ -855,7 +860,7 @@ <h1>Reviewflow</h1>
       return `
         <div class="mr-item-accordion" data-mr-id="${mr.id}">
           <div class="mr-item-header" onclick="toggleMrAccordion('${accordionId}')">
-            <div class="review-status ${type === 'pending-fix' ? 'running' : 'completed'}"></div>
+            <div class="review-status ${type === 'pending-fix' ? 'running' : (type === 'pending-approval' && mr.totalWarnings > 0) ? 'warnings' : 'completed'}"></div>
             <div class="mr-info">
               <div class="mr-title">
                 <a href="${mr.url}" target="_blank" onclick="event.stopPropagation()">${mrPrefix}${mr.mrNumber}</a> - ${mr.title}
diff --git a/src/interface-adapters/views/dashboard/styles.css b/src/interface-adapters/views/dashboard/styles.css
@@ -276,6 +276,7 @@ h1 { font-size: 1.75rem; font-weight: 600; }
 .review-status.queued { background: #3b82f6; }
 .review-status.completed { background: #22c55e; }
 .review-status.failed { background: #ef4444; }
+.review-status.warnings { background: #f59e0b; }
 
 .review-info { flex: 1; min-width: 0; }
 .review-title { font-weight: 500; white-space: nowrap; overflow: hidden; text-overflow: ellipsis; }
@@ -762,6 +763,7 @@ h1 { font-size: 1.75rem; font-weight: 600; }
 }
 .mr-threads.has-open { color: #ef4444; }
 .mr-threads.all-resolved { color: #22c55e; }
+.mr-threads.has-warnings { color: #f59e0b; }
 .mr-actions {
   display: flex;
   gap: 0.5rem;
diff --git a/src/main/cli.ts b/src/main/cli.ts
@@ -12,6 +12,7 @@ import { StartDaemonUseCase, type StartDaemonDependencies } from '../usecases/cl
 import { StopDaemonUseCase, type StopDaemonDependencies } from '../usecases/cli/stopDaemon.usecase.js';
 import { QueryStatusUseCase, type QueryStatusDependencies } from '../usecases/cli/queryStatus.usecase.js';
 import { ReadLogsUseCase, type ReadLogsDependencies } from '../usecases/cli/readLogs.usecase.js';
+import { FollowupImportantsUseCase } from '../usecases/cli/followupImportants.usecase.js';
 import { readPidFile, writePidFile, removePidFile } from '../shared/services/pidFileManager.js';
 import { isProcessRunning } from '../shared/services/processChecker.js';
 import { PID_FILE_PATH, LOG_FILE_PATH } from '../shared/services/daemonPaths.js';
@@ -51,6 +52,7 @@ Commands:
   status                   Show server status
   logs                     Show daemon logs
   validate                 Validate configuration
+  followup-importants      Trigger followups for pending-approval MRs with Important issues
 
 Init options:
   -y, --yes                Accept all defaults (non-interactive)
@@ -77,6 +79,10 @@ Logs options:
 Validate options:
   --fix                    Auto-fix correctable issues
 
+Followup-importants options:
+  -p, --project <path>     Scan specific project only
+  -y, --yes                Skip confirmation prompt
+
 General options:
   -v, --version            Show version
   -h, --help               Show this help
@@ -246,6 +252,23 @@ export function executeLogs(follow: boolean, lines: number, deps: LogsDeps): voi
   }
 }
 
+async function executeFollowupImportants(project: string | undefined): Promise<void> {
+  const pidData = readPidFile(PID_FILE_PATH);
+  if (!pidData || !isProcessRunning(pidData.pid)) {
+    console.error(red('Server is not running. Start with: reviewflow start'));
+    process.exit(1);
+  }
+
+  const usecase = new FollowupImportantsUseCase({
+    serverPort: pidData.port,
+    log: console.log,
+    error: console.error,
+    fetch: globalThis.fetch,
+  });
+
+  await usecase.execute({ project });
+}
+
 const DEFAULT_SCAN_PATHS = [
   join(homedir(), 'Documents'),
   join(homedir(), 'Projects'),
@@ -571,5 +594,9 @@ if (isDirectlyExecuted) {
     case 'validate':
       executeValidate(args.fix);
       break;
+
+    case 'followup-importants':
+      executeFollowupImportants(args.project);
+      break;
   }
 }
diff --git a/src/tests/units/cli/parseCliArgs.test.ts b/src/tests/units/cli/parseCliArgs.test.ts
@@ -247,6 +247,60 @@ describe('parseCliArgs', () => {
     });
   });
 
+  describe('followup-importants command', () => {
+    it('should detect followup-importants command', () => {
+      const result = parseCliArgs(['followup-importants']);
+
+      expect(result.command).toBe('followup-importants');
+    });
+
+    it('should detect --project flag with value', () => {
+      const result = parseCliArgs(['followup-importants', '--project', '/path/to/repo']);
+
+      expect(result).toEqual(expect.objectContaining({
+        command: 'followup-importants',
+        project: '/path/to/repo',
+      }));
+    });
+
+    it('should detect -p short flag for project', () => {
+      const result = parseCliArgs(['followup-importants', '-p', '/path/to/repo']);
+
+      expect(result).toEqual(expect.objectContaining({
+        command: 'followup-importants',
+        project: '/path/to/repo',
+      }));
+    });
+
+    it('should detect --yes flag', () => {
+      const result = parseCliArgs(['followup-importants', '--yes']);
+
+      expect(result).toEqual(expect.objectContaining({
+        command: 'followup-importants',
+        yes: true,
+      }));
+    });
+
+    it('should detect -y short flag for yes', () => {
+      const result = parseCliArgs(['followup-importants', '-y']);
+
+      expect(result).toEqual(expect.objectContaining({
+        command: 'followup-importants',
+        yes: true,
+      }));
+    });
+
+    it('should default project to undefined and yes to false', () => {
+      const result = parseCliArgs(['followup-importants']);
+
+      expect(result).toEqual(expect.objectContaining({
+        command: 'followup-importants',
+        project: undefined,
+        yes: false,
+      }));
+    });
+  });
+
   describe('version and help flags', () => {
     it('should detect --version flag', () => {
       const result = parseCliArgs(['--version']);
diff --git a/src/tests/units/usecases/cli/followupImportants.usecase.test.ts b/src/tests/units/usecases/cli/followupImportants.usecase.test.ts
@@ -0,0 +1,95 @@
+import { describe, it, expect, vi } from 'vitest';
+import { FollowupImportantsUseCase } from '../../../../usecases/cli/followupImportants.usecase.js';
+
+function createMockFetch(response: object) {
+  return vi.fn().mockResolvedValue({
+    ok: true,
+    json: () => Promise.resolve(response),
+  });
+}
+
+function createDeps(overrides: Partial<Parameters<typeof FollowupImportantsUseCase.prototype.execute>[1]> = {}) {
+  return {
+    serverPort: 3000,
+    log: vi.fn(),
+    error: vi.fn(),
+    fetch: createMockFetch({ success: true, triggered: 0, candidates: [], failed: [] }),
+    ...overrides,
+  };
+}
+
+describe('FollowupImportantsUseCase', () => {
+  it('should call the batch API endpoint', async () => {
+    const fetch = createMockFetch({ success: true, triggered: 0, candidates: [], failed: [] });
+    const deps = createDeps({ fetch });
+    const usecase = new FollowupImportantsUseCase(deps);
+
+    await usecase.execute({});
+
+    expect(fetch).toHaveBeenCalledWith(
+      'http://localhost:3000/api/mr-tracking/followup-importants',
+      expect.objectContaining({ method: 'POST' }),
+    );
+  });
+
+  it('should pass projectPath when provided', async () => {
+    const fetch = createMockFetch({ success: true, triggered: 0, candidates: [], failed: [] });
+    const deps = createDeps({ fetch });
+    const usecase = new FollowupImportantsUseCase(deps);
+
+    await usecase.execute({ project: '/path/to/repo' });
+
+    expect(fetch).toHaveBeenCalledWith(
+      expect.any(String),
+      expect.objectContaining({
+        body: JSON.stringify({ projectPath: '/path/to/repo' }),
+      }),
+    );
+  });
+
+  it('should log message when no candidates found', async () => {
+    const deps = createDeps();
+    const usecase = new FollowupImportantsUseCase(deps);
+
+    await usecase.execute({});
+
+    expect(deps.log).toHaveBeenCalledWith(expect.stringContaining('No pending-approval'));
+  });
+
+  it('should log triggered count and candidate details', async () => {
+    const fetch = createMockFetch({
+      success: true,
+      triggered: 2,
+      candidates: [
+        { mrId: 'gitlab-proj-10', mrNumber: 10, title: 'Fix auth' },
+        { mrId: 'gitlab-proj-11', mrNumber: 11, title: 'Add tests' },
+      ],
+      failed: [],
+    });
+    const deps = createDeps({ fetch });
+    const usecase = new FollowupImportantsUseCase(deps);
+
+    await usecase.execute({});
+
+    expect(deps.log).toHaveBeenCalledWith(expect.stringContaining('2'));
+    expect(deps.log).toHaveBeenCalledWith(expect.stringContaining('!10'));
+    expect(deps.log).toHaveBeenCalledWith(expect.stringContaining('!11'));
+  });
+
+  it('should log failures when some followups fail', async () => {
+    const fetch = createMockFetch({
+      success: true,
+      triggered: 1,
+      candidates: [
+        { mrId: 'gitlab-proj-10', mrNumber: 10, title: 'Fix auth' },
+      ],
+      failed: [{ mrId: 'gitlab-proj-11', error: 'timeout' }],
+    });
+    const deps = createDeps({ fetch });
+    const usecase = new FollowupImportantsUseCase(deps);
+
+    await usecase.execute({});
+
+    expect(deps.error).toHaveBeenCalledWith(expect.stringContaining('1 failed'));
+  });
+});
diff --git a/src/tests/units/usecases/tracking/checkFollowupNeeded.usecase.test.ts b/src/tests/units/usecases/tracking/checkFollowupNeeded.usecase.test.ts
diff --git a/src/usecases/cli/followupImportants.usecase.ts b/src/usecases/cli/followupImportants.usecase.ts
diff --git a/src/usecases/tracking/checkFollowupNeeded.usecase.ts b/src/usecases/tracking/checkFollowupNeeded.usecase.ts
