ros/automaker
1
0
Fork 0
mirror of https://github.com/AutoMaker-Org/automaker.git synced 2026-01-30 06:12:03 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: address PR 161 review comments

Browse Source 
- Fix unknown status bypassing worktree filtering in use-board-column-features.ts
- Remove unused props projectPath and onWorktreeCreated from use-board-drag-drop.ts
- Fix test expecting worktreePath during edit (worktrees created at execution time)
- Remove unused setAutoModeRunning from dependency array
- Remove unused imports (BranchAutocomplete, cn)
- Fix htmlFor accessibility issue in branch-selector.tsx
- Remove empty finally block in resume-feature.ts
- Remove duplicate setTimeout state reset in create-pr-dialog.tsx
- Consolidate duplicate state reset logic in context-view.tsx
- Simplify branch name defaulting logic in use-board-actions.ts
- Fix branchName reset to null when worktree is deleted
This commit is contained in:
Cody Seibert
2025-12-17 23:24:54 -05:00
parent f7cb92fa9d
commit 35c6beca37
12 changed files with 67 additions and 78 deletions
Download Patch File Download Diff File Expand all files Collapse all files

53
apps/app/src/components/views/board-view.tsx
View File

@@ -412,6 +412,12 @@ export function BoardView() {
autoModeRunningRef.current = autoMode.isRunning;
}, [autoMode.isRunning]);
// Use a ref to track the latest features to avoid effect re-runs when features change
const hookFeaturesRef = useRef(hookFeatures);
useEffect(() => {
hookFeaturesRef.current = hookFeatures;
}, [hookFeatures]);
// Track features that are pending (started but not yet confirmed running)
const pendingFeaturesRef = useRef<Set<string>>(new Set());
@@ -489,33 +495,30 @@ export function BoardView() {
}
// Filter backlog features by the currently selected worktree branch
const primaryBranch = currentProject.path
? getPrimaryWorktreeBranch(currentProject.path)
: null;
const backlogFeatures = hookFeatures.filter((f) => {
// This logic mirrors use-board-column-features.ts for consistency
// Use ref to get the latest features without causing effect re-runs
const currentFeatures = hookFeaturesRef.current;
const backlogFeatures = currentFeatures.filter((f) => {
if (f.status !== "backlog") return false;
// Determine the feature's branch (default to primary branch if not set)
const featureBranch = f.branchName || primaryBranch || "main";
const featureBranch = f.branchName;
// If no worktree is selected (currentWorktreeBranch is null or matches primary),
// show features with no branch or primary branch
if (
!currentWorktreeBranch ||
(currentProject.path &&
isPrimaryWorktreeBranch(
currentProject.path,
currentWorktreeBranch
))
) {
return (
!f.branchName ||
(currentProject.path &&
isPrimaryWorktreeBranch(currentProject.path, featureBranch))
);
// Features without branchName are considered unassigned (show only on primary worktree)
if (!featureBranch) {
// No branch assigned - show only when viewing primary worktree
const isViewingPrimary = currentWorktreePath === null;
return isViewingPrimary;
}
// Otherwise, only show features matching the selected worktree branch
if (currentWorktreeBranch === null) {
// We're viewing main but branch hasn't been initialized yet
// Show features assigned to primary worktree's branch
return currentProject.path
? isPrimaryWorktreeBranch(currentProject.path, featureBranch)
: false;
}
// Match by branch name
return featureBranch === currentWorktreeBranch;
});
@@ -531,7 +534,7 @@ export function BoardView() {
// Filter out features with blocking dependencies if dependency blocking is enabled
const eligibleFeatures = enableDependencyBlocking
? sortedBacklog.filter((f) => {
const blockingDeps = getBlockingDependencies(f, hookFeatures);
const blockingDeps = getBlockingDependencies(f, currentFeatures);
return blockingDeps.length === 0;
})
: sortedBacklog;
@@ -591,7 +594,7 @@ export function BoardView() {
currentProject,
runningAutoTasks,
maxConcurrency,
hookFeatures,
// hookFeatures is accessed via hookFeaturesRef to prevent effect re-runs
currentWorktreeBranch,
currentWorktreePath,
getPrimaryWorktreeBranch,
@@ -617,8 +620,6 @@ export function BoardView() {
runningAutoTasks,
persistFeatureUpdate,
handleStartImplementation,
projectPath: currentProject?.path || null,
onWorktreeCreated: () => setWorktreeRefreshKey((k) => k + 1),
});
// Use column features hook

1
apps/app/src/components/views/board-view/dialogs/add-feature-dialog.tsx
View File

@@ -14,7 +14,6 @@ import { Button } from "@/components/ui/button";
import { HotkeyButton } from "@/components/ui/hotkey-button";
import { Label } from "@/components/ui/label";
import { CategoryAutocomplete } from "@/components/ui/category-autocomplete";
import { BranchAutocomplete } from "@/components/ui/branch-autocomplete";
import {
DescriptionImageDropZone,
FeatureImagePath as DescriptionImagePath,

34
apps/app/src/components/views/board-view/dialogs/create-pr-dialog.tsx
View File

@@ -1,6 +1,6 @@
"use client";
import { useState, useEffect } from "react";
import { useState, useEffect, useRef } from "react";
import {
Dialog,
DialogContent,
@@ -49,6 +49,8 @@ export function CreatePRDialog({
const [prUrl, setPrUrl] = useState<string | null>(null);
const [browserUrl, setBrowserUrl] = useState<string | null>(null);
const [showBrowserFallback, setShowBrowserFallback] = useState(false);
// Track whether an operation completed that warrants a refresh
const operationCompletedRef = useRef(false);
// Reset state when dialog opens or worktree changes
useEffect(() => {
@@ -65,6 +67,8 @@ export function CreatePRDialog({
setPrUrl(null);
setBrowserUrl(null);
setShowBrowserFallback(false);
// Reset operation tracking
operationCompletedRef.current = false;
} else {
// Reset everything when dialog closes
setTitle("");
@@ -76,6 +80,7 @@ export function CreatePRDialog({
setPrUrl(null);
setBrowserUrl(null);
setShowBrowserFallback(false);
operationCompletedRef.current = false;
}
}, [open, worktree?.path]);
@@ -102,6 +107,8 @@ export function CreatePRDialog({
if (result.success && result.result) {
if (result.result.prCreated && result.result.prUrl) {
setPrUrl(result.result.prUrl);
// Mark operation as completed for refresh on close
operationCompletedRef.current = true;
toast.success("Pull request created!", {
description: `PR created from ${result.result.branch}`,
action: {
@@ -122,6 +129,8 @@ export function CreatePRDialog({
if (prError === "gh_cli_not_available" || !result.result.ghCliAvailable) {
setBrowserUrl(result.result.browserUrl ?? null);
setShowBrowserFallback(true);
// Mark operation as completed - branch was pushed successfully
operationCompletedRef.current = true;
toast.success("Branch pushed", {
description: result.result.committed
? `Commit ${result.result.commitHash} pushed to ${result.result.branch}`
@@ -147,6 +156,8 @@ export function CreatePRDialog({
// Show error but also provide browser option
setBrowserUrl(result.result.browserUrl ?? null);
setShowBrowserFallback(true);
// Mark operation as completed - branch was pushed even though PR creation failed
operationCompletedRef.current = true;
toast.error("PR creation failed", {
description: errorMessage,
duration: 8000,
@@ -187,22 +198,13 @@ export function CreatePRDialog({
};
const handleClose = () => {
// Call onCreated() to refresh worktrees when dialog closes
// This ensures the worktree list is updated after any operation
onCreated();
// Only call onCreated() if an actual operation completed
// This prevents unnecessary refreshes when user cancels
if (operationCompletedRef.current) {
onCreated();
}
onOpenChange(false);
// Reset state after dialog closes
setTimeout(() => {
setTitle("");
setBody("");
setCommitMessage("");
setBaseBranch("main");
setIsDraft(false);
setError(null);
setPrUrl(null);
setBrowserUrl(null);
setShowBrowserFallback(false);
}, 200);
// State reset is handled by useEffect when open becomes false
};
if (!worktree) return null;

1
apps/app/src/components/views/board-view/dialogs/edit-feature-dialog.tsx
View File

@@ -14,7 +14,6 @@ import { Button } from "@/components/ui/button";
import { HotkeyButton } from "@/components/ui/hotkey-button";
import { Label } from "@/components/ui/label";
import { CategoryAutocomplete } from "@/components/ui/category-autocomplete";
import { BranchAutocomplete } from "@/components/ui/branch-autocomplete";
import {
DescriptionImageDropZone,
FeatureImagePath as DescriptionImagePath,

8
apps/app/src/components/views/board-view/hooks/use-board-actions.ts
View File

@@ -103,10 +103,7 @@ export function useBoardActions({
// Simplified: Only store branchName, no worktree creation on add
// Worktrees are created at execution time (when feature starts)
// Empty string means user chose "use current branch" - don't save a branch name
const finalBranchName =
featureData.branchName === ""
? undefined
: featureData.branchName || undefined;
const finalBranchName = featureData.branchName || undefined;
const newFeatureData = {
...featureData,
@@ -139,8 +136,7 @@ export function useBoardActions({
requirePlanApproval?: boolean;
}
) => {
const finalBranchName =
updates.branchName === "" ? undefined : updates.branchName || undefined;
const finalBranchName = updates.branchName || undefined;
const finalUpdates = {
...updates,

4
apps/app/src/components/views/board-view/hooks/use-board-column-features.ts
View File

@@ -98,7 +98,9 @@ export function useBoardColumnFeatures({
}
} else {
// Unknown status, default to backlog
map.backlog.push(f);
if (matchesWorktree) {
map.backlog.push(f);
}
}
}
});

4
apps/app/src/components/views/board-view/hooks/use-board-drag-drop.ts
View File

@@ -14,8 +14,6 @@ interface UseBoardDragDropProps {
updates: Partial<Feature>
) => Promise<void>;
handleStartImplementation: (feature: Feature) => Promise<boolean>;
projectPath: string | null; // Main project path
onWorktreeCreated?: () => void; // Callback when a new worktree is created
}
export function useBoardDragDrop({
@@ -24,8 +22,6 @@ export function useBoardDragDrop({
runningAutoTasks,
persistFeatureUpdate,
handleStartImplementation,
projectPath,
onWorktreeCreated,
}: UseBoardDragDropProps) {
const [activeFeature, setActiveFeature] = useState<Feature | null>(null);
const { moveFeature } = useAppStore();

4
apps/app/src/components/views/board-view/shared/branch-selector.tsx
View File

@@ -3,7 +3,6 @@
import { Label } from "@/components/ui/label";
import { BranchAutocomplete } from "@/components/ui/branch-autocomplete";
import { RadioGroup, RadioGroupItem } from "@/components/ui/radio-group";
import { cn } from "@/lib/utils";
interface BranchSelectorProps {
useCurrentBranch: boolean;
@@ -32,12 +31,13 @@ export function BranchSelector({
return (
<div className="space-y-2">
<Label htmlFor={`${testIdPrefix}-selector`}>Target Branch</Label>
<Label id={`${testIdPrefix}-label`}>Target Branch</Label>
<RadioGroup
value={useCurrentBranch ? "current" : "other"}
onValueChange={(value: string) => onUseCurrentBranchChange(value === "current")}
disabled={disabled}
data-testid={`${testIdPrefix}-radio-group`}
aria-labelledby={`${testIdPrefix}-label`}
>
<div className="flex items-center space-x-2">
<RadioGroupItem value="current" id={`${testIdPrefix}-current`} />

14
apps/app/src/components/views/context-view.tsx
View File

@@ -212,20 +212,14 @@ export function ContextView() {
// Write text file with content (or empty if no content)
await api.writeFile(filePath, newFileContent);
}
// Close dialog and reset state immediately after successful file write
setIsAddDialogOpen(false);
setNewFileName("");
setNewFileType("text");
setUploadedImageData(null);
setNewFileContent("");
setIsDropHovering(false);
// Load files after dialog is closed
// Only reload files on success
await loadContextFiles();
} catch (error) {
console.error("Failed to add file:", error);
// Still close the dialog even if loadContextFiles fails
// Optionally show error toast to user here
} finally {
// Close dialog and reset state
setIsAddDialogOpen(false);
setNewFileName("");
setNewFileType("text");

1
apps/app/src/hooks/use-auto-mode.ts
View File

@@ -327,7 +327,6 @@ export function useAutoMode() {
projectId,
addRunningTask,
removeRunningTask,
setAutoModeRunning,
addAutoModeActivity,
getProjectIdFromPath,
setPendingPlanApproval,

18
apps/app/tests/worktree-integration.spec.ts
View File

@@ -2353,7 +2353,7 @@ test.describe("Worktree Integration Tests", () => {
// Edit Feature with Branch Change
// ==========================================================================
test("should create worktree when editing a feature and selecting a new branch", async ({
test("should update branchName when editing a feature and selecting a new branch", async ({
page,
}) => {
await setupProjectWithPath(page, testRepo.path);
@@ -2399,7 +2399,7 @@ test.describe("Worktree Integration Tests", () => {
const newBranchName = "feature/edited-branch";
const expectedWorktreePath = getWorktreePath(testRepo.path, newBranchName);
// Verify worktree does NOT exist before editing
// Verify worktree does NOT exist before editing (worktrees are created at execution time)
expect(fs.existsSync(expectedWorktreePath)).toBe(false);
// Find and click the edit button on the feature card
@@ -2431,20 +2431,22 @@ test.describe("Worktree Integration Tests", () => {
const saveButton = page.locator('[data-testid="confirm-edit-feature"]');
await saveButton.click();
// Wait for the dialog to close and worktree to be created
// Wait for the dialog to close
await page.waitForTimeout(2000);
// Verify worktree was automatically created
expect(fs.existsSync(expectedWorktreePath)).toBe(true);
// Verify worktree was NOT created during editing (worktrees are created at execution time)
expect(fs.existsSync(expectedWorktreePath)).toBe(false);
// Verify the branch was created
// Verify the branch was created (if branch creation is part of the autocomplete flow)
const branches = await listBranches(testRepo.path);
expect(branches).toContain(newBranchName);
// Verify feature was updated with correct branch and worktreePath
// Verify feature was updated with correct branchName only
// Note: worktreePath is no longer stored - worktrees are created server-side at execution time
featureData = JSON.parse(fs.readFileSync(featureFilePath, "utf-8"));
expect(featureData.branchName).toBe(newBranchName);
expect(featureData.worktreePath).toBe(expectedWorktreePath);
// worktreePath should not exist in the feature data
expect(featureData.worktreePath).toBeUndefined();
});
test("should not create worktree when editing a feature and selecting main branch", async ({

3
apps/server/src/routes/auto-mode/routes/resume-feature.ts
View File

@@ -32,8 +32,7 @@ export function createResumeFeatureHandler(autoModeService: AutoModeService) {
.resumeFeature(projectPath, featureId, useWorktrees ?? false)
.catch((error) => {
logger.error(`[AutoMode] Resume feature ${featureId} error:`, error);
})
.finally(() => {});
});
res.json({ success: true });
} catch (error) {