diff --git a/src/main/ipc/guards.ts b/src/main/ipc/guards.ts index 26f4aa74..9829be8a 100644 --- a/src/main/ipc/guards.ts +++ b/src/main/ipc/guards.ts @@ -12,7 +12,7 @@ const SESSION_ID_PATTERN = /^[a-zA-Z0-9][a-zA-Z0-9._-]{0,127}$/; const SUBAGENT_ID_PATTERN = /^[a-zA-Z0-9][a-zA-Z0-9._-]{0,127}$/; const NOTIFICATION_ID_PATTERN = /^[a-zA-Z0-9][a-zA-Z0-9._-]{0,127}$/; const TRIGGER_ID_PATTERN = /^[a-zA-Z0-9][a-zA-Z0-9._-]{0,127}$/; -const TEAM_NAME_PATTERN = /^[a-zA-Z0-9][a-zA-Z0-9._-]{0,127}$/; +const TEAM_NAME_PATTERN = /^[a-z0-9][a-z0-9-]{0,127}$/; const TASK_ID_PATTERN = /^\d{1,10}$/; const MEMBER_NAME_PATTERN = /^[a-zA-Z0-9][a-zA-Z0-9._-]{0,127}$/; const FROM_FIELD_PATTERN = /^[a-zA-Z0-9][a-zA-Z0-9._-]{0,127}$/; diff --git a/src/renderer/components/dashboard/DashboardView.tsx b/src/renderer/components/dashboard/DashboardView.tsx index 3213440e..0469cc5b 100644 --- a/src/renderer/components/dashboard/DashboardView.tsx +++ b/src/renderer/components/dashboard/DashboardView.tsx @@ -371,10 +371,11 @@ function findMatchingWorktree( } const NewProjectCard = (): React.JSX.Element => { - const { repositoryGroups, fetchRepositoryGroups } = useStore( + const { repositoryGroups, fetchRepositoryGroups, openTeamsTab } = useStore( useShallow((s) => ({ repositoryGroups: s.repositoryGroups, fetchRepositoryGroups: s.fetchRepositoryGroups, + openTeamsTab: s.openTeamsTab, })) ); @@ -396,6 +397,7 @@ const NewProjectCard = (): React.JSX.Element => { const match = findMatchingWorktree(repositoryGroups, selectedPath); if (match) { navigateToMatch(match); + openTeamsTab(); return; } @@ -405,6 +407,7 @@ const NewProjectCard = (): React.JSX.Element => { const matchAfterRefresh = findMatchingWorktree(refreshedGroups, selectedPath); if (matchAfterRefresh) { navigateToMatch(matchAfterRefresh); + openTeamsTab(); return; } @@ -437,6 +440,7 @@ const NewProjectCard = (): React.JSX.Element => { repositoryGroups: [syntheticGroup, ...state.repositoryGroups], })); navigateToMatch({ repoId: encodedId, worktreeId: encodedId }); + openTeamsTab(); } catch (error) { logger.error('Error selecting folder:', error); } diff --git a/src/renderer/components/team/TeamListView.tsx b/src/renderer/components/team/TeamListView.tsx index 0ebb1ba0..b473646f 100644 --- a/src/renderer/components/team/TeamListView.tsx +++ b/src/renderer/components/team/TeamListView.tsx @@ -541,7 +541,7 @@ export const TeamListView = (): React.JSX.Element => { const renderHeader = (): React.JSX.Element => (
-

Teams

+

Select Team

diff --git a/src/renderer/components/team/editor/QuickOpenDialog.tsx b/src/renderer/components/team/editor/QuickOpenDialog.tsx index dc589ae4..e2399f9c 100644 --- a/src/renderer/components/team/editor/QuickOpenDialog.tsx +++ b/src/renderer/components/team/editor/QuickOpenDialog.tsx @@ -5,7 +5,7 @@ * Loads ALL project files via backend API on mount (not limited to expanded dirs). */ -import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; +import { useCallback, useEffect, useRef, useState } from 'react'; import { useStore } from '@renderer/store'; import { Command } from 'cmdk'; diff --git a/src/renderer/components/team/review/ChangeReviewDialog.tsx b/src/renderer/components/team/review/ChangeReviewDialog.tsx index e9ac3484..ec12f5e8 100644 --- a/src/renderer/components/team/review/ChangeReviewDialog.tsx +++ b/src/renderer/components/team/review/ChangeReviewDialog.tsx @@ -1,11 +1,11 @@ import React, { useCallback, useEffect, useMemo, useRef, useState } from 'react'; import { undo } from '@codemirror/commands'; -import { goToNextChunk, rejectChunk } from '@codemirror/merge'; +import { rejectChunk } from '@codemirror/merge'; import { isElectronMode } from '@renderer/api'; import { EditorSelectionMenu } from '@renderer/components/team/editor/EditorSelectionMenu'; import { useContinuousScrollNav } from '@renderer/hooks/useContinuousScrollNav'; -import { isLastChunkInFile, useDiffNavigation } from '@renderer/hooks/useDiffNavigation'; +import { useDiffNavigation } from '@renderer/hooks/useDiffNavigation'; import { useViewedFiles } from '@renderer/hooks/useViewedFiles'; import { cn } from '@renderer/lib/utils'; import { useStore } from '@renderer/store'; @@ -69,6 +69,7 @@ export const ChangeReviewDialog = ({ applying, applyError, setHunkDecision, + clearHunkDecisionByOriginalIndex, setCollapseUnchanged, fetchFileContent, acceptAllFile, @@ -108,6 +109,11 @@ export const ChangeReviewDialog = ({ const scrollContainerRef = useRef(null); // Last focused CM editor — for Cmd+Z outside editor const lastFocusedEditorRef = useRef(null); + // Timestamp of last bulk accept/reject-all operation (for Ctrl/Cmd+Z UX) + const lastBulkActionAtRef = useRef(0); + // Track recent per-hunk actions so Ctrl/Cmd+Z can clear persisted decisions (reopen-safe) + const lastHunkActionAtRef = useRef>({}); + const hunkDecisionUndoStackRef = useRef>({}); // Proxy ref for useDiffNavigation (points to active file's editor) const activeEditorViewRef = useRef(null); @@ -171,6 +177,7 @@ export const ChangeReviewDialog = ({ const handleAcceptAll = useCallback(() => { if (!activeChangeSet) return; pushReviewUndoSnapshot(); + lastBulkActionAtRef.current = Date.now(); for (const file of activeChangeSet.files) { acceptAllFile(file.filePath); } @@ -184,6 +191,7 @@ export const ChangeReviewDialog = ({ const handleRejectAll = useCallback(() => { if (!activeChangeSet) return; pushReviewUndoSnapshot(); + lastBulkActionAtRef.current = Date.now(); for (const file of activeChangeSet.files) { rejectAllFile(file.filePath); } @@ -197,14 +205,18 @@ export const ChangeReviewDialog = ({ // Per-file callbacks for ContinuousScrollView const handleHunkAccepted = useCallback( (filePath: string, hunkIndex: number) => { - setHunkDecision(filePath, hunkIndex, 'accepted'); + const originalIndex = setHunkDecision(filePath, hunkIndex, 'accepted'); + lastHunkActionAtRef.current[filePath] = Date.now(); + (hunkDecisionUndoStackRef.current[filePath] ??= []).push(originalIndex); }, [setHunkDecision] ); const handleHunkRejected = useCallback( (filePath: string, hunkIndex: number) => { - setHunkDecision(filePath, hunkIndex, 'rejected'); + const originalIndex = setHunkDecision(filePath, hunkIndex, 'rejected'); + lastHunkActionAtRef.current[filePath] = Date.now(); + (hunkDecisionUndoStackRef.current[filePath] ??= []).push(originalIndex); if (REVIEW_INSTANT_APPLY) { void applySingleFileDecision(teamName, filePath, taskId, memberName); } @@ -422,7 +434,8 @@ export const ChangeReviewDialog = ({ }); }, [activeChangeSet, initialFilePath, scrollToFile]); - // Clear selection state on close + cleanup timer + // Clear selection state on close + cleanup timer. + // setState here is intentional: reset transient UI state when dialog closes. useEffect(() => { if (!open) { setSelectionInfo(null); @@ -469,32 +482,55 @@ export const ChangeReviewDialog = ({ if (!open) return; const handler = (e: KeyboardEvent): void => { if ((e.metaKey || e.ctrlKey) && e.code === 'KeyZ' && !e.shiftKey) { - // Don't intercept if focus is inside a CM editor — let CM handle its own undo - if (document.activeElement?.closest('.cm-editor')) return; // Don't intercept native undo in input/textarea const tag = document.activeElement?.tagName; if (tag === 'INPUT' || tag === 'TEXTAREA') return; - // Try to undo in the last focused CM editor + // Prefer bulk undo (Accept All / Reject All) shortly after bulk action, + // even if focus is inside a CM editor (focus often remains there after clicking buttons). + const now = Date.now(); + const bulkRecently = now - lastBulkActionAtRef.current < 10_000; + if (bulkRecently && useStore.getState().reviewUndoStack.length > 0) { + e.preventDefault(); + e.stopPropagation(); + handleUndoBulk(); + return; + } + + // If the last action was a hunk keep/undo (accept/reject) and we're undoing immediately, + // we must also clear the persisted decision. Otherwise reopening the dialog will replay it. + if (document.activeElement?.closest('.cm-editor')) { + const lastView = lastFocusedEditorRef.current; + const fp = activeFilePathRef.current; + const stack = fp ? hunkDecisionUndoStackRef.current[fp] : undefined; + const lastAt = fp ? (lastHunkActionAtRef.current[fp] ?? 0) : 0; + const hunkRecently = fp ? now - lastAt < 5_000 : false; + + if (fp && stack && stack.length > 0 && hunkRecently && lastView?.dom.isConnected) { + e.preventDefault(); + e.stopPropagation(); + undo(lastView); + const originalIndex = stack.pop()!; + clearHunkDecisionByOriginalIndex(fp, originalIndex); + return; + } + + // Otherwise, let CM handle its own undo + return; + } + + // Otherwise try to undo in the last focused CM editor const lastView = lastFocusedEditorRef.current; if (lastView?.dom.isConnected) { e.preventDefault(); e.stopPropagation(); undo(lastView); - return; - } - - // Fall back to bulk undo (Accept All / Reject All) - if (useStore.getState().reviewUndoStack.length > 0) { - e.preventDefault(); - e.stopPropagation(); - handleUndoBulk(); } } }; document.addEventListener('keydown', handler, true); return () => document.removeEventListener('keydown', handler, true); - }, [open, handleUndoBulk]); + }, [open, handleUndoBulk, clearHunkDecisionByOriginalIndex]); // Cmd+N IPC listener (forwarded from main process) useEffect(() => { diff --git a/src/renderer/components/team/review/portionCollapse.ts b/src/renderer/components/team/review/portionCollapse.ts index a9dc4976..ec29a460 100644 --- a/src/renderer/components/team/review/portionCollapse.ts +++ b/src/renderer/components/team/review/portionCollapse.ts @@ -281,29 +281,53 @@ const portionCollapseTheme = EditorView.theme({ // CM recognizes it's the same field → keeps accumulated state (expanded regions) → no create() call. // Config is read from portionCollapseConfigFacet (controlled by compartment). -const portionCollapseField = StateField.define({ - create(state: EditorState): DecorationSet { +interface PortionCollapseState { + decorations: DecorationSet; + /** + * Once the user expands everything (i.e. removes the last collapse widget), + * we should NOT re-initialize collapse on harmless transactions like cursor moves. + */ + userExpandedAll: boolean; +} + +const portionCollapseField = StateField.define({ + create(state: EditorState): PortionCollapseState { const cfg = state.facet(portionCollapseConfigFacet); - return buildPortionRanges(state, cfg.margin, cfg.minSize, cfg.portionSize); + return { + decorations: buildPortionRanges(state, cfg.margin, cfg.minSize, cfg.portionSize), + userExpandedAll: false, + }; }, - update(deco: DecorationSet, tr: Transaction): DecorationSet { + update(value: PortionCollapseState, tr: Transaction): PortionCollapseState { const cfg = tr.state.facet(portionCollapseConfigFacet); // 1. Expand effects - let result = deco; + let nextDeco = value.decorations; + let userExpandedAll = value.userExpandedAll; let hasExpandEffect = false; for (const effect of tr.effects) { if (effect.is(expandPortion)) { hasExpandEffect = true; - result = handleExpandPortion(result, effect.value, tr.state, cfg.minSize, cfg.portionSize); + nextDeco = handleExpandPortion( + nextDeco, + effect.value, + tr.state, + cfg.minSize, + cfg.portionSize + ); } if (effect.is(expandAllAtPos)) { hasExpandEffect = true; - result = handleExpandAll(result, effect.value); + nextDeco = handleExpandAll(nextDeco, effect.value); } } - if (hasExpandEffect) return result; + if (hasExpandEffect) { + if (nextDeco === Decoration.none) { + userExpandedAll = true; + } + return { decorations: nextDeco, userExpandedAll }; + } // 2. Accept chunk (updateOriginalDoc) — editor doc unchanged, keep decorations. // Full rebuild here would destroy user's expanded state (Expand All / Expand N). @@ -311,27 +335,47 @@ const portionCollapseField = StateField.define({ // When mirrorEditsAfterResolve adds updateOriginalDoc to a docChanged transaction, // we must NOT short-circuit — fall through to docChanged handler for proper rebuild. if (tr.effects.some((e) => e.is(updateOriginalDoc)) && !tr.docChanged) { - return deco; + return value; } - // 3. Document changed (reject, user edit) → full rebuild + // 3. Document changed (reject, user edit) + // + // Rebuilding from scratch here causes a bad UX: after the user expands (or when a hunk is + // applied) the editor can suddenly re-collapse unchanged regions, hiding the code the user + // was actively looking at. Instead, keep the user's current collapsed/expanded state stable + // by mapping existing decorations through the document changes. if (tr.docChanged) { - return buildPortionRanges(tr.state, cfg.margin, cfg.minSize, cfg.portionSize); + const mapped = value.decorations.map(tr.changes); + // If we previously had no collapse decoration but chunks are now available, initialize once. + // BUT: if the user explicitly expanded everything, never re-collapse automatically. + if (!value.userExpandedAll && value.decorations === Decoration.none) { + const chunks = getChunks(tr.state); + if (chunks) { + return { + decorations: buildPortionRanges(tr.state, cfg.margin, cfg.minSize, cfg.portionSize), + userExpandedAll: false, + }; + } + } + return { decorations: mapped, userExpandedAll: value.userExpandedAll }; } // 4. Lazy init - if (deco === Decoration.none) { + if (!value.userExpandedAll && value.decorations === Decoration.none) { const chunks = getChunks(tr.state); if (chunks) { - return buildPortionRanges(tr.state, cfg.margin, cfg.minSize, cfg.portionSize); + return { + decorations: buildPortionRanges(tr.state, cfg.margin, cfg.minSize, cfg.portionSize), + userExpandedAll: false, + }; } } - return deco; + return value; }, provide(f) { - return EditorView.decorations.from(f); + return EditorView.decorations.from(f, (v) => v.decorations); }, }); diff --git a/src/renderer/hooks/useDiffNavigation.ts b/src/renderer/hooks/useDiffNavigation.ts index 59a5cc9b..a761d858 100644 --- a/src/renderer/hooks/useDiffNavigation.ts +++ b/src/renderer/hooks/useDiffNavigation.ts @@ -264,14 +264,18 @@ export function useDiffNavigation( } }, [selectedFilePath, currentHunkIndex, onHunkRejected]); - // Store refs for stable closure + // Store refs for stable closure (avoids re-registering keydown on every render) const onCloseRef = useRef(onClose); const onSaveFileRef = useRef(onSaveFile); + const onHunkAcceptedRef = useRef(onHunkAccepted); + const selectedFilePathRef = useRef(selectedFilePath); useEffect(() => { onCloseRef.current = onClose; onSaveFileRef.current = onSaveFile; - }, [onClose, onSaveFile]); + onHunkAcceptedRef.current = onHunkAccepted; + selectedFilePathRef.current = selectedFilePath; + }, [onClose, onSaveFile, onHunkAccepted, selectedFilePath]); // Keyboard handler useEffect(() => { @@ -327,11 +331,14 @@ export function useDiffNavigation( event.preventDefault(); const view = getActiveEditorView(editorViewRef, continuousOptionsRef.current); if (view) { - const filePath = getActiveFilePath(selectedFilePath, continuousOptionsRef.current); - if (filePath && onHunkAccepted) { + const filePath = getActiveFilePath( + selectedFilePathRef.current, + continuousOptionsRef.current + ); + if (filePath && onHunkAcceptedRef.current) { const cursorPos = view.state.selection.main.head; const idx = computeChunkIndexAtPos(view.state, cursorPos); - onHunkAccepted(filePath, idx); + onHunkAcceptedRef.current(filePath, idx); } acceptChunk(view); requestAnimationFrame(() => goToNextHunk()); diff --git a/src/renderer/store/slices/changeReviewSlice.ts b/src/renderer/store/slices/changeReviewSlice.ts index 68d6baf2..c854f9e4 100644 --- a/src/renderer/store/slices/changeReviewSlice.ts +++ b/src/renderer/store/slices/changeReviewSlice.ts @@ -94,7 +94,13 @@ export interface ChangeReviewSlice { clearDecisionsFromDisk: (teamName: string, scopeKey: string) => Promise; // Phase 2 actions - setHunkDecision: (filePath: string, hunkIndex: number, decision: HunkDecision) => void; + /** + * Set decision for a hunk at the current (visible) CM index. + * Returns the stable/original hunk index used as the decision key. + */ + setHunkDecision: (filePath: string, hunkIndex: number, decision: HunkDecision) => number; + /** Clear a persisted decision using the stable/original hunk index */ + clearHunkDecisionByOriginalIndex: (filePath: string, originalIndex: number) => void; setFileDecision: (filePath: string, decision: HunkDecision) => void; setFileChunkCount: (filePath: string, count: number) => void; pushReviewUndoSnapshot: () => void; @@ -429,6 +435,17 @@ export const createChangeReviewSlice: StateCreator ({ hunkDecisions: { ...s.hunkDecisions, [key]: decision }, })); + return originalIndex; + }, + + clearHunkDecisionByOriginalIndex: (filePath: string, originalIndex: number) => { + const key = `${filePath}:${originalIndex}`; + set((s) => { + if (!(key in s.hunkDecisions)) return s; + const next = { ...s.hunkDecisions }; + delete next[key]; + return { hunkDecisions: next }; + }); }, setFileDecision: (filePath: string, decision: HunkDecision) => { @@ -727,14 +744,14 @@ export const createChangeReviewSlice: StateCreator(operation: string, fn: () => Promise): Promise { try { return await fn(); } catch (error) { const message = error instanceof Error ? error.message : String(error); - logger.error(`[${operation}] ${message}`); + if (EXPECTED_IPC_SIGNALS.some((sig) => message.includes(sig))) { + logger.debug(`[${operation}] ${message}`); + } else { + logger.error(`[${operation}] ${message}`); + } throw new IpcError(operation, message, error); } }