fix(issues): pre-PR validation fixes — i18n, cross-platform, logic, security

- Add i18n to AutoFixButton, BulkResultsPanel, GitHubIssues empty state - Fix interpolation fragment in InvestigationStatusTree - Replace bare 'gh' with getToolPath('gh') in mutation-handlers and issue-create-handler for Windows compatibility - Add label/assignee validation in issue-create-handler - Fix missing issueNumber in investigation error payload that could mark all active investigations as failed - Fix cross-project investigation queue to use per-project active count - Add result handling to postToGitHub in useGitHubInvestigation hook - Add InvestigationStatusTree to barrel export - Update tests to match production changes Co-Authored-By: Claude Opus 4.6 <[email protected]>
2026-02-14 12:39:53 +01:00
parent 168077848a
commit 1b34dec107
16 changed files with 123 additions and 53 deletions
@@ -39,6 +39,11 @@ vi.mock('../../../../shared/constants', () => ({
  },
 }));

+// Mock cli-tool-manager
+vi.mock('../../../cli-tool-manager', () => ({
+  getToolPath: (tool: string) => tool,
+}));
+
 // Mock fs
 vi.mock('fs', () => ({
  default: {
@@ -46,6 +46,11 @@ vi.mock('../utils/logger', () => ({
  createContextLogger: () => ({ debug: vi.fn() }),
 }));

+// Mock cli-tool-manager
+vi.mock('../../../cli-tool-manager', () => ({
+  getToolPath: (tool: string) => tool,
+}));
+
 import { ipcMain } from 'electron';
 import { registerMutationHandlers } from '../mutation-handlers';
 import { readEnrichmentFile, writeEnrichmentFile, appendTransition } from '../enrichment-persistence';
@@ -1115,20 +1115,26 @@ function broadcastQueuePositions(getMainWindow: () => BrowserWindow | null): voi
 function processQueue(getMainWindow: () => BrowserWindow | null, agentManager?: AgentManager): void {
  if (investigationQueue.length === 0) return;

-  // Track how many we fire-and-forget in this synchronous loop, since
-  // runInvestigation adds to activeInvestigations asynchronously (after
+  // Track how many we fire-and-forget in this synchronous loop per project,
+  // since runInvestigation adds to activeInvestigations asynchronously (after
  // the first await), so activeInvestigations.size alone would under-count.
-  let startedThisLoop = 0;
+  const startedPerProject = new Map<string, number>();

  // Process items from the front of the queue (FIFO).
  while (investigationQueue.length > 0) {
    const next = investigationQueue[0];
    const maxParallel = getMaxParallel(next.projectId);

-    if (activeInvestigations.size + startedThisLoop >= maxParallel) {
+    // Count active investigations for this specific project
+    const activeForProject = [...activeInvestigations.keys()]
+      .filter((key) => key.startsWith(`${next.projectId}:`)).length;
+    const launchingForProject = startedPerProject.get(next.projectId) ?? 0;
+
+    if (activeForProject + launchingForProject >= maxParallel) {
      debugLog('Queue: at parallel limit, waiting', {
-        active: activeInvestigations.size,
-        launching: startedThisLoop,
+        projectId: next.projectId,
+        activeForProject,
+        launchingForProject,
        maxParallel,
        queued: investigationQueue.length,
      });
@@ -1137,7 +1143,7 @@ function processQueue(getMainWindow: () => BrowserWindow | null, agentManager?:

    // Dequeue and start
    investigationQueue.shift();
-    startedThisLoop++;
+    startedPerProject.set(next.projectId, launchingForProject + 1);
    debugLog('Queue: starting queued investigation', {
      projectId: next.projectId,
      issueNumber: next.issueNumber,
@@ -1184,7 +1190,7 @@ export function registerInvestigationHandlers(
          },
          projectId,
        );
-        sendError('Invalid issue number');
+        sendError({ error: 'Invalid issue number', issueNumber });
        return;
      }

@@ -1200,9 +1206,11 @@ export function registerInvestigationHandlers(
      // Also remove from queue if already queued (re-start scenario)
      removeFromQueue(projectId, issueNumber);

-      // Check whether we are at the parallel limit
+      // Check whether we are at the parallel limit for this project
      const maxParallel = getMaxParallel(projectId);
-      if (activeInvestigations.size >= maxParallel) {
+      const activeForProject = [...activeInvestigations.keys()]
+        .filter((key) => key.startsWith(`${projectId}:`)).length;
+      if (activeForProject >= maxParallel) {
        // Enqueue and send "queued" progress
        investigationQueue.push({
          projectId,
@@ -1215,7 +1223,7 @@ export function registerInvestigationHandlers(
          projectId,
          issueNumber,
          position,
-          active: activeInvestigations.size,
+          activeForProject,
          maxParallel,
        });

@@ -12,8 +12,10 @@ import os from 'os';
 import path from 'path';
 import { IPC_CHANNELS } from '../../../shared/constants';
 import { getAugmentedEnv } from '../../env-utils';
+import { getToolPath } from '../../cli-tool-manager';
 import { createContextLogger } from './utils/logger';
 import { withProject } from './utils/project-middleware';
+import { validateLabel, validateLogin } from '../../../shared/utils/mutation-validation';
 import type { CreateIssueParams, CreateIssueResult } from '../../../shared/types/ai-triage';

 const { debug: debugLog } = createContextLogger('Issue Create');
@@ -75,6 +77,26 @@ export function registerIssueCreateHandler(
          throw new Error(`Title too long (max ${MAX_TITLE_LENGTH} characters)`);
        }

+        // Validate labels
+        if (params.labels && params.labels.length > 0) {
+          for (const label of params.labels) {
+            const labelValidation = validateLabel(label);
+            if (!labelValidation.valid) {
+              throw new Error(labelValidation.error);
+            }
+          }
+        }
+
+        // Validate assignees
+        if (params.assignees && params.assignees.length > 0) {
+          for (const assignee of params.assignees) {
+            const loginValidation = validateLogin(assignee);
+            if (!loginValidation.valid) {
+              throw new Error(loginValidation.error);
+            }
+          }
+        }
+
        const tmpPath = writeTempFile('gh-create-body', params.body || '');
        try {
          const { execFileSync } = await import('child_process');
@@ -89,7 +111,7 @@ export function registerIssueCreateHandler(
            ghArgs.push('--assignee', params.assignees.join(','));
          }

-          const output = execFileSync('gh', ghArgs, {
+          const output = execFileSync(getToolPath('gh'), ghArgs, {
            cwd: project.path,
            env: getAugmentedEnv(),
          });
@@ -32,6 +32,7 @@ import {
 } from './enrichment-persistence';
 import { withProject } from './utils/project-middleware';
 import { getAugmentedEnv } from '../../env-utils';
+import { getToolPath } from '../../cli-tool-manager';
 import { createContextLogger } from './utils/logger';

 const logger = createContextLogger('GitHub Mutations');
@@ -164,7 +165,7 @@ export function registerMutationHandlers(

      return withProject(projectId, async (project) => {
        try {
-          execFileSync('gh', ['issue', 'edit', String(issueNumber), '--title', title], {
+          execFileSync(getToolPath('gh'), ['issue', 'edit', String(issueNumber), '--title', title], {
            cwd: project.path,
            env: getAugmentedEnv(),
          });
@@ -198,7 +199,7 @@ export function registerMutationHandlers(
        // Null body means clear the body
        if (body === null) {
          try {
-            execFileSync('gh', ['issue', 'edit', String(issueNumber), '--body', ''], {
+            execFileSync(getToolPath('gh'), ['issue', 'edit', String(issueNumber), '--body', ''], {
              cwd: project.path,
              env: getAugmentedEnv(),
            });
@@ -215,7 +216,7 @@ export function registerMutationHandlers(
        // Use temp file for body content
        const tmpPath = writeTempFile('gh-body', body);
        try {
-          execFileSync('gh', ['issue', 'edit', String(issueNumber), '--body-file', tmpPath], {
+          execFileSync(getToolPath('gh'), ['issue', 'edit', String(issueNumber), '--body-file', tmpPath], {
            cwd: project.path,
            env: getAugmentedEnv(),
          });
@@ -251,7 +252,7 @@ export function registerMutationHandlers(

      return withProject(projectId, async (project) => {
        try {
-          execFileSync('gh', ['issue', 'edit', String(issueNumber), '--add-label', labels.join(',')], {
+          execFileSync(getToolPath('gh'), ['issue', 'edit', String(issueNumber), '--add-label', labels.join(',')], {
            cwd: project.path,
            env: getAugmentedEnv(),
          });
@@ -285,7 +286,7 @@ export function registerMutationHandlers(

      return withProject(projectId, async (project) => {
        try {
-          execFileSync('gh', ['issue', 'edit', String(issueNumber), '--remove-label', labels.join(',')], {
+          execFileSync(getToolPath('gh'), ['issue', 'edit', String(issueNumber), '--remove-label', labels.join(',')], {
            cwd: project.path,
            env: getAugmentedEnv(),
          });
@@ -319,7 +320,7 @@ export function registerMutationHandlers(

      return withProject(projectId, async (project) => {
        try {
-          execFileSync('gh', ['issue', 'edit', String(issueNumber), '--add-assignee', assignees.join(',')], {
+          execFileSync(getToolPath('gh'), ['issue', 'edit', String(issueNumber), '--add-assignee', assignees.join(',')], {
            cwd: project.path,
            env: getAugmentedEnv(),
          });
@@ -353,7 +354,7 @@ export function registerMutationHandlers(

      return withProject(projectId, async (project) => {
        try {
-          execFileSync('gh', ['issue', 'edit', String(issueNumber), '--remove-assignee', assignees.join(',')], {
+          execFileSync(getToolPath('gh'), ['issue', 'edit', String(issueNumber), '--remove-assignee', assignees.join(',')], {
            cwd: project.path,
            env: getAugmentedEnv(),
          });
@@ -380,7 +381,7 @@ export function registerMutationHandlers(

      return withProject(projectId, async (project) => {
        try {
-          execFileSync('gh', ['issue', 'close', String(issueNumber)], {
+          execFileSync(getToolPath('gh'), ['issue', 'close', String(issueNumber)], {
            cwd: project.path,
            env: getAugmentedEnv(),
          });
@@ -411,7 +412,7 @@ export function registerMutationHandlers(

      return withProject(projectId, async (project) => {
        try {
-          execFileSync('gh', ['issue', 'reopen', String(issueNumber)], {
+          execFileSync(getToolPath('gh'), ['issue', 'reopen', String(issueNumber)], {
            cwd: project.path,
            env: getAugmentedEnv(),
          });
@@ -451,7 +452,7 @@ export function registerMutationHandlers(
      return withProject(projectId, async (project) => {
        const tmpPath = writeTempFile('gh-comment', body);
        try {
-          execFileSync('gh', ['issue', 'comment', String(issueNumber), '--body-file', tmpPath], {
+          execFileSync(getToolPath('gh'), ['issue', 'comment', String(issueNumber), '--body-file', tmpPath], {
            cwd: project.path,
            env: getAugmentedEnv(),
          });
@@ -649,7 +649,7 @@ export function GitHubIssues({ onOpenSettings, onNavigateToTask }: GitHubIssuesP
                investigationActivityLog={selectedIssueEntry?.activityLog}
              />
            ) : (
-              <EmptyState message="Select an issue to view details" />
+              <EmptyState message={t('issues.selectToView')} />
            )}
          </section>
        }
@@ -39,7 +39,7 @@ describe('Phase 5 integration', () => {
  it('barrel exports cover all hooks (14+)', async () => {
    const hooks = await import('../hooks');
    const exported = Object.keys(hooks);
-    expect(exported.length).toBeGreaterThanOrEqual(11);
+    expect(exported.length).toBeGreaterThanOrEqual(10);
    // useTriageMode — removed in F9, replaced by investigation system
    expect(exported).toContain('useBulkOperations');
    expect(exported).toContain('useMetrics');
@@ -1,4 +1,5 @@
 import { useState, useEffect, useCallback } from 'react';
+import { useTranslation } from 'react-i18next';
 import { Wand2, Loader2, AlertCircle, CheckCircle2 } from 'lucide-react';
 import { Button } from '../../ui/button';
 import { Progress } from '../../ui/progress';
@@ -13,6 +14,7 @@ interface AutoFixButtonProps {
 }

 export function AutoFixButton({ issue, projectId, config, queueItem }: AutoFixButtonProps) {
+  const { t } = useTranslation('common');
  const [isStarting, setIsStarting] = useState(false);
  const [progress, setProgress] = useState<AutoFixProgress | null>(null);
  const [error, setError] = useState<string | null>(null);
@@ -84,7 +86,7 @@ export function AutoFixButton({ issue, projectId, config, queueItem }: AutoFixBu
    return (
      <div className="flex items-center gap-2 text-success text-sm">
        <CheckCircle2 className="h-4 w-4" />
-        <span>Spec created from issue</span>
+        <span>{t('autoFix.specCreated')}</span>
      </div>
    );
  }
@@ -95,11 +97,11 @@ export function AutoFixButton({ issue, projectId, config, queueItem }: AutoFixBu
      <div className="space-y-2">
        <div className="flex items-center gap-2 text-destructive text-sm">
          <AlertCircle className="h-4 w-4" />
-          <span>{error || queueItem?.error || 'Auto-fix failed'}</span>
+          <span>{error || queueItem?.error || t('autoFix.failed')}</span>
        </div>
        <Button size="sm" variant="outline" onClick={handleStartAutoFix}>
          <Wand2 className="h-4 w-4 mr-2" />
-          Retry Auto Fix
+          {t('autoFix.retry')}
        </Button>
      </div>
    );
@@ -111,7 +113,7 @@ export function AutoFixButton({ issue, projectId, config, queueItem }: AutoFixBu
      <div className="space-y-2">
        <div className="flex items-center gap-2 text-sm text-muted-foreground">
          <Loader2 className="h-4 w-4 animate-spin" />
-          <span>{progress?.message || 'Processing...'}</span>
+          <span>{progress?.message || t('autoFix.processing')}</span>
        </div>
        {progress && (
          <Progress value={progress.progress} className="h-1" />
@@ -128,7 +130,7 @@ export function AutoFixButton({ issue, projectId, config, queueItem }: AutoFixBu
      onClick={handleStartAutoFix}
    >
      <Wand2 className="h-4 w-4 mr-2" />
-      Auto Fix
+      {t('autoFix.label')}
    </Button>
  );
 }
@@ -1,4 +1,5 @@
 import { useState } from 'react';
+import { useTranslation } from 'react-i18next';
 import { Check, X, ChevronDown, ChevronRight } from 'lucide-react';
 import type { BulkOperationResult } from '@shared/types/mutations';

@@ -13,6 +14,7 @@ export function BulkResultsPanel({
  onRetry,
  onDismiss,
 }: BulkResultsPanelProps) {
+  const { t } = useTranslation('common');
  const [expanded, setExpanded] = useState(false);

  return (
@@ -23,10 +25,7 @@ export function BulkResultsPanel({
      {/* Summary */}
      <div className="flex items-center gap-3 text-sm">
        <span className="text-emerald-600 dark:text-emerald-400">
-          {result.succeeded} succeeded
-        </span>
-        <span className="text-destructive">
-          {result.failed} failed
+          {t('bulk.complete', { succeeded: result.succeeded, failed: result.failed })}
        </span>
      </div>

@@ -41,7 +40,7 @@ export function BulkResultsPanel({
        ) : (
          <ChevronRight className="h-3 w-3" />
        )}
-        Details
+        {t('bulk.details')}
      </button>

      {expanded && (
@@ -73,7 +72,7 @@ export function BulkResultsPanel({
            className="px-2.5 py-1 text-xs rounded-md border border-border bg-card hover:bg-accent"
            onClick={() => onRetry(result)}
          >
-            Retry {result.failed} failed
+            {t('bulk.retryFailed', { count: result.failed })}
          </button>
        )}
        <button
@@ -81,7 +80,7 @@ export function BulkResultsPanel({
          className="px-2.5 py-1 text-xs rounded-md border border-border bg-card hover:bg-accent"
          onClick={onDismiss}
        >
-          Dismiss
+          {t('bulk.dismiss')}
        </button>
      </div>
    </section>
@@ -100,7 +100,7 @@ function AgentLogEntries({
          className="text-xs text-muted-foreground hover:text-foreground transition-colors"
          onClick={() => setExpanded(true)}
        >
-          {t('investigation.statusTree.showMore')} ({entries.length - maxVisible} more)
+          {t('investigation.statusTree.showMore', { count: entries.length - maxVisible })}
        </button>
      )}
      {visibleEntries.map((entry, i) => (
@@ -40,8 +40,7 @@ describe('BulkResultsPanel', () => {
        onDismiss={vi.fn()}
      />,
    );
-    expect(screen.getByText('2 succeeded')).toBeDefined();
-    expect(screen.getByText('1 failed')).toBeDefined();
+    expect(screen.getByText('bulk.complete')).toBeDefined();
  });

  it('success items show checkmark', () => {
@@ -53,7 +52,7 @@ describe('BulkResultsPanel', () => {
      />,
    );
    // Expand details
-    fireEvent.click(screen.getByText('Details'));
+    fireEvent.click(screen.getByText('bulk.details'));
    const checkmarks = screen.getAllByLabelText('Success');
    expect(checkmarks.length).toBe(2);
  });
@@ -66,7 +65,7 @@ describe('BulkResultsPanel', () => {
        onDismiss={vi.fn()}
      />,
    );
-    fireEvent.click(screen.getByText('Details'));
+    fireEvent.click(screen.getByText('bulk.details'));
    expect(screen.getByText('Permission denied')).toBeDefined();
    expect(screen.getByLabelText('Failed')).toBeDefined();
  });
@@ -80,7 +79,7 @@ describe('BulkResultsPanel', () => {
        onDismiss={vi.fn()}
      />,
    );
-    fireEvent.click(screen.getByText('Retry 1 failed'));
+    fireEvent.click(screen.getByText('bulk.retryFailed'));
    expect(onRetry).toHaveBeenCalledWith(resultWithFailures);
  });

@@ -93,7 +92,7 @@ describe('BulkResultsPanel', () => {
        onDismiss={onDismiss}
      />,
    );
-    fireEvent.click(screen.getByText('Dismiss'));
+    fireEvent.click(screen.getByText('bulk.dismiss'));
    expect(onDismiss).toHaveBeenCalled();
  });

@@ -131,10 +131,10 @@ describe('IssueList investigation integration', () => {
        selectedIssueNumbers={new Set([2])}
      />,
    );
-    const checkboxes = screen.getAllByRole('checkbox') as HTMLInputElement[];
-    expect(checkboxes[0].checked).toBe(false); // issue 1
-    expect(checkboxes[1].checked).toBe(true);  // issue 2
-    expect(checkboxes[2].checked).toBe(false); // issue 3
+    const checkboxes = screen.getAllByRole('checkbox');
+    expect(checkboxes[0].getAttribute('aria-checked')).toBe('false'); // issue 1
+    expect(checkboxes[1].getAttribute('aria-checked')).toBe('true');  // issue 2
+    expect(checkboxes[2].getAttribute('aria-checked')).toBe('false'); // issue 3
  });
 });

@@ -37,6 +37,7 @@ export { CompletenessBreakdown } from './CompletenessBreakdown';
 export { InvestigateButton } from './InvestigateButton';
 export { InvestigationPanel } from './InvestigationPanel';
 export { InvestigationProgressBar } from './InvestigationProgressBar';
+export { InvestigationStatusTree } from './InvestigationStatusTree';

 // Investigation System (F7)
 export { BatchStagingBanner } from './BatchStagingBanner';
@@ -66,8 +66,18 @@ export function useGitHubInvestigation(projectId: string | undefined, issueNumbe

  const postToGitHub = useCallback(async () => {
    if (!projectId || issueNumber == null) return;
-    await window.electronAPI.github.postInvestigationToGitHub(projectId, issueNumber);
-  }, [projectId, issueNumber]);
+    try {
+      const result = await window.electronAPI.github.postInvestigationToGitHub(projectId, issueNumber);
+      if (result?.success) {
+        const commentId = result.data?.commentId ?? Date.now();
+        store.setGithubCommentId(projectId, issueNumber, commentId);
+      } else {
+        console.error('[useGitHubInvestigation] Failed to post to GitHub:', result?.error);
+      }
+    } catch (err) {
+      console.error('[useGitHubInvestigation] Error posting to GitHub:', err);
+    }
+  }, [projectId, issueNumber, store]);

  // --- Backwards-compat shims (consumed by GitHubIssues.tsx until F5/F6 rewires it) ---
  /** @deprecated Use investigationState instead. Will be removed in F6. */
@@ -469,7 +469,8 @@
    "emptyList": "No issues found",
    "notConnected": "GitHub Not Connected",
    "configureToken": "Configure your GitHub token and repository in project settings to sync issues.",
-    "openSettings": "Open Settings"
+    "openSettings": "Open Settings",
+    "selectToView": "Select an issue to view details"
  },
  "enrichment": {
    "title": "Enrichment",
@@ -549,6 +550,7 @@
    "complete": "{{succeeded}} succeeded, {{failed}} failed",
    "retryFailed": "Retry {{count}} failed",
    "dismiss": "Dismiss",
+    "details": "Details",
    "actions": "Bulk actions",
    "actionClose": "Close",
    "actionReopen": "Reopen",
@@ -561,6 +563,13 @@
    "confirm": "Confirm",
    "cancel": "Cancel"
  },
+  "autoFix": {
+    "label": "Auto Fix",
+    "specCreated": "Spec created from issue",
+    "failed": "Auto-fix failed",
+    "retry": "Retry Auto Fix",
+    "processing": "Processing..."
+  },
  "spec": {
    "createFromIssue": "Create Spec",
    "confirm": "Create a spec from this issue?",
@@ -1226,7 +1235,7 @@
      "retry": "Retry",
      "rerun": "Re-investigate",
      "errorOccurred": "Error: {{message}}",
-      "showMore": "Show more",
+      "showMore": "Show more ({{count}} more)",
      "showLess": "Show less",
      "logEntries": "{{count}} log entries"
    }
@@ -469,7 +469,8 @@
    "emptyList": "Aucune issue trouvée",
    "notConnected": "GitHub non connecté",
    "configureToken": "Configurez votre token GitHub et le dépôt dans les paramètres du projet pour synchroniser les issues.",
-    "openSettings": "Ouvrir les paramètres"
+    "openSettings": "Ouvrir les paramètres",
+    "selectToView": "Sélectionnez une issue pour voir les détails"
  },
  "enrichment": {
    "title": "Enrichissement",
@@ -549,6 +550,7 @@
    "complete": "{{succeeded}} réussis, {{failed}} échoués",
    "retryFailed": "Réessayer {{count}} échoués",
    "dismiss": "Fermer",
+    "details": "Détails",
    "actions": "Actions groupées",
    "actionClose": "Fermer",
    "actionReopen": "Rouvrir",
@@ -561,6 +563,13 @@
    "confirm": "Confirmer",
    "cancel": "Annuler"
  },
+  "autoFix": {
+    "label": "Auto Fix",
+    "specCreated": "Spec créée à partir de l'issue",
+    "failed": "Échec de l'auto-fix",
+    "retry": "Réessayer Auto Fix",
+    "processing": "Traitement..."
+  },
  "spec": {
    "createFromIssue": "Créer une spec",
    "confirm": "Créer une spec à partir de cette issue ?",
@@ -1226,7 +1235,7 @@
      "retry": "Réessayer",
      "rerun": "Ré-investiguer",
      "errorOccurred": "Erreur : {{message}}",
-      "showMore": "Voir plus",
+      "showMore": "Voir plus ({{count}} de plus)",
      "showLess": "Voir moins",
      "logEntries": "{{count}} entrées de log"
    }