Skip to content

Commit 53b99b2

Browse files
BunsDevBunsDev
authored
Unify git action flows and reduce sticky thread errors (#166)
1 parent 64b93f5 commit 53b99b2

4 files changed

Lines changed: 164 additions & 51 deletions

File tree

‎apps/web/src/components/GitActionsControl.tsx‎

Lines changed: 139 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,8 @@ interface GitActionFailureDialogState {
141141
retryInput: RetryableGitActionInput;
142142
}
143143

144+
type GitDialogAction = GitStackedAction;
145+
144146
const isGitActionFailure = Schema.is(GitActionFailureSchema);
145147

146148
function toRetryableGitActionInput(input: RunGitActionWithToastInput): RetryableGitActionInput {
@@ -262,9 +264,62 @@ function getMenuActionDisabledReason({
262264
return "Create PR is currently unavailable.";
263265
}
264266

265-
const COMMIT_DIALOG_TITLE = "Commit changes";
266-
const COMMIT_DIALOG_DESCRIPTION =
267-
"Review and confirm your commit. Leave the message blank to auto-generate one.";
267+
function dialogIncludesCommit(action: GitDialogAction | null, gitStatus: GitStatusResult | null): boolean {
268+
if (!action) return false;
269+
return action === "commit" || !!gitStatus?.hasWorkingTreeChanges;
270+
}
271+
272+
function resolveDialogCopy(input: {
273+
action: GitDialogAction | null;
274+
includesCommit: boolean;
275+
}): {
276+
title: string;
277+
description: string;
278+
confirmLabel: string;
279+
newBranchLabel: string;
280+
} {
281+
if (input.action === "commit_push_pr") {
282+
if (input.includesCommit) {
283+
return {
284+
title: "Commit, push, and create PR",
285+
description:
286+
"Review the commit details, then continue through the full publish flow with PR creation.",
287+
confirmLabel: "Commit, push & create PR",
288+
newBranchLabel: "Use new branch instead",
289+
};
290+
}
291+
return {
292+
title: "Create pull request",
293+
description: "Push local commits if needed, then create a pull request for this branch.",
294+
confirmLabel: "Push & create PR",
295+
newBranchLabel: "Use new branch instead",
296+
};
297+
}
298+
299+
if (input.action === "commit_push") {
300+
if (input.includesCommit) {
301+
return {
302+
title: "Commit and push changes",
303+
description: "Review the commit details, then publish this branch.",
304+
confirmLabel: "Commit & push",
305+
newBranchLabel: "Use new branch instead",
306+
};
307+
}
308+
return {
309+
title: "Push branch",
310+
description: "Push local commits on this branch.",
311+
confirmLabel: "Push",
312+
newBranchLabel: "Use new branch instead",
313+
};
314+
}
315+
316+
return {
317+
title: "Commit changes",
318+
description: "Review and confirm your commit. Leave the message blank to auto-generate one.",
319+
confirmLabel: "Commit",
320+
newBranchLabel: "Commit on new branch",
321+
};
322+
}
268323

269324
function GitActionItemIcon({ icon }: { icon: GitActionIconName }) {
270325
if (icon === "commit") return <GitCommitIcon />;
@@ -299,7 +354,7 @@ export default function GitActionsControl({ gitCwd, activeThreadId }: GitActions
299354
[activeThreadId],
300355
);
301356
const queryClient = useQueryClient();
302-
const [isCommitDialogOpen, setIsCommitDialogOpen] = useState(false);
357+
const [activeDialogAction, setActiveDialogAction] = useState<GitDialogAction | null>(null);
303358
const [dialogCommitMessage, setDialogCommitMessage] = useState("");
304359
const [excludedFiles, setExcludedFiles] = useState<ReadonlySet<string>>(new Set());
305360
const [isEditingFiles, setIsEditingFiles] = useState(false);
@@ -345,6 +400,11 @@ export default function GitActionsControl({ gitCwd, activeThreadId }: GitActions
345400
const selectedFiles = allFiles.filter((f) => !excludedFiles.has(f.path));
346401
const allSelected = excludedFiles.size === 0;
347402
const noneSelected = selectedFiles.length === 0;
403+
const activeDialogIncludesCommit = dialogIncludesCommit(activeDialogAction, gitStatusForActions);
404+
const activeDialogCopy = resolveDialogCopy({
405+
action: activeDialogAction,
406+
includesCommit: activeDialogIncludesCommit,
407+
});
348408

349409
const initMutation = useMutation(gitInitMutationOptions({ cwd: gitCwd, queryClient }));
350410

@@ -824,22 +884,28 @@ export default function GitActionsControl({ gitCwd, activeThreadId }: GitActions
824884
}, [pendingDefaultBranchAction]);
825885

826886
const runDialogActionOnNewBranch = useCallback(() => {
827-
if (!isCommitDialogOpen) return;
887+
if (!activeDialogAction || !activeDialogIncludesCommit) return;
828888
const commitMessage = dialogCommitMessage.trim();
829889

830-
setIsCommitDialogOpen(false);
890+
setActiveDialogAction(null);
831891
setDialogCommitMessage("");
832892
setExcludedFiles(new Set());
833893
setIsEditingFiles(false);
834894

835895
void runGitActionWithToast({
836-
action: "commit",
896+
action: activeDialogAction,
837897
...(commitMessage ? { commitMessage } : {}),
838898
...(!allSelected ? { filePaths: selectedFiles.map((f) => f.path) } : {}),
839899
featureBranch: true,
840900
skipDefaultBranchPrompt: true,
841901
});
842-
}, [allSelected, isCommitDialogOpen, dialogCommitMessage, selectedFiles]);
902+
}, [
903+
activeDialogAction,
904+
activeDialogIncludesCommit,
905+
allSelected,
906+
dialogCommitMessage,
907+
selectedFiles,
908+
]);
843909

844910
const conflictedFiles = useMemo(
845911
() => gitStatusForActions?.conflictedFiles ?? [],
@@ -1002,9 +1068,18 @@ export default function GitActionsControl({ gitCwd, activeThreadId }: GitActions
10021068
return;
10031069
}
10041070
if (quickAction.action) {
1005-
void runGitActionWithToast({ action: quickAction.action });
1071+
setDialogCommitMessage("");
1072+
setExcludedFiles(new Set());
1073+
setIsEditingFiles(false);
1074+
setActiveDialogAction(quickAction.action);
10061075
}
1007-
}, [openConflictedFilesInEditor, openExistingPr, quickAction, runPullWithToast, threadToastData]);
1076+
}, [
1077+
openConflictedFilesInEditor,
1078+
openExistingPr,
1079+
quickAction,
1080+
runPullWithToast,
1081+
threadToastData,
1082+
]);
10081083

10091084
const openDialogForMenuItem = useCallback(
10101085
(item: GitActionMenuItem) => {
@@ -1013,40 +1088,48 @@ export default function GitActionsControl({ gitCwd, activeThreadId }: GitActions
10131088
void openExistingPr();
10141089
return;
10151090
}
1091+
setDialogCommitMessage("");
1092+
setExcludedFiles(new Set());
1093+
setIsEditingFiles(false);
10161094
if (item.dialogAction === "push") {
1017-
void runGitActionWithToast({ action: "commit_push", forcePushOnlyProgress: true });
1095+
setActiveDialogAction("commit_push");
10181096
return;
10191097
}
10201098
if (item.dialogAction === "create_pr") {
1021-
void runGitActionWithToast({ action: "commit_push_pr" });
1099+
setActiveDialogAction("commit_push_pr");
10221100
return;
10231101
}
1024-
setExcludedFiles(new Set());
1025-
setIsEditingFiles(false);
1026-
setIsCommitDialogOpen(true);
1102+
setActiveDialogAction("commit");
10271103
},
1028-
[openExistingPr, setIsCommitDialogOpen],
1104+
[openExistingPr],
10291105
);
10301106

10311107
const runDialogAction = useCallback(() => {
1032-
if (!isCommitDialogOpen) return;
1108+
if (!activeDialogAction) return;
10331109
const commitMessage = dialogCommitMessage.trim();
1034-
setIsCommitDialogOpen(false);
1110+
const includesCommit = dialogIncludesCommit(activeDialogAction, gitStatusForActions);
1111+
setActiveDialogAction(null);
10351112
setDialogCommitMessage("");
10361113
setExcludedFiles(new Set());
10371114
setIsEditingFiles(false);
10381115
void runGitActionWithToast({
1039-
action: "commit",
1040-
...(commitMessage ? { commitMessage } : {}),
1041-
...(!allSelected ? { filePaths: selectedFiles.map((f) => f.path) } : {}),
1116+
action: activeDialogAction,
1117+
...(includesCommit && commitMessage ? { commitMessage } : {}),
1118+
...(includesCommit
1119+
? !allSelected
1120+
? { filePaths: selectedFiles.map((f) => f.path) }
1121+
: {}
1122+
: activeDialogAction !== "commit"
1123+
? { forcePushOnlyProgress: true }
1124+
: {}),
10421125
});
10431126
}, [
1127+
activeDialogAction,
10441128
allSelected,
10451129
dialogCommitMessage,
1046-
isCommitDialogOpen,
1130+
gitStatusForActions,
10471131
selectedFiles,
10481132
setDialogCommitMessage,
1049-
setIsCommitDialogOpen,
10501133
]);
10511134

10521135
const openChangedFileInEditor = useCallback(
@@ -1309,10 +1392,10 @@ export default function GitActionsControl({ gitCwd, activeThreadId }: GitActions
13091392
)}
13101393

13111394
<Dialog
1312-
open={isCommitDialogOpen}
1395+
open={activeDialogAction !== null}
13131396
onOpenChange={(open: boolean) => {
13141397
if (!open) {
1315-
setIsCommitDialogOpen(false);
1398+
setActiveDialogAction(null);
13161399
setDialogCommitMessage("");
13171400
setExcludedFiles(new Set());
13181401
setIsEditingFiles(false);
@@ -1321,8 +1404,8 @@ export default function GitActionsControl({ gitCwd, activeThreadId }: GitActions
13211404
>
13221405
<DialogPopup>
13231406
<DialogHeader>
1324-
<DialogTitle>{COMMIT_DIALOG_TITLE}</DialogTitle>
1325-
<DialogDescription>{COMMIT_DIALOG_DESCRIPTION}</DialogDescription>
1407+
<DialogTitle>{activeDialogCopy.title}</DialogTitle>
1408+
<DialogDescription>{activeDialogCopy.description}</DialogDescription>
13261409
</DialogHeader>
13271410
<DialogPanel className="space-y-4">
13281411
<div className="space-y-3 rounded-lg border border-input bg-muted/40 p-3 text-xs">
@@ -1340,7 +1423,7 @@ export default function GitActionsControl({ gitCwd, activeThreadId }: GitActions
13401423
<div className="space-y-1">
13411424
<div className="flex items-center justify-between">
13421425
<div className="flex items-center gap-2">
1343-
{isEditingFiles && allFiles.length > 0 && (
1426+
{activeDialogIncludesCommit && isEditingFiles && allFiles.length > 0 && (
13441427
<Checkbox
13451428
checked={allSelected}
13461429
indeterminate={!allSelected && !noneSelected}
@@ -1352,13 +1435,13 @@ export default function GitActionsControl({ gitCwd, activeThreadId }: GitActions
13521435
/>
13531436
)}
13541437
<span className="text-muted-foreground">Files</span>
1355-
{!allSelected && !isEditingFiles && (
1438+
{activeDialogIncludesCommit && !allSelected && !isEditingFiles && (
13561439
<span className="text-muted-foreground">
13571440
({selectedFiles.length} of {allFiles.length})
13581441
</span>
13591442
)}
13601443
</div>
1361-
{allFiles.length > 0 && (
1444+
{activeDialogIncludesCommit && allFiles.length > 0 && (
13621445
<Button
13631446
variant="ghost"
13641447
size="xs"
@@ -1381,7 +1464,7 @@ export default function GitActionsControl({ gitCwd, activeThreadId }: GitActions
13811464
key={file.path}
13821465
className="flex w-full items-center gap-2 rounded-md px-2 py-1 font-mono text-xs transition-colors hover:bg-accent/50"
13831466
>
1384-
{isEditingFiles && (
1467+
{activeDialogIncludesCommit && isEditingFiles && (
13851468
<Checkbox
13861469
checked={!excludedFiles.has(file.path)}
13871470
onCheckedChange={() => {
@@ -1437,39 +1520,47 @@ export default function GitActionsControl({ gitCwd, activeThreadId }: GitActions
14371520
)}
14381521
</div>
14391522
</div>
1440-
<div className="space-y-1">
1441-
<p className="text-xs font-medium">Commit message (optional)</p>
1442-
<Textarea
1443-
value={dialogCommitMessage}
1444-
onChange={(event) => setDialogCommitMessage(event.target.value)}
1445-
placeholder="Leave empty to auto-generate"
1446-
size="sm"
1447-
/>
1448-
</div>
1523+
{activeDialogIncludesCommit ? (
1524+
<div className="space-y-1">
1525+
<p className="text-xs font-medium">Commit message (optional)</p>
1526+
<Textarea
1527+
value={dialogCommitMessage}
1528+
onChange={(event) => setDialogCommitMessage(event.target.value)}
1529+
placeholder="Leave empty to auto-generate"
1530+
size="sm"
1531+
/>
1532+
</div>
1533+
) : null}
14491534
</DialogPanel>
14501535
<DialogFooter>
14511536
<Button
14521537
variant="outline"
14531538
size="sm"
14541539
onClick={() => {
1455-
setIsCommitDialogOpen(false);
1540+
setActiveDialogAction(null);
14561541
setDialogCommitMessage("");
14571542
setExcludedFiles(new Set());
14581543
setIsEditingFiles(false);
14591544
}}
14601545
>
14611546
Cancel
14621547
</Button>
1548+
{activeDialogIncludesCommit ? (
1549+
<Button
1550+
variant="outline"
1551+
size="sm"
1552+
disabled={noneSelected}
1553+
onClick={runDialogActionOnNewBranch}
1554+
>
1555+
{activeDialogCopy.newBranchLabel}
1556+
</Button>
1557+
) : null}
14631558
<Button
1464-
variant="outline"
14651559
size="sm"
1466-
disabled={noneSelected}
1467-
onClick={runDialogActionOnNewBranch}
1560+
disabled={activeDialogIncludesCommit && noneSelected}
1561+
onClick={runDialogAction}
14681562
>
1469-
Commit on new branch
1470-
</Button>
1471-
<Button size="sm" disabled={noneSelected} onClick={runDialogAction}>
1472-
Commit
1563+
{activeDialogCopy.confirmLabel}
14731564
</Button>
14741565
</DialogFooter>
14751566
</DialogPopup>

‎apps/web/src/components/Sidebar.logic.test.ts‎

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,14 +128,36 @@ describe("resolveThreadStatusPill", () => {
128128
resolveThreadStatusPill({
129129
thread: {
130130
...baseThread,
131-
error: "Socket disconnected",
131+
session: {
132+
...baseThread.session,
133+
status: "error",
134+
orchestrationStatus: "error",
135+
},
132136
},
133137
hasPendingApprovals: true,
134138
hasPendingUserInput: true,
135139
}),
136140
).toMatchObject({ label: "Error", pulse: false });
137141
});
138142

143+
it("ignores historical error text when the session is no longer errored", () => {
144+
expect(
145+
resolveThreadStatusPill({
146+
thread: {
147+
...baseThread,
148+
error: "Socket disconnected",
149+
session: {
150+
...baseThread.session,
151+
status: "ready",
152+
orchestrationStatus: "ready",
153+
},
154+
},
155+
hasPendingApprovals: false,
156+
hasPendingUserInput: false,
157+
}),
158+
).not.toMatchObject({ label: "Error" });
159+
});
160+
139161
it("shows awaiting input when plan mode is blocked on user answers", () => {
140162
expect(
141163
resolveThreadStatusPill({

‎apps/web/src/components/Sidebar.logic.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ export function resolveThreadStatusPill(input: {
119119
}): ThreadStatusPill | null {
120120
const { hasPendingApprovals, hasPendingUserInput, thread } = input;
121121

122-
if (thread.error || thread.session?.status === "error") {
122+
if (thread.session?.status === "error") {
123123
return {
124124
label: "Error",
125125
colorClass: "text-rose-600 dark:text-rose-300/90",

‎apps/web/src/store.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -300,7 +300,7 @@ export function syncServerReadModel(state: AppState, readModel: OrchestrationRea
300300
createdAt: proposedPlan.createdAt,
301301
updatedAt: proposedPlan.updatedAt,
302302
})),
303-
error: thread.session?.lastError ?? null,
303+
error: thread.session?.status === "error" ? (thread.session.lastError ?? null) : null,
304304
createdAt: thread.createdAt,
305305
updatedAt: thread.updatedAt,
306306
latestTurn: thread.latestTurn,

0 commit comments

Comments
 (0)