Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
113 changes: 109 additions & 4 deletions apps/ui/src/components/views/github-issues-view.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// @ts-nocheck
import { useState, useCallback, useMemo } from 'react';
import { useState, useCallback, useMemo, useEffect, useRef } from 'react';
import { createLogger } from '@automaker/utils/logger';
import { CircleDot, RefreshCw } from 'lucide-react';
import { getElectronAPI, GitHubIssue, IssueValidationResult } from '@/lib/electron';
Expand All @@ -11,7 +11,7 @@ import { cn, pathsEqual } from '@/lib/utils';
import { toast } from 'sonner';
import { useGithubIssues, useIssueValidation } from './github-issues-view/hooks';
import { IssueRow, IssueDetailPanel, IssuesListHeader } from './github-issues-view/components';
import { ValidationDialog } from './github-issues-view/dialogs';
import { ValidationDialog, ImportIssuesDialog } from './github-issues-view/dialogs';
import { formatDate, getFeaturePriority } from './github-issues-view/utils';
import { useModelOverride } from '@/components/shared';
import type { ValidateIssueOptions } from './github-issues-view/types';
Expand All @@ -23,11 +23,19 @@ export function GitHubIssuesView() {
const [validationResult, setValidationResult] = useState<IssueValidationResult | null>(null);
const [showValidationDialog, setShowValidationDialog] = useState(false);
const [showRevalidateConfirm, setShowRevalidateConfirm] = useState(false);
const [showImportDialog, setShowImportDialog] = useState(false);
const [pendingRevalidateOptions, setPendingRevalidateOptions] =
useState<ValidateIssueOptions | null>(null);

const { currentProject, defaultAIProfileId, aiProfiles, getCurrentWorktree, worktreesByProject } =
useAppStore();
const {
currentProject,
defaultAIProfileId,
aiProfiles,
getCurrentWorktree,
worktreesByProject,
githubAutoValidate,
setGithubAutoValidate,
} = useAppStore();

// Model override for validation
const validationModelOverride = useModelOverride({ phase: 'validationModel' });
Expand All @@ -45,6 +53,54 @@ export function GitHubIssuesView() {
onShowValidationDialogChange: setShowValidationDialog,
});

// Track which issues we've already triggered auto-validate for (to avoid duplicates)
const autoValidatedIssuesRef = useRef<Set<number>>(new Set());

// Auto-validate issues when enabled and issues are loaded
useEffect(() => {
if (!githubAutoValidate || loading || openIssues.length === 0) return;

// Find issues that need validation (not cached and not currently validating)
const issuesToValidate = openIssues.filter((issue) => {
// Skip if already auto-validated in this session
if (autoValidatedIssuesRef.current.has(issue.number)) return false;
// Skip if already has cached validation
if (cachedValidations.has(issue.number)) return false;
// Skip if currently validating
if (validatingIssues.has(issue.number)) return false;
return true;
});

// Validate up to 3 issues at a time to avoid overwhelming the system
const batchSize = 3;
const batch = issuesToValidate.slice(0, batchSize);

for (const issue of batch) {
autoValidatedIssuesRef.current.add(issue.number);
handleValidateIssue(issue);
}

if (batch.length > 0) {
logger.info(
`Auto-validating ${batch.length} issues (${issuesToValidate.length - batch.length} remaining)`
);
}
}, [
githubAutoValidate,
loading,
openIssues,
cachedValidations,
validatingIssues,
handleValidateIssue,
]);

// Reset auto-validated tracking when auto-validate is toggled off
useEffect(() => {
if (!githubAutoValidate) {
autoValidatedIssuesRef.current.clear();
}
}, [githubAutoValidate]);

// Get default AI profile for task creation
const defaultProfile = useMemo(() => {
if (!defaultAIProfileId) return null;
Expand Down Expand Up @@ -132,6 +188,42 @@ export function GitHubIssuesView() {
[currentProject?.path, defaultProfile, currentBranch]
);

// Handle bulk import of issues as tasks
const handleImportIssues = useCallback(
async (issues: GitHubIssue[]) => {
if (!currentProject?.path) {
toast.error('No project selected');
return;
}

let successCount = 0;
let errorCount = 0;

for (const issue of issues) {
const validation = cachedValidations.get(issue.number);
if (!validation?.result) {
errorCount++;
continue;
}

try {
await handleConvertToTask(issue, validation.result);
successCount++;
} catch {
errorCount++;
}
}
Comment on lines +199 to +215
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

For a better user experience with bulk imports, you can process the issue imports in parallel instead of sequentially. Using Promise.allSettled will execute all import operations concurrently, which can significantly speed up the process when importing multiple issues. This also allows for more robust error handling by collecting all results, whether fulfilled or rejected.

      const results = await Promise.allSettled(
        issues.map((issue) => {
          const validation = cachedValidations.get(issue.number);
          if (!validation?.result) {
            return Promise.reject(new Error(`Issue #${issue.number} is not validated.`));
          }
          return handleConvertToTask(issue, validation.result);
        })
      );

      const successCount = results.filter((r) => r.status === 'fulfilled').length;
      const errorCount = results.length - successCount;

      if (errorCount > 0) {
        const failedReasons = results
          .filter((r): r is PromiseRejectedResult => r.status === 'rejected')
          .map((r) => r.reason);
        logger.error('Failed to import some issues:', failedReasons);
      }


if (successCount > 0) {
toast.success(`Imported ${successCount} issue${successCount !== 1 ? 's' : ''} as tasks`);
}
if (errorCount > 0) {
toast.error(`Failed to import ${errorCount} issue${errorCount !== 1 ? 's' : ''}`);
}
},
[currentProject?.path, cachedValidations, handleConvertToTask]
);
Comment on lines +191 to +225
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

handleConvertToTask doesn't indicate success/failure to caller, causing incorrect counts.

handleConvertToTask handles errors internally with toast.error but doesn't return a success/failure indicator. This means handleImportIssues will always increment successCount for every issue, even when the task creation fails.

🐛 Suggested fix

First, modify handleConvertToTask to return a boolean:

   const handleConvertToTask = useCallback(
-    async (issue: GitHubIssue, validation: IssueValidationResult) => {
+    async (issue: GitHubIssue, validation: IssueValidationResult): Promise<boolean> => {
       if (!currentProject?.path) {
         toast.error('No project selected');
-        return;
+        return false;
       }

       try {
         const api = getElectronAPI();
         if (api.features?.create) {
           // ... build description and feature ...

           const result = await api.features.create(currentProject.path, feature);
           if (result.success) {
             toast.success(`Created task: ${issue.title}`);
+            return true;
           } else {
             toast.error(result.error || 'Failed to create task');
+            return false;
           }
         }
+        return false;
       } catch (err) {
         logger.error('Convert to task error:', err);
         toast.error(err instanceof Error ? err.message : 'Failed to create task');
+        return false;
       }
     },
     [currentProject?.path, defaultProfile, currentBranch]
   );

Then update handleImportIssues to use the return value:

       try {
-        await handleConvertToTask(issue, validation.result);
-        successCount++;
+        const success = await handleConvertToTask(issue, validation.result);
+        if (success) {
+          successCount++;
+        } else {
+          errorCount++;
+        }
       } catch {
         errorCount++;
       }
🤖 Prompt for AI Agents
In @apps/ui/src/components/views/github-issues-view.tsx around lines 191 - 225,
handleConvertToTask currently swallows errors and doesn't return status, so
update its signature to return a boolean (true on success, false on failure) and
ensure it returns false when any error occurs instead of only showing toast;
keep toast behavior but avoid throwing. Then update handleImportIssues to await
the boolean from handleConvertToTask (instead of assuming success) and increment
successCount only when it returns true, increment errorCount when it returns
false or when validation is missing; keep existing toast summaries. Also update
any other callers of handleConvertToTask to handle the new boolean return type.


if (loading) {
return <LoadingState />;
}
Expand All @@ -157,6 +249,9 @@ export function GitHubIssuesView() {
closedCount={closedIssues.length}
refreshing={refreshing}
onRefresh={refresh}
autoValidate={githubAutoValidate}
onAutoValidateChange={setGithubAutoValidate}
onImportClick={() => setShowImportDialog(true)}
/>

{/* Issues List */}
Expand Down Expand Up @@ -265,6 +360,16 @@ export function GitHubIssuesView() {
}
}}
/>

{/* Import Issues Dialog */}
<ImportIssuesDialog
open={showImportDialog}
onOpenChange={setShowImportDialog}
issues={openIssues}
cachedValidations={cachedValidations}
validatingIssues={validatingIssues}
onImport={handleImportIssues}
/>
</div>
);
}
Original file line number Diff line number Diff line change
@@ -1,19 +1,27 @@
import { CircleDot, RefreshCw } from 'lucide-react';
import { CircleDot, RefreshCw, Zap, Import } from 'lucide-react';
import { Button } from '@/components/ui/button';
import { Switch } from '@/components/ui/switch';
import { Tooltip, TooltipContent, TooltipProvider, TooltipTrigger } from '@/components/ui/tooltip';
import { cn } from '@/lib/utils';

interface IssuesListHeaderProps {
openCount: number;
closedCount: number;
refreshing: boolean;
onRefresh: () => void;
autoValidate: boolean;
onAutoValidateChange: (enabled: boolean) => void;
onImportClick: () => void;
}

export function IssuesListHeader({
openCount,
closedCount,
refreshing,
onRefresh,
autoValidate,
onAutoValidateChange,
onImportClick,
}: IssuesListHeaderProps) {
const totalIssues = openCount + closedCount;

Expand All @@ -30,9 +38,55 @@ export function IssuesListHeader({
</p>
</div>
</div>
<Button variant="outline" size="sm" onClick={onRefresh} disabled={refreshing}>
<RefreshCw className={cn('h-4 w-4', refreshing && 'animate-spin')} />
</Button>
<div className="flex items-center gap-3">
{/* Auto-validate toggle */}
<TooltipProvider>
<Tooltip>
<TooltipTrigger asChild>
<div className="flex items-center gap-2">
<Zap
className={cn(
'h-4 w-4',
autoValidate ? 'text-yellow-500' : 'text-muted-foreground'
)}
/>
<Switch
checked={autoValidate}
onCheckedChange={onAutoValidateChange}
aria-label="Auto-validate issues"
/>
</div>
</TooltipTrigger>
<TooltipContent>
<p>Auto-validate: {autoValidate ? 'ON' : 'OFF'}</p>
<p className="text-xs text-muted-foreground">
{autoValidate
? 'Issues are validated automatically when loaded'
: 'Click validate button for each issue'}
</p>
</TooltipContent>
</Tooltip>
</TooltipProvider>

{/* Import button */}
<TooltipProvider>
<Tooltip>
<TooltipTrigger asChild>
<Button variant="outline" size="sm" onClick={onImportClick}>
<Import className="h-4 w-4" />
</Button>
</TooltipTrigger>
<TooltipContent>
<p>Import issues as tasks</p>
</TooltipContent>
</Tooltip>
</TooltipProvider>

{/* Refresh button */}
<Button variant="outline" size="sm" onClick={onRefresh} disabled={refreshing}>
<RefreshCw className={cn('h-4 w-4', refreshing && 'animate-spin')} />
</Button>
</div>
Comment on lines +41 to +89
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

It's a best practice to wrap your application (or a section of it) in a single <TooltipProvider>. This is more efficient and cleaner than using a separate provider for each tooltip. You can wrap the div containing all the buttons with one <TooltipProvider>.

      <TooltipProvider>
        <div className="flex items-center gap-3">
          {/* Auto-validate toggle */}
          <Tooltip>
            <TooltipTrigger asChild>
              <div className="flex items-center gap-2">
                <Zap
                  className={cn(
                    'h-4 w-4',
                    autoValidate ? 'text-yellow-500' : 'text-muted-foreground'
                  )}
                />
                <Switch
                  checked={autoValidate}
                  onCheckedChange={onAutoValidateChange}
                  aria-label="Auto-validate issues"
                />
              </div>
            </TooltipTrigger>
            <TooltipContent>
              <p>Auto-validate: {autoValidate ? 'ON' : 'OFF'}</p>
              <p className="text-xs text-muted-foreground">
                {autoValidate
                  ? 'Issues are validated automatically when loaded'
                  : 'Click validate button for each issue'}
              </p>
            </TooltipContent>
          </Tooltip>

          {/* Import button */}
          <Tooltip>
            <TooltipTrigger asChild>
              <Button variant="outline" size="sm" onClick={onImportClick}>
                <Import className="h-4 w-4" />
              </Button>
            </TooltipTrigger>
            <TooltipContent>
              <p>Import issues as tasks</p>
            </TooltipContent>
          </Tooltip>

          {/* Refresh button */}
          <Button variant="outline" size="sm" onClick={onRefresh} disabled={refreshing}>
            <RefreshCw className={cn('h-4 w-4', refreshing && 'animate-spin')} />
          </Button>
        </div>
      </TooltipProvider>

</div>
);
}
Loading