Skip to content

Commit 17b2a22

Browse files
authored
fix(dexdex): align plan shortcuts and subtask dispatch contracts (#289)
## Summary - align plan decision keyboard contract to `A=Approve`, `V=Revise`, `Shift+X=Reject/Cancel` and move plan shortcut handling into the plan decision surface - fix waiting-session global shortcut flow by restoring immediate session input context on `/tasks?waitingSession=<session_id>` - add an explicit dispatcher path for executing already-created subtasks and wire `CreateSubTask`/`RetrySubTask` to dispatch the same created subtask ID - make `RunAutoFixNow` create and immediately dispatch a `PR_REVIEW_FIX` subtask (manual auto-fix path) - sync DexDex docs for shortcut semantics and clarify that poller-driven `auto_fix_enabled` auto-dispatch remains planned ## Validation - `go test ./servers/dexdex-main-server/...` - `cd apps/dexdex && pnpm test` ## Notes - no external proto/API schema changes - `buf generate` for dexdex web plugins is currently blocked in this environment due missing local protoc plugins (`protoc-gen-es`, `protoc-gen-connect-query`)
1 parent 97cd78a commit 17b2a22

14 files changed

Lines changed: 508 additions & 92 deletions

apps/dexdex/src/App.test.tsx

Lines changed: 51 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ import {
3636
PrStatus,
3737
AgentCliType,
3838
WorkspaceType,
39+
PlanDecision as ProtoPlanDecision,
3940
PullRequestRecordSchema,
4041
} from "./gen/v1/dexdex_pb";
4142
import { AUTO_REPOSITORY_GROUP_PREFIX } from "./lib/repository-target";
@@ -268,6 +269,13 @@ let lastCreateUnitTaskRequest:
268269
repositoryId?: string;
269270
}
270271
| null = null;
272+
let lastSubmitPlanDecisionRequest:
273+
| {
274+
subTaskId?: string;
275+
decision?: ProtoPlanDecision;
276+
revisionNote?: string;
277+
}
278+
| null = null;
271279

272280
interface TestTransportOptions {
273281
workspaces?: Array<{
@@ -321,10 +329,17 @@ function createTestTransport(options: TestTransportOptions = {}) {
321329
}),
322330
};
323331
},
324-
submitPlanDecision: () => ({
325-
updatedSubTask: undefined,
326-
createdSubTask: undefined,
327-
}),
332+
submitPlanDecision: (req) => {
333+
lastSubmitPlanDecisionRequest = {
334+
subTaskId: req.subTaskId,
335+
decision: req.decision,
336+
revisionNote: req.revisionNote,
337+
};
338+
return {
339+
updatedSubTask: undefined,
340+
createdSubTask: undefined,
341+
};
342+
},
328343
});
329344
router.service(NotificationService, {
330345
listNotifications: () => ({ notifications: mockNotifications }),
@@ -489,6 +504,7 @@ function renderWithProviders(
489504
beforeEach(() => {
490505
localStorageMock.clear();
491506
lastCreateUnitTaskRequest = null;
507+
lastSubmitPlanDecisionRequest = null;
492508
document.documentElement.classList.remove("dark");
493509
localStorageMock.setItem("dexdex-active-workspace-id", DEFAULT_WORKSPACE_ID);
494510
});
@@ -793,6 +809,37 @@ describe("App", () => {
793809
});
794810
});
795811

812+
it("submits revise plan decision with keyboard shortcut using sub_task_id", async () => {
813+
const user = userEvent.setup();
814+
renderWithProviders(<App />);
815+
816+
await screen.findByTestId("task-row-task-002");
817+
await user.click(screen.getByTestId("task-row-task-002"));
818+
await user.keyboard("v");
819+
820+
const reviseInput = await screen.findByTestId("revision-note-input");
821+
await user.type(reviseInput, "Please add rollback verification tests.");
822+
await user.keyboard("{Meta>}{Enter}{/Meta}");
823+
824+
await waitFor(() => {
825+
expect(lastSubmitPlanDecisionRequest?.subTaskId).toBe("sub-002-1");
826+
expect(lastSubmitPlanDecisionRequest?.decision).toBe(ProtoPlanDecision.REVISE);
827+
expect(lastSubmitPlanDecisionRequest?.revisionNote).toBe("Please add rollback verification tests.");
828+
});
829+
});
830+
831+
it("shows waiting-session input context on tasks route", async () => {
832+
renderWithProviders(<App />, {
833+
initialEntries: ["/tasks?waitingSession=sess-004-1"],
834+
});
835+
836+
expect(await screen.findByTestId("waiting-session-context")).toBeTruthy();
837+
const sessionInput = await screen.findByTestId("session-input-textarea");
838+
await waitFor(() => {
839+
expect(document.activeElement).toBe(sessionInput);
840+
});
841+
});
842+
796843
it("auto-focuses session input for waiting-for-input subtasks", async () => {
797844
const user = userEvent.setup();
798845
renderWithProviders(<App />);

apps/dexdex/src/App.tsx

Lines changed: 47 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import { PrDetailPage } from "./features/prs/pr-detail-page";
2020
import { SettingsPage } from "./features/settings/settings-page";
2121
import { RepositoryGroupsPage } from "./features/repositories/repository-groups-page";
2222
import { RepositoriesPage } from "./features/repositories/repositories-page";
23+
import { SessionInputForm } from "./features/sessions/session-input-form";
2324
import { useKeyboardShortcuts } from "./hooks/use-keyboard-shortcuts";
2425
import { useWorkspaceStream } from "./hooks/use-workspace-stream";
2526
import { useTrayStatus } from "./hooks/use-tray-status";
@@ -267,25 +268,6 @@ function App() {
267268
handleTabClick(nextTab);
268269
}
269270
},
270-
onApprovePlan: () => {
271-
// Context-sensitive: only if on task detail with waiting subtask
272-
if (!currentPath.startsWith("/tasks/")) return;
273-
const taskId = currentPath.replace("/tasks/", "");
274-
const task = tasks.find((t) => t.unitTaskId === taskId);
275-
if (task) {
276-
// We need to find a waiting subtask - delegate to handlePlanDecision
277-
// This is a simplified version - the full version would need subtask data
278-
handlePlanDecision(taskId, PlanDecision.APPROVE);
279-
}
280-
},
281-
onRevisePlan: () => {
282-
// For V key - context sensitive
283-
if (!currentPath.startsWith("/tasks/")) return;
284-
},
285-
onRejectPlan: () => {
286-
// For Shift+X - context sensitive: cancel task if in progress, reject plan if waiting
287-
if (!currentPath.startsWith("/tasks/")) return;
288-
},
289271
});
290272

291273
// Task detail renderer (used by route)
@@ -307,6 +289,51 @@ function App() {
307289
);
308290
}
309291

292+
function TaskListRoute() {
293+
const waitingSessionId = new URLSearchParams(location.search).get("waitingSession")?.trim() ?? "";
294+
const hasWaitingSessionContext = waitingSessionId.length > 0;
295+
296+
return (
297+
<div style={{ height: "100%", display: "flex", flexDirection: "column", minHeight: 0 }}>
298+
{hasWaitingSessionContext && (
299+
<div
300+
style={{
301+
padding: "var(--space-3) var(--space-6)",
302+
borderBottom: "1px solid var(--color-border)",
303+
backgroundColor: "var(--color-bg-secondary)",
304+
}}
305+
data-testid="waiting-session-context"
306+
>
307+
<div
308+
style={{
309+
fontSize: "var(--font-size-xs)",
310+
color: "var(--color-text-tertiary)",
311+
marginBottom: "var(--space-2)",
312+
}}
313+
>
314+
Waiting session: {waitingSessionId}
315+
</div>
316+
<SessionInputForm
317+
workspaceId={activeWorkspaceId}
318+
sessionId={waitingSessionId}
319+
onSubmitted={() => routerNavigate("/tasks", { replace: true })}
320+
/>
321+
</div>
322+
)}
323+
<div style={{ flex: 1, minHeight: 0 }}>
324+
<TaskList
325+
tasks={tasks}
326+
isLoading={tasksLoading}
327+
onTaskSelect={(taskId) => navigate(`/tasks/${taskId}`)}
328+
onCreateTask={() => setCreateDialogOpen(true)}
329+
selectedIndex={selectedTaskIndex}
330+
onSelectIndex={setSelectedTaskIndex}
331+
/>
332+
</div>
333+
</div>
334+
);
335+
}
336+
310337
// PR detail renderer (used by route)
311338
function PrDetailRoute() {
312339
const prTrackingId = currentPath.replace("/prs/", "");
@@ -356,19 +383,7 @@ function App() {
356383
<div style={contentAreaStyle}>
357384
<ErrorBoundary>
358385
<Routes>
359-
<Route
360-
path="/tasks"
361-
element={
362-
<TaskList
363-
tasks={tasks}
364-
isLoading={tasksLoading}
365-
onTaskSelect={(taskId) => navigate(`/tasks/${taskId}`)}
366-
onCreateTask={() => setCreateDialogOpen(true)}
367-
selectedIndex={selectedTaskIndex}
368-
onSelectIndex={setSelectedTaskIndex}
369-
/>
370-
}
371-
/>
386+
<Route path="/tasks" element={<TaskListRoute />} />
372387
<Route path="/tasks/:taskId" element={<TaskDetailRoute />} />
373388
<Route
374389
path="/inbox"

apps/dexdex/src/features/tasks/plan-decisions.tsx

Lines changed: 80 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
* Plan decision controls for subtasks waiting for plan approval.
33
*/
44

5-
import { type CSSProperties, useRef, useState } from "react";
5+
import { type CSSProperties, useCallback, useEffect, useRef, useState } from "react";
66
import { PlanDecision, SubTaskStatus } from "../../lib/status";
77
import type { SubTask } from "../../lib/mock-data";
88
import { useFocusOnShow } from "../../hooks/use-dialog-accessibility";
@@ -19,6 +19,74 @@ export function PlanDecisions({ subtask, onDecision }: PlanDecisionsProps) {
1919

2020
useFocusOnShow(showReviseInput, revisionInputRef);
2121

22+
const submitRevision = useCallback(() => {
23+
const trimmedRevisionNote = revisionNote.trim();
24+
if (!trimmedRevisionNote) {
25+
return;
26+
}
27+
onDecision(subtask.subTaskId, PlanDecision.REVISE, trimmedRevisionNote);
28+
}, [onDecision, revisionNote, subtask.subTaskId]);
29+
30+
useEffect(() => {
31+
if (subtask.status !== SubTaskStatus.WAITING_FOR_PLAN_APPROVAL) {
32+
return;
33+
}
34+
35+
const handleKeyDown = (event: KeyboardEvent) => {
36+
if (event.isComposing || event.keyCode === 229) {
37+
return;
38+
}
39+
40+
const activeElement = document.activeElement as HTMLElement | null;
41+
const isTextInputFocused =
42+
activeElement?.tagName.toLowerCase() === "input" ||
43+
activeElement?.tagName.toLowerCase() === "textarea" ||
44+
activeElement?.isContentEditable;
45+
if (isTextInputFocused) {
46+
if (
47+
activeElement === revisionInputRef.current &&
48+
(event.metaKey || event.ctrlKey) &&
49+
event.key === "Enter"
50+
) {
51+
event.preventDefault();
52+
submitRevision();
53+
}
54+
return;
55+
}
56+
57+
const isMetaPressed = event.metaKey || event.ctrlKey;
58+
const isAltPressed = event.altKey;
59+
60+
if (!isMetaPressed && !isAltPressed && !event.shiftKey && event.key.toLowerCase() === "a") {
61+
event.preventDefault();
62+
onDecision(subtask.subTaskId, PlanDecision.APPROVE);
63+
return;
64+
}
65+
66+
if (!isMetaPressed && !isAltPressed && !event.shiftKey && event.key.toLowerCase() === "v") {
67+
event.preventDefault();
68+
if (!showReviseInput) {
69+
setShowReviseInput(true);
70+
} else {
71+
revisionInputRef.current?.focus();
72+
}
73+
return;
74+
}
75+
76+
if (!isMetaPressed && !isAltPressed && event.shiftKey && event.key === "X") {
77+
event.preventDefault();
78+
onDecision(subtask.subTaskId, PlanDecision.REJECT);
79+
return;
80+
}
81+
82+
};
83+
84+
document.addEventListener("keydown", handleKeyDown);
85+
return () => {
86+
document.removeEventListener("keydown", handleKeyDown);
87+
};
88+
}, [onDecision, showReviseInput, submitRevision, subtask.status, subtask.subTaskId]);
89+
2290
if (subtask.status !== SubTaskStatus.WAITING_FOR_PLAN_APPROVAL) {
2391
return null;
2492
}
@@ -89,6 +157,15 @@ export function PlanDecisions({ subtask, onDecision }: PlanDecisionsProps) {
89157
placeholder="Describe the revisions needed..."
90158
value={revisionNote}
91159
onChange={(e) => setRevisionNote(e.target.value)}
160+
onKeyDown={(event) => {
161+
if (event.isComposing || event.keyCode === 229) {
162+
return;
163+
}
164+
if ((event.metaKey || event.ctrlKey) && event.key === "Enter") {
165+
event.preventDefault();
166+
submitRevision();
167+
}
168+
}}
92169
data-testid="revision-note-input"
93170
/>
94171
</div>
@@ -126,12 +203,9 @@ export function PlanDecisions({ subtask, onDecision }: PlanDecisionsProps) {
126203
color: "#fff",
127204
borderColor: "var(--color-status-action)",
128205
}}
129-
onClick={() => {
130-
if (revisionNote.trim()) {
131-
onDecision(subtask.subTaskId, PlanDecision.REVISE, revisionNote.trim());
132-
}
133-
}}
206+
onClick={submitRevision}
134207
disabled={!revisionNote.trim()}
208+
data-testid="submit-revision-button"
135209
>
136210
Submit Revision
137211
</button>

apps/dexdex/src/hooks/use-keyboard-shortcuts.ts

Lines changed: 0 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,6 @@ interface KeyboardShortcutsConfig {
1515
onSwitchTabRight?: () => void;
1616
onListDown?: () => void;
1717
onListUp?: () => void;
18-
onApprovePlan?: () => void;
19-
onRevisePlan?: () => void;
20-
onRejectPlan?: () => void;
2118
}
2219

2320
/**
@@ -35,9 +32,6 @@ interface KeyboardShortcutsConfig {
3532
* - C: Create new task (when not in an input)
3633
* - J: Navigate down in task list
3734
* - K: Navigate up in task list
38-
* - A: Approve plan (when viewing plan decision)
39-
* - V: Revise plan
40-
* - Shift+X: Reject plan
4135
*/
4236
export function useKeyboardShortcuts(config: KeyboardShortcutsConfig): void {
4337
const pendingG = useRef(false);
@@ -162,26 +156,6 @@ export function useKeyboardShortcuts(config: KeyboardShortcutsConfig): void {
162156
return;
163157
}
164158

165-
// A: Approve plan (when viewing plan decision)
166-
if (e.key === "a" && !meta && !e.altKey && !e.shiftKey) {
167-
e.preventDefault();
168-
config.onApprovePlan?.();
169-
return;
170-
}
171-
172-
// V: Revise plan
173-
if (e.key === "v" && !meta && !e.altKey && !e.shiftKey) {
174-
e.preventDefault();
175-
config.onRevisePlan?.();
176-
return;
177-
}
178-
179-
// Shift+X: Reject plan
180-
if (e.key === "X" && e.shiftKey && !meta && !e.altKey) {
181-
e.preventDefault();
182-
config.onRejectPlan?.();
183-
return;
184-
}
185159
}
186160

187161
document.addEventListener("keydown", handleKeyDown);

docs/apps-dexdex-desktop-app-foundation.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,9 +55,9 @@ Behavior contracts:
5555
- `Cmd+T`: create new task
5656
- `Cmd+W`: close active tab
5757
- `J` / `K`: navigate up/down in list views
58-
- `A`: approve plan or review item
59-
- `V`: open diff viewer for selected commit
60-
- `Shift+X`: cancel running task
58+
- `A`: approve waiting plan
59+
- `V`: open revise input for waiting plan
60+
- `Shift+X`: reject waiting plan or cancel running task
6161
- Diff viewer component for inline review of commit changes with side-by-side and unified modes.
6262

6363
Data and UX invariants:

docs/apps-dexdex-ui-contract.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,9 @@ Keyboard contracts:
5454
- list navigation shortcuts (`J`, `K`, `Enter`, `Cmd+Enter`)
5555
- multiline submit shortcut (`Cmd+Enter`) with IME-safe behavior
5656
- decision shortcuts for plan mode (`A`, `V`, `Shift+X`)
57+
- `A`: approve waiting plan
58+
- `V`: open revise input for waiting plan
59+
- `Shift+X`: reject waiting plan (or cancel running task when no waiting plan is active)
5760

5861
Accessibility and responsive contracts:
5962
- keyboard-first operation

docs/project-dexdex.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,8 @@ When implementation details differ from documented contracts, follow-up sync wor
7070
- Workspace CRUD operations: `CreateWorkspace`, `UpdateWorkspace`, `DeleteWorkspace`, `SetActiveWorkspace`.
7171
- Task cancellation and subtask management: `CancelUnitTask`, `CancelSubTask`, `CreateSubTask`, `ListSubTaskCommits`, `RetrySubTask`.
7272
- PR tracking and auto-fix control: `TrackPullRequest`, `RunAutoFixNow`, `SetAutoFixPolicy`.
73+
- `RunAutoFixNow` creates and immediately dispatches a remediation SubTask.
74+
- Poller-driven automatic remediation for `auto_fix_enabled` policy remains planned.
7375
- Review assist resolution: `ResolveReviewAssistItem`.
7476
- Badge theme management: `ListBadgeThemes`, `UpsertBadgeTheme`.
7577
- Agent session lifecycle: `ListAgentSessions`, `GetAgentSessionLog`, `StopAgentSession`.

0 commit comments

Comments
 (0)