From fe9298a7add87dd5249b570fb04dc1fc74f84c86 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Flor=C3=AAncio?= Date: Thu, 19 Feb 2026 18:05:52 +0000 Subject: [PATCH] feat: improve ux (#254) --- apps/backend/internal/worktree/manager.go | 78 +++++-- .../backend/internal/worktree/manager_test.go | 171 ++++++++++++++ apps/web/components/diff/diff-viewer.tsx | 4 +- apps/web/components/diff/use-diff-options.tsx | 3 + .../codemirror/codemirror-code-editor.tsx | 196 +++++++++++----- .../codemirror/use-codemirror-editor-state.ts | 8 +- .../editors/monaco/monaco-code-block.tsx | 3 +- .../editors/monaco/monaco-code-editor.tsx | 28 ++- .../editors/monaco/monaco-editor-toolbar.tsx | 157 +++++++++---- .../editors/monaco/use-monaco-editor-lsp.ts | 106 ++++++++- apps/web/components/review/review-dialog.tsx | 11 +- .../components/review/review-diff-list.tsx | 9 +- .../task/changes-panel-file-row.tsx | 6 +- .../task/changes-panel-timeline.tsx | 10 +- apps/web/components/task/changes-panel.tsx | 9 +- .../task/dockview-desktop-layout.tsx | 25 +- .../components/task/file-browser-parts.tsx | 93 +++++++- apps/web/components/task/file-browser.tsx | 103 +++++++-- .../components/task/file-editor-content.tsx | 4 + .../web/components/task/file-editor-panel.tsx | 13 +- .../web/components/task/task-center-panel.tsx | 1 + .../components/task/task-changes-panel.tsx | 50 +++- apps/web/hooks/use-file-editors.ts | 213 ++++++++++++++++-- apps/web/lib/state/dockview-store.ts | 26 ++- 24 files changed, 1110 insertions(+), 217 deletions(-) diff --git a/apps/backend/internal/worktree/manager.go b/apps/backend/internal/worktree/manager.go index ffa952c21e..e80ad636bc 100644 --- a/apps/backend/internal/worktree/manager.go +++ b/apps/backend/internal/worktree/manager.go @@ -2,6 +2,7 @@ package worktree import ( "context" + "errors" "fmt" "os" "os/exec" @@ -16,6 +17,11 @@ import ( "github.com/kandev/kandev/internal/common/logger" ) +const ( + defaultGitFetchTimeout = 8 * time.Second + defaultGitPullTimeout = 8 * time.Second +) + // repoLockEntry tracks a repository lock and its reference count. type repoLockEntry struct { mu *sync.Mutex @@ -35,6 +41,10 @@ type Manager struct { // Optional dependencies for script execution repoProvider RepositoryProvider scriptMsgHandler ScriptMessageHandler + + // Timeouts for best-effort remote sync before creating a worktree. + fetchTimeout time.Duration + pullTimeout time.Duration } // ScriptMessageHandler provides script execution and message streaming. @@ -83,11 +93,13 @@ func NewManager(cfg Config, store Store, log *logger.Logger) (*Manager, error) { } return &Manager{ - config: cfg, - logger: log.WithFields(zap.String("component", "worktree-manager")), - store: store, - worktrees: make(map[string]*Worktree), - repoLocks: make(map[string]*repoLockEntry), + config: cfg, + logger: log.WithFields(zap.String("component", "worktree-manager")), + store: store, + worktrees: make(map[string]*Worktree), + repoLocks: make(map[string]*repoLockEntry), + fetchTimeout: defaultGitFetchTimeout, + pullTimeout: defaultGitPullTimeout, }, nil } @@ -724,6 +736,40 @@ func (m *Manager) currentBranch(repoPath string) string { return strings.TrimSpace(string(output)) } +func (m *Manager) newNonInteractiveGitCmd(ctx context.Context, repoPath string, args ...string) *exec.Cmd { + cmd := exec.CommandContext(ctx, "git", args...) + cmd.Dir = repoPath + cmd.Env = append(os.Environ(), + "GIT_TERMINAL_PROMPT=0", + "GCM_INTERACTIVE=Never", + "GIT_ASKPASS=echo", + "SSH_ASKPASS=/bin/false", + "GIT_SSH_COMMAND=ssh -oBatchMode=yes", + ) + // After the context cancels and the process is killed, child processes + // (e.g. credential helpers) may still hold stdout/stderr pipes open. + // WaitDelay bounds how long CombinedOutput waits for those pipes to close. + cmd.WaitDelay = 500 * time.Millisecond + return cmd +} + +func classifyGitFallbackReason(cmdErr error, cmdOutput string, ctxErr error) string { + if errors.Is(ctxErr, context.DeadlineExceeded) || errors.Is(cmdErr, context.DeadlineExceeded) { + return "timeout" + } + + out := strings.ToLower(cmdOutput) + if strings.Contains(out, "authentication failed") || + strings.Contains(out, "terminal prompts disabled") || + strings.Contains(out, "could not read username") || + strings.Contains(out, "username for 'https://") || + strings.Contains(out, "askpass") { + return "non_interactive_auth_failed" + } + + return "git_command_failed" +} + // pullBaseBranch fetches the latest changes from origin and returns the best ref to use // for creating a new worktree. The function handles three scenarios: // @@ -737,19 +783,20 @@ func (m *Manager) pullBaseBranch(repoPath, baseBranch string) string { localBranch := strings.TrimPrefix(baseBranch, "origin/") isRemoteRef := localBranch != baseBranch - // Fetch the branch from origin (30s timeout for fetch + potential pull) - fetchCtx, cancel := context.WithTimeout(context.Background(), 30*time.Second) - defer cancel() + // Fetch branch from origin in non-interactive mode. + fetchCtx, cancelFetch := context.WithTimeout(context.Background(), m.fetchTimeout) + defer cancelFetch() fetchArgs := []string{"fetch", "origin"} if localBranch != "" { fetchArgs = append(fetchArgs, localBranch) } - fetchCmd := exec.CommandContext(fetchCtx, "git", fetchArgs...) - fetchCmd.Dir = repoPath + fetchCmd := m.newNonInteractiveGitCmd(fetchCtx, repoPath, fetchArgs...) if output, err := fetchCmd.CombinedOutput(); err != nil { - m.logger.Error("git fetch failed before worktree creation", + m.logger.Warn("git fetch failed before worktree creation; continuing with fallback ref", zap.String("branch", baseBranch), + zap.String("reason", classifyGitFallbackReason(err, string(output), fetchCtx.Err())), + zap.String("fallback_ref", baseBranch), zap.String("output", string(output)), zap.Error(err)) return baseBranch @@ -766,11 +813,14 @@ func (m *Manager) pullBaseBranch(repoPath, baseBranch string) string { if currentBranch == baseBranch { // We're on the target branch - try to pull (fast-forward only) - pullCmd := exec.CommandContext(fetchCtx, "git", "pull", "--ff-only", "origin", baseBranch) - pullCmd.Dir = repoPath + pullCtx, cancelPull := context.WithTimeout(context.Background(), m.pullTimeout) + defer cancelPull() + + pullCmd := m.newNonInteractiveGitCmd(pullCtx, repoPath, "pull", "--ff-only", "origin", baseBranch) if output, err := pullCmd.CombinedOutput(); err != nil { - m.logger.Warn("git pull failed before worktree creation, falling back to remote ref", + m.logger.Warn("git pull failed before worktree creation; continuing with remote ref", zap.String("branch", baseBranch), + zap.String("reason", classifyGitFallbackReason(err, string(output), pullCtx.Err())), zap.String("remote_ref", remoteRef), zap.String("output", string(output)), zap.Error(err)) diff --git a/apps/backend/internal/worktree/manager_test.go b/apps/backend/internal/worktree/manager_test.go index 10ef70c23a..8e19bf0c85 100644 --- a/apps/backend/internal/worktree/manager_test.go +++ b/apps/backend/internal/worktree/manager_test.go @@ -5,7 +5,9 @@ import ( "os" "path/filepath" "regexp" + "strings" "testing" + "time" "github.com/kandev/kandev/internal/common/logger" ) @@ -596,3 +598,172 @@ func TestRepoLocks_ReleaseNonexistent(t *testing.T) { t.Errorf("expected 0 locks in map, got %d", count) } } + +func writeFakeGitScript(t *testing.T, scriptBody string) string { + t.Helper() + + scriptDir := t.TempDir() + scriptPath := filepath.Join(scriptDir, "git") + content := "#!/bin/sh\nset -eu\n\n" + scriptBody + "\n" + if err := os.WriteFile(scriptPath, []byte(content), 0755); err != nil { + t.Fatalf("failed to write fake git script: %v", err) + } + return scriptDir +} + +func TestPullBaseBranch_UsesNonInteractiveGitEnv(t *testing.T) { + logPath := filepath.Join(t.TempDir(), "git-env.log") + scriptDir := writeFakeGitScript(t, ` +case "${1:-}" in + fetch) + printf "%s|%s|%s|%s|%s" \ + "${GIT_TERMINAL_PROMPT:-}" \ + "${GCM_INTERACTIVE:-}" \ + "${GIT_ASKPASS:-}" \ + "${SSH_ASKPASS:-}" \ + "${GIT_SSH_COMMAND:-}" > "${KD_GIT_ENV_LOG:?}" + exit 0 + ;; + rev-parse) + if [ "${2:-}" = "--abbrev-ref" ]; then + echo "master" + fi + exit 0 + ;; + *) + exit 0 + ;; +esac +`) + + t.Setenv("KD_GIT_ENV_LOG", logPath) + t.Setenv("PATH", scriptDir+string(os.PathListSeparator)+os.Getenv("PATH")) + + cfg := newTestConfig(t) + log := newTestLogger() + store := newMockStore() + mgr, err := NewManager(cfg, store, log) + if err != nil { + t.Fatalf("NewManager failed: %v", err) + } + + repoPath := t.TempDir() + ref := mgr.pullBaseBranch(repoPath, "origin/master") + if ref != "origin/master" { + t.Fatalf("pullBaseBranch() ref = %q, want %q", ref, "origin/master") + } + + envBytes, err := os.ReadFile(logPath) + if err != nil { + t.Fatalf("failed reading fake git env log: %v", err) + } + + got := string(envBytes) + want := "0|Never|echo|/bin/false|ssh -oBatchMode=yes" + if got != want { + t.Fatalf("fake git env = %q, want %q", got, want) + } +} + +func TestPullBaseBranch_FetchTimeoutFallsBackQuickly(t *testing.T) { + scriptDir := writeFakeGitScript(t, ` +case "${1:-}" in + fetch) + sleep 2 + exit 0 + ;; + rev-parse) + if [ "${2:-}" = "--abbrev-ref" ]; then + echo "master" + fi + exit 0 + ;; + *) + exit 0 + ;; +esac +`) + + t.Setenv("PATH", scriptDir+string(os.PathListSeparator)+os.Getenv("PATH")) + + cfg := newTestConfig(t) + log := newTestLogger() + store := newMockStore() + mgr, err := NewManager(cfg, store, log) + if err != nil { + t.Fatalf("NewManager failed: %v", err) + } + mgr.fetchTimeout = 100 * time.Millisecond + + repoPath := t.TempDir() + start := time.Now() + ref := mgr.pullBaseBranch(repoPath, "master") + elapsed := time.Since(start) + + if ref != "master" { + t.Fatalf("pullBaseBranch() ref = %q, want %q", ref, "master") + } + // Allow CI scheduling variance while still asserting we timed out + // well before the fake 2s fetch command completes. + if elapsed > 1500*time.Millisecond { + t.Fatalf("pullBaseBranch() took too long: %v", elapsed) + } +} + +func TestPullBaseBranch_PullFailureFallsBackToRemoteRef(t *testing.T) { + scriptDir := writeFakeGitScript(t, ` +case "${1:-}" in + fetch) + exit 0 + ;; + pull) + echo "Authentication failed" 1>&2 + exit 1 + ;; + rev-parse) + if [ "${2:-}" = "--abbrev-ref" ]; then + echo "master" + exit 0 + fi + if [ "${2:-}" = "--verify" ]; then + exit 0 + fi + exit 0 + ;; + *) + exit 0 + ;; +esac +`) + + t.Setenv("PATH", scriptDir+string(os.PathListSeparator)+os.Getenv("PATH")) + + cfg := newTestConfig(t) + log := newTestLogger() + store := newMockStore() + mgr, err := NewManager(cfg, store, log) + if err != nil { + t.Fatalf("NewManager failed: %v", err) + } + mgr.pullTimeout = 300 * time.Millisecond + + repoPath := t.TempDir() + ref := mgr.pullBaseBranch(repoPath, "master") + if ref != "origin/master" { + t.Fatalf("pullBaseBranch() ref = %q, want %q", ref, "origin/master") + } +} + +func TestClassifyGitFallbackReason_AuthPrompt(t *testing.T) { + reason := classifyGitFallbackReason(nil, "fatal: could not read Username for 'https://github.com'", nil) + if reason != "non_interactive_auth_failed" { + t.Fatalf("classifyGitFallbackReason() = %q, want %q", reason, "non_interactive_auth_failed") + } +} + +func TestClassifyGitFallbackReason_Timeout(t *testing.T) { + reason := classifyGitFallbackReason(context.DeadlineExceeded, "", context.DeadlineExceeded) + if !strings.EqualFold(reason, "timeout") { + t.Fatalf("classifyGitFallbackReason() = %q, want %q", reason, "timeout") + } +} diff --git a/apps/web/components/diff/diff-viewer.tsx b/apps/web/components/diff/diff-viewer.tsx index 5535246b96..7bc91b2bac 100644 --- a/apps/web/components/diff/diff-viewer.tsx +++ b/apps/web/components/diff/diff-viewer.tsx @@ -114,7 +114,7 @@ export const DiffViewer = memo(function DiffViewer({ return (
No diff available @@ -134,7 +134,7 @@ export const DiffViewer = memo(function DiffViewer({ renderHoverUtility={renderHoverUtility} className={cn( 'rounded-md border border-border/50', - compact && 'text-xs' + 'text-xs' )} />
diff --git a/apps/web/components/diff/use-diff-options.tsx b/apps/web/components/diff/use-diff-options.tsx index 51ab665349..644f5cb0b5 100644 --- a/apps/web/components/diff/use-diff-options.tsx +++ b/apps/web/components/diff/use-diff-options.tsx @@ -9,6 +9,7 @@ import type { AnnotationMetadata } from './use-diff-annotation-renderer'; /** CSS overrides for the Pierre diff viewer, injected via unsafeCSS. */ const DIFF_UNSAFE_CSS = ` + [data-diffs], pre[data-diffs] { background-color: var(--background) !important; --diffs-bg: var(--background) !important; @@ -22,6 +23,8 @@ const DIFF_UNSAFE_CSS = ` --diffs-deletion-color-override: rgb(var(--git-deletion)) !important; --diffs-font-size: ${FONT.size}px !important; --diffs-font-family: ${FONT.mono} !important; + font-size: ${FONT.size}px !important; + font-family: ${FONT.mono} !important; } [data-change-icon] { width: 12px !important; diff --git a/apps/web/components/editors/codemirror/codemirror-code-editor.tsx b/apps/web/components/editors/codemirror/codemirror-code-editor.tsx index a53060c76d..b6ca5ef9cc 100644 --- a/apps/web/components/editors/codemirror/codemirror-code-editor.tsx +++ b/apps/web/components/editors/codemirror/codemirror-code-editor.tsx @@ -1,9 +1,9 @@ 'use client'; -import { useState, useRef } from 'react'; +import { useRef } from 'react'; import CodeMirror, { type ReactCodeMirrorRef } from '@uiw/react-codemirror'; import { Button } from '@kandev/ui/button'; -import { IconDeviceFloppy, IconLoader2, IconTrash, IconTextWrap, IconTextWrapDisabled, IconMessagePlus } from '@tabler/icons-react'; +import { IconDeviceFloppy, IconLoader2, IconTrash, IconTextWrap, IconTextWrapDisabled, IconMessagePlus, IconRefresh } from '@tabler/icons-react'; import { Tooltip, TooltipContent, TooltipTrigger } from '@kandev/ui/tooltip'; import { formatDiffStats } from '@/lib/utils/file-diff'; import { toRelativePath } from '@/lib/utils'; @@ -11,32 +11,151 @@ import { vscodeDark } from '@uiw/codemirror-theme-vscode'; import { EditorCommentPopover } from '@/components/task/editor-comment-popover'; import { CommentViewPopover } from '@/components/task/comment-view-popover'; import { PanelHeaderBarSplit } from '@/components/task/panel-primitives'; -import { useDockviewStore } from '@/lib/state/dockview-store'; import { useCodeMirrorEditorState } from './use-codemirror-editor-state'; +const SAVE_SHORTCUT = typeof navigator !== 'undefined' && navigator.platform.includes('Mac') ? '\u2318' : 'Ctrl'; + type FileEditorContentProps = { path: string; + content: string; originalContent: string; isDirty: boolean; + hasRemoteUpdate?: boolean; + vcsDiff?: string; isSaving: boolean; sessionId?: string; worktreePath?: string; enableComments?: boolean; onChange: (newContent: string) => void; onSave: () => void; + onReloadFromAgent?: () => void; onDelete?: () => void; }; +function CodeMirrorCommentBadge({ + enableComments, + sessionId, + commentCount, +}: { + enableComments: boolean; + sessionId?: string; + commentCount: number; +}) { + if (!enableComments || !sessionId || commentCount <= 0) return null; + + return ( +
+ + {commentCount} comment{commentCount > 1 ? 's' : ''} +
+ ); +} + +function CodeMirrorWrapButton({ wrapEnabled, onToggleWrap }: { wrapEnabled: boolean; onToggleWrap: () => void }) { + return ( + + + + + {wrapEnabled ? 'Disable word wrap' : 'Enable word wrap'} + + ); +} + +function CodeMirrorReloadButton({ + hasRemoteUpdate, + onReloadFromAgent, +}: { + hasRemoteUpdate?: boolean; + onReloadFromAgent?: () => void; +}) { + if (!hasRemoteUpdate || !onReloadFromAgent) return null; + + return ( + + + + + Apply latest agent changes to this file + + ); +} + +function CodeMirrorDeleteButton({ onDelete }: { onDelete?: () => void }) { + if (!onDelete) return null; + + return ( + + + + + Delete file + + ); +} + +function CodeMirrorSaveButton({ + isDirty, + isSaving, + onSave, +}: { + isDirty: boolean; + isSaving: boolean; + onSave: () => void; +}) { + return ( + + ); +} + /** Toolbar for the CodeMirror code editor. */ function CodeMirrorToolbar({ path, worktreePath, isDirty, isSaving, diffStats, wrapEnabled, - enableComments, sessionId, commentCount, - onToggleWrap, onSave, onDelete, + enableComments, sessionId, commentCount, hasRemoteUpdate, + onToggleWrap, onSave, onReloadFromAgent, onDelete, }: { path: string; worktreePath?: string; isDirty: boolean; isSaving: boolean; diffStats: { additions: number; deletions: number } | null; wrapEnabled: boolean; enableComments: boolean; sessionId?: string; - commentCount: number; onToggleWrap: () => void; onSave: () => void; + commentCount: number; hasRemoteUpdate?: boolean; onToggleWrap: () => void; onSave: () => void; + onReloadFromAgent?: () => void; onDelete?: () => void; }) { return ( @@ -53,47 +172,15 @@ function CodeMirrorToolbar({ } right={
- {enableComments && sessionId && commentCount > 0 && ( -
- - {commentCount} comment{commentCount > 1 ? 's' : ''} -
- )} - - - - - {wrapEnabled ? 'Disable word wrap' : 'Enable word wrap'} - - {onDelete && ( - - - - - Delete file - - )} - + + + + +
} /> @@ -101,18 +188,15 @@ function CodeMirrorToolbar({ } export function CodeMirrorCodeEditor({ - path, originalContent, isDirty, isSaving, + path, content, originalContent, isDirty, hasRemoteUpdate = false, isSaving, sessionId, worktreePath, enableComments = false, - onChange, onSave, onDelete, + onChange, onSave, onReloadFromAgent, onDelete, }: FileEditorContentProps) { - const [initialContent] = useState( - () => useDockviewStore.getState().openFiles.get(path)?.content ?? '' - ); const wrapperRef = useRef(null); const editorRef = useRef(null); const state = useCodeMirrorEditorState({ - path, originalContent, initialContent, isDirty, isSaving, + path, content, originalContent, isDirty, isSaving, sessionId, enableComments, onChange, onSave, wrapperRef, editorRef, }); @@ -122,15 +206,15 @@ export function CodeMirrorCodeEditor({ path={path} worktreePath={worktreePath} isDirty={isDirty} isSaving={isSaving} diffStats={state.diffStats} wrapEnabled={state.wrapEnabled} enableComments={enableComments} sessionId={sessionId} - commentCount={state.comments.length} + commentCount={state.comments.length} hasRemoteUpdate={hasRemoteUpdate} onToggleWrap={() => state.setWrapEnabled(!state.wrapEnabled)} - onSave={onSave} onDelete={onDelete} + onSave={onSave} onReloadFromAgent={onReloadFromAgent} onDelete={onDelete} />
(null); const [commentView, setCommentView] = useState(null); const mousePositionRef = useRef<{ x: number; y: number }>({ x: 0, y: 0 }); - const contentRef = useRef(opts.initialContent); + const contentRef = useRef(opts.content); const { toast } = useToast(); const { setOpen: setCommandPanelOpen } = useCommandPanelOpen(); @@ -277,6 +277,10 @@ export function useCodeMirrorEditorState(opts: UseCodeMirrorEditorStateOpts) { }, [isDirty, originalContent]); useEffect(() => { computeDiffStats(); }, [computeDiffStats]); + useEffect(() => { + contentRef.current = opts.content; + }, [opts.content]); + // Cmd+I to open comment popover useEffect(() => { const wrapper = wrapperRef.current; diff --git a/apps/web/components/editors/monaco/monaco-code-block.tsx b/apps/web/components/editors/monaco/monaco-code-block.tsx index a280d53d96..513e1132cd 100644 --- a/apps/web/components/editors/monaco/monaco-code-block.tsx +++ b/apps/web/components/editors/monaco/monaco-code-block.tsx @@ -65,13 +65,14 @@ export function MonacoCodeBlock({ children, className }: MonacoCodeBlockProps) { glyphMargin: false, folding: false, renderLineHighlight: 'none', - scrollbar: { vertical: 'hidden', horizontal: 'hidden' }, + scrollbar: { vertical: 'hidden', horizontal: 'hidden', alwaysConsumeMouseWheel: false }, overviewRulerLanes: 0, hideCursorInOverviewRuler: true, overviewRulerBorder: false, automaticLayout: true, domReadOnly: true, contextmenu: false, + mouseWheelZoom: false, padding: { top: 4, bottom: 4 }, }} className="rounded-md overflow-hidden text-xs" diff --git a/apps/web/components/editors/monaco/monaco-code-editor.tsx b/apps/web/components/editors/monaco/monaco-code-editor.tsx index f3efae2523..62bec528d5 100644 --- a/apps/web/components/editors/monaco/monaco-code-editor.tsx +++ b/apps/web/components/editors/monaco/monaco-code-editor.tsx @@ -1,6 +1,6 @@ 'use client'; -import { useState, useRef } from 'react'; +import { useEffect, useRef } from 'react'; import Editor from '@monaco-editor/react'; import { useTheme } from 'next-themes'; import { Button } from '@kandev/ui/button'; @@ -9,7 +9,6 @@ import { getMonacoLanguage } from '@/lib/editor/language-map'; import { EDITOR_FONT_FAMILY, EDITOR_FONT_SIZE } from '@/lib/theme/editor-theme'; import { CommentForm } from '@/components/diff/comment-form'; import { CommentDisplay } from '@/components/diff/comment-display'; -import { useDockviewStore } from '@/lib/state/dockview-store'; import { useEditorViewZoneComments } from '@/hooks/use-editor-view-zone-comments'; import { MonacoEditorToolbar } from './monaco-editor-toolbar'; import { useMonacoEditorComments } from './use-monaco-editor-state'; @@ -20,14 +19,18 @@ initMonacoThemes(); type MonacoCodeEditorProps = { path: string; + content: string; originalContent: string; isDirty: boolean; + hasRemoteUpdate?: boolean; + vcsDiff?: string; isSaving: boolean; sessionId?: string; worktreePath?: string; enableComments?: boolean; onChange: (newContent: string) => void; onSave: () => void; + onReloadFromAgent?: () => void; onDelete?: () => void; }; @@ -55,15 +58,12 @@ const EDITOR_OPTIONS = { }; export function MonacoCodeEditor({ - path, originalContent, isDirty, isSaving, + path, content, originalContent, isDirty, hasRemoteUpdate = false, vcsDiff, isSaving, sessionId, worktreePath, enableComments = false, - onChange, onSave, onDelete, + onChange, onSave, onReloadFromAgent, onDelete, }: MonacoCodeEditorProps) { const { resolvedTheme } = useTheme(); - const [initialContent] = useState( - () => useDockviewStore.getState().openFiles.get(path)?.content ?? '' - ); - const contentRef = useRef(initialContent); + const contentRef = useRef(content); const wrapperRef = useRef(null); const language = getMonacoLanguage(path); @@ -77,9 +77,14 @@ export function MonacoCodeEditor({ const { diffStats } = useMonacoDiffDecorations({ originalContent, isDirty, showDiffIndicators: state.showDiffIndicators, + vcsDiff, editorReady: state.editorInstance, contentRef, editorRef: state.editorRef, diffDecorationsRef: state.diffDecorationsRef, }); + useEffect(() => { + contentRef.current = content; + }, [content]); + useEditorViewZoneComments(state.editorInstance, [state.comments, state.formZoneRange, state.editingCommentId], (addZone) => { for (const comment of state.comments) { const isEditing = state.editingCommentId === comment.id; @@ -101,16 +106,19 @@ export function MonacoCodeEditor({ path={path} worktreePath={worktreePath} isDirty={isDirty} isSaving={isSaving} diffStats={diffStats} wrapEnabled={state.wrapEnabled} showDiffIndicators={state.showDiffIndicators} enableComments={enableComments} sessionId={sessionId} commentCount={state.comments.length} + hasRemoteUpdate={hasRemoteUpdate} hasVcsDiff={Boolean(vcsDiff)} lspStatus={lsp.lspStatus} lspLanguage={lsp.lspLanguage} onToggleLsp={lsp.toggleLsp} onToggleWrap={() => state.setWrapEnabled(!state.wrapEnabled)} onToggleDiffIndicators={() => state.setShowDiffIndicators(!state.showDiffIndicators)} - onSave={onSave} onDelete={onDelete} + onSave={onSave} + onReloadFromAgent={onReloadFromAgent} + onDelete={onDelete} />
+ + {commentCount} comment{commentCount > 1 ? 's' : ''} +
+ ); +} + +function DiffIndicatorsButton({ + isVisible, + onToggle, +}: { + isVisible: boolean; + onToggle: () => void; +}) { + const buttonClass = `h-8 w-8 p-0 cursor-pointer ${isVisible ? 'text-foreground' : 'text-muted-foreground'}`; + return ( + + + + + {isVisible ? 'Hide diff indicators' : 'Show diff indicators'} + + ); +} + +function WrapButton({ wrapEnabled, onToggleWrap }: { wrapEnabled: boolean; onToggleWrap: () => void }) { + const wrapClass = `h-8 w-8 p-0 cursor-pointer ${wrapEnabled ? 'text-foreground' : 'text-muted-foreground'}`; + const wrapIcon = wrapEnabled ? : ; + const wrapLabel = wrapEnabled ? 'Disable word wrap' : 'Enable word wrap'; + + return ( + + + + + {wrapLabel} + + ); +} + +function ReloadFromAgentButton({ + hasRemoteUpdate, + onReloadFromAgent, +}: { + hasRemoteUpdate?: boolean; + onReloadFromAgent?: () => void; +}) { + if (!hasRemoteUpdate || !onReloadFromAgent) return null; + + return ( + + + + + Apply latest agent changes to this file + + ); +} + +function DeleteButton({ onDelete }: { onDelete?: () => void }) { + if (!onDelete) return null; + + return ( + + + + + Delete file + + ); +} + interface MonacoEditorToolbarProps { path: string; worktreePath?: string; @@ -59,68 +162,38 @@ interface MonacoEditorToolbarProps { enableComments: boolean; sessionId?: string; commentCount: number; + hasRemoteUpdate?: boolean; + hasVcsDiff?: boolean; lspStatus: LspStatus; lspLanguage: string | null; onToggleLsp: () => void; onToggleWrap: () => void; onToggleDiffIndicators: () => void; onSave: () => void; + onReloadFromAgent?: () => void; onDelete?: () => void; } export function MonacoEditorToolbar({ path, worktreePath, isDirty, isSaving, diffStats, wrapEnabled, showDiffIndicators, enableComments, sessionId, - commentCount, lspStatus, lspLanguage, - onToggleLsp, onToggleWrap, onToggleDiffIndicators, onSave, onDelete, + commentCount, hasRemoteUpdate = false, hasVcsDiff = false, lspStatus, lspLanguage, + onToggleLsp, onToggleWrap, onToggleDiffIndicators, onSave, onReloadFromAgent, onDelete, }: MonacoEditorToolbarProps) { - const wrapIcon = wrapEnabled ? : ; - const wrapLabel = wrapEnabled ? 'Disable word wrap' : 'Enable word wrap'; - const wrapClass = `h-8 w-8 p-0 cursor-pointer ${wrapEnabled ? 'text-foreground' : 'text-muted-foreground'}`; - const diffClass = `h-8 w-8 p-0 cursor-pointer ${showDiffIndicators ? 'text-foreground' : 'text-muted-foreground'}`; - return ( } right={
- {enableComments && sessionId && commentCount > 0 && ( -
- - {commentCount} comment{commentCount > 1 ? 's' : ''} -
- )} + - {isDirty && ( - - - - - {showDiffIndicators ? 'Hide diff indicators' : 'Show diff indicators'} - + {(isDirty || hasVcsDiff) && ( + )} - - - - - {wrapLabel} - + + - {onDelete && ( - - - - - Delete file - - )} +
} diff --git a/apps/web/components/editors/monaco/use-monaco-editor-lsp.ts b/apps/web/components/editors/monaco/use-monaco-editor-lsp.ts index b6976a621d..e60c77a1d1 100644 --- a/apps/web/components/editors/monaco/use-monaco-editor-lsp.ts +++ b/apps/web/components/editors/monaco/use-monaco-editor-lsp.ts @@ -59,6 +59,87 @@ export function computeDiffGutterDecorations( return decorations; } +function computePatchGutterDecorations(diffText: string): monacoEditor.IModelDeltaDecoration[] { + const decorations: monacoEditor.IModelDeltaDecoration[] = []; + const lines = diffText.split('\n'); + let currentLine = 1; + let removedCount = 0; + let addedCount = 0; + + const flushRun = () => { + if (removedCount === 0 && addedCount === 0) return; + if (removedCount > 0 && addedCount > 0) { + for (let i = 0; i < addedCount; i++) { + decorations.push({ + range: { + startLineNumber: currentLine + i, + startColumn: 1, + endLineNumber: currentLine + i, + endColumn: 1, + }, + options: { isWholeLine: true, linesDecorationsClassName: 'monaco-diff-modified-gutter' }, + }); + } + currentLine += addedCount; + } else if (addedCount > 0) { + for (let i = 0; i < addedCount; i++) { + decorations.push({ + range: { + startLineNumber: currentLine + i, + startColumn: 1, + endLineNumber: currentLine + i, + endColumn: 1, + }, + options: { isWholeLine: true, linesDecorationsClassName: 'monaco-diff-added-gutter' }, + }); + } + currentLine += addedCount; + } else { + const indicatorLine = Math.max(1, currentLine - 1); + decorations.push({ + range: { + startLineNumber: indicatorLine, + startColumn: 1, + endLineNumber: indicatorLine, + endColumn: 1, + }, + options: { isWholeLine: true, linesDecorationsClassName: 'monaco-diff-deleted-gutter' }, + }); + } + removedCount = 0; + addedCount = 0; + }; + + for (const line of lines) { + if (line.startsWith('@@')) { + flushRun(); + const match = line.match(/^@@\s*-\d+(?:,\d+)?\s+\+(\d+)(?:,\d+)?\s*@@/); + if (match) currentLine = Number.parseInt(match[1], 10); + continue; + } + if (line.startsWith('---') || line.startsWith('+++') || line.startsWith('diff --git') || line.startsWith('index ')) { + continue; + } + if (line.startsWith('-')) { + removedCount++; + continue; + } + if (line.startsWith('+')) { + addedCount++; + continue; + } + if (line.startsWith(' ')) { + flushRun(); + currentLine++; + continue; + } + flushRun(); + } + + flushRun(); + return decorations; +} + // --------------------------------------------------------------------------- // useMonacoEditorLsp — LSP integration and toasts // --------------------------------------------------------------------------- @@ -130,27 +211,40 @@ type UseMonacoDiffDecorationsOpts = { originalContent: string; isDirty: boolean; showDiffIndicators: boolean; + vcsDiff?: string; + editorReady?: monacoEditor.IStandaloneCodeEditor | null; contentRef: RefObject; editorRef: RefObject; diffDecorationsRef: RefObject; }; export function useMonacoDiffDecorations(opts: UseMonacoDiffDecorationsOpts) { - const { originalContent, isDirty, showDiffIndicators, contentRef, editorRef, diffDecorationsRef } = opts; + const { originalContent, isDirty, showDiffIndicators, vcsDiff, editorReady, contentRef, editorRef, diffDecorationsRef } = opts; const updateDiffDecorations = useCallback(() => { if (!diffDecorationsRef.current || !editorRef.current) return; - if (!showDiffIndicators || !isDirty || !originalContent) { + if (!showDiffIndicators) { diffDecorationsRef.current.set([]); return; } - diffDecorationsRef.current.set(computeDiffGutterDecorations(originalContent, contentRef.current)); - }, [originalContent, showDiffIndicators, isDirty, contentRef, editorRef, diffDecorationsRef]); + + if (isDirty && originalContent) { + diffDecorationsRef.current.set(computeDiffGutterDecorations(originalContent, contentRef.current)); + return; + } + + if (vcsDiff) { + diffDecorationsRef.current.set(computePatchGutterDecorations(vcsDiff)); + return; + } + + diffDecorationsRef.current.set([]); + }, [originalContent, showDiffIndicators, isDirty, vcsDiff, contentRef, editorRef, diffDecorationsRef]); const diffTimerRef = useRef | null>(null); useEffect(() => { updateDiffDecorations(); - const editor = editorRef.current; + const editor = editorReady ?? editorRef.current; if (!editor) return; const model = editor.getModel(); if (!model) return; @@ -159,7 +253,7 @@ export function useMonacoDiffDecorations(opts: UseMonacoDiffDecorationsOpts) { diffTimerRef.current = setTimeout(updateDiffDecorations, 150); }); return () => { if (diffTimerRef.current) clearTimeout(diffTimerRef.current); disposable.dispose(); }; - }, [updateDiffDecorations, editorRef]); + }, [updateDiffDecorations, editorRef, editorReady]); // Diff stats const [diffStats, setDiffStats] = useState<{ additions: number; deletions: number } | null>(null); diff --git a/apps/web/components/review/review-dialog.tsx b/apps/web/components/review/review-dialog.tsx index 8ee61dbbd4..a925b85e9b 100644 --- a/apps/web/components/review/review-dialog.tsx +++ b/apps/web/components/review/review-dialog.tsx @@ -1,6 +1,6 @@ 'use client'; -import { memo, useMemo, useCallback, createRef, useState } from 'react'; +import { memo, useMemo, useCallback, createRef, useState, useRef, useEffect } from 'react'; import { Dialog, DialogContent, DialogTitle } from '@kandev/ui/dialog'; import type { DiffComment } from '@/lib/diff/types'; import type { FileInfo, CumulativeDiff } from '@/lib/state/slices/session-runtime/types'; @@ -100,6 +100,15 @@ export const ReviewDialog = memo(function ReviewDialog({ const commentCountByFile = useMemo(() => computeCommentCounts(byId, sessionCommentIds), [byId, sessionCommentIds]); const totalCommentCount = useMemo(() => Object.values(commentCountByFile).reduce((sum, c) => sum + c, 0), [commentCountByFile]); const fileRefs = useMemo(() => { const refs = new Map>(); for (const file of allFiles) refs.set(file.path, createRef()); return refs; }, [allFiles]); + const prevCountRef = useRef(null); + + useEffect(() => { + const prevCount = prevCountRef.current; + if (open && prevCount !== null && prevCount > 0 && allFiles.length === 0) { + onOpenChange(false); + } + prevCountRef.current = allFiles.length; + }, [open, allFiles.length, onOpenChange]); const handleToggleSplitView = useCallback((split: boolean) => { setSplitView(split); diff --git a/apps/web/components/review/review-diff-list.tsx b/apps/web/components/review/review-diff-list.tsx index a9d70b1139..1bfbafb574 100644 --- a/apps/web/components/review/review-diff-list.tsx +++ b/apps/web/components/review/review-diff-list.tsx @@ -146,7 +146,14 @@ function FileDiffSection({ file, isReviewed, isStale, sessionId, autoMarkOnScrol const hash = await calculateHash(currentContent); const lines = currentContent.split('\n'); lines.splice(info.addStart - 1, info.addCount, ...info.oldLines); - await updateFileContent(client, sessionId, filePath, generateUnifiedDiff(currentContent, lines.join('\n'), filePath), hash); + const nextContent = lines.join('\n'); + if (nextContent === currentContent) return; + + const patch = generateUnifiedDiff(currentContent, nextContent, filePath); + // Guard against no-op diffs ("No valid patches in input") from createTwoFilesPatch. + if (!patch || !/^@@/m.test(patch)) return; + + await updateFileContent(client, sessionId, filePath, patch, hash); } catch (err) { console.error('Failed to revert change block:', err); } diff --git a/apps/web/components/task/changes-panel-file-row.tsx b/apps/web/components/task/changes-panel-file-row.tsx index 091ffb2480..4cca331308 100644 --- a/apps/web/components/task/changes-panel-file-row.tsx +++ b/apps/web/components/task/changes-panel-file-row.tsx @@ -35,7 +35,7 @@ type ChangedFile = { type FileRowProps = { file: ChangedFile; isPending: boolean; - onOpenFile: (path: string) => void; + onOpenDiff: (path: string) => void; onStage: (path: string) => void; onUnstage: (path: string) => void; onDiscard: (path: string) => void; @@ -45,7 +45,7 @@ type FileRowProps = { export function FileRow({ file, isPending, - onOpenFile, + onOpenDiff, onStage, onUnstage, onDiscard, @@ -56,7 +56,7 @@ export function FileRow({ return (
  • onOpenFile(file.path)} + onClick={() => onOpenDiff(file.path)} >
    void; - onOpenFile: (path: string) => void; + onOpenDiff: (path: string) => void; + onEditFile: (path: string) => void; onStage: (path: string) => void; onUnstage: (path: string) => void; onDiscard: (path: string) => void; @@ -201,7 +202,8 @@ export function FileListSection({ isLast, actionLabel, onAction, - onOpenFile, + onOpenDiff, + onEditFile, onStage, onUnstage, onDiscard, @@ -232,11 +234,11 @@ export function FileListSection({ key={file.path} file={file} isPending={pendingStageFiles.has(file.path)} - onOpenFile={onOpenFile} + onOpenDiff={onOpenDiff} onStage={onStage} onUnstage={onUnstage} onDiscard={onDiscard} - onEditFile={onOpenFile} + onEditFile={onEditFile} /> ))} diff --git a/apps/web/components/task/changes-panel.tsx b/apps/web/components/task/changes-panel.tsx index b3c9054f65..54dd443f79 100644 --- a/apps/web/components/task/changes-panel.tsx +++ b/apps/web/components/task/changes-panel.tsx @@ -34,7 +34,8 @@ import { DiscardDialog, CommitDialog, PRDialog } from './changes-panel-dialogs'; import { FileListSection, CommitsSection, ActionButtonsSection, ReviewProgressBar } from './changes-panel-timeline'; type ChangesPanelProps = { - onOpenFile: (path: string) => void; + onOpenDiffFile: (path: string) => void; + onEditFile: (path: string) => void; onOpenCommitDetail?: (sha: string) => void; onOpenDiffAll?: () => void; onOpenReview?: () => void; @@ -237,7 +238,7 @@ function useChangesPanelStoreData() { return { activeSessionId, taskTitle, baseBranch, displayBranch }; } -const ChangesPanel = memo(function ChangesPanel({ onOpenFile, onOpenCommitDetail, onOpenDiffAll, onOpenReview }: ChangesPanelProps) { +const ChangesPanel = memo(function ChangesPanel({ onOpenDiffFile, onEditFile, onOpenCommitDetail, onOpenDiffAll, onOpenReview }: ChangesPanelProps) { const isArchived = useIsTaskArchived(); const { activeSessionId, taskTitle, baseBranch, displayBranch } = useChangesPanelStoreData(); @@ -279,8 +280,8 @@ const ChangesPanel = memo(function ChangesPanel({ onOpenFile, onOpenCommitDetail {!hasAnything &&
    Your changed files will appear here
    } {hasAnything && (
    - {hasUnstaged && } - {hasStaged && } + {hasUnstaged && } + {hasStaged && } {hasCommits && } {hasCommits && }
    diff --git a/apps/web/components/task/dockview-desktop-layout.tsx b/apps/web/components/task/dockview-desktop-layout.tsx index e53cb0e687..b441aaeeb5 100644 --- a/apps/web/components/task/dockview-desktop-layout.tsx +++ b/apps/web/components/task/dockview-desktop-layout.tsx @@ -105,22 +105,34 @@ function ChatPanel(props: IDockviewPanelProps) { ); } -function DiffViewerPanelComponent() { +function DiffViewerPanelComponent(props: IDockviewPanelProps<{ kind?: 'all' | 'file'; path?: string; content?: string }>) { const selectedDiff = useDockviewStore((s) => s.selectedDiff); const setSelectedDiff = useDockviewStore((s) => s.setSelectedDiff); const { openFile } = useFileEditors(); + const panelKind = props.params?.kind ?? 'all'; + const selectedPath = panelKind === 'file' ? props.params?.path : undefined; + const panelSelectedDiff = panelKind === 'all' ? selectedDiff : null; + const handleClosePanel = useCallback(() => { + const dockApi = useDockviewStore.getState().api; + const panel = dockApi?.getPanel(props.api.id); + if (dockApi && panel) dockApi.removePanel(panel); + }, [props.api.id]); return ( setSelectedDiff(null)} onOpenFile={openFile} + onBecameEmpty={handleClosePanel} /> ); } function ChangesPanelWrapper(props: IDockviewPanelProps) { const addDiffViewerPanel = useDockviewStore((s) => s.addDiffViewerPanel); + const addFileDiffPanel = useDockviewStore((s) => s.addFileDiffPanel); const addCommitDetailPanel = useDockviewStore((s) => s.addCommitDetailPanel); const { openFile } = useFileEditors(); @@ -139,10 +151,14 @@ function ChangesPanelWrapper(props: IDockviewPanelProps) { } }, [totalCount, props.api]); - const handleOpenFile = useCallback((path: string) => { + const handleEditFile = useCallback((path: string) => { openFile(path); }, [openFile]); + const handleOpenDiffFile = useCallback((path: string) => { + addFileDiffPanel(path); + }, [addFileDiffPanel]); + const handleOpenCommitDetail = useCallback((sha: string) => { addCommitDetailPanel(sha); }, [addCommitDetailPanel]); @@ -157,7 +173,8 @@ function ChangesPanelWrapper(props: IDockviewPanelProps) { return ( { if (a.is_dir !== b.is_dir) return a.is_dir ? -1 : 1; @@ -56,6 +59,7 @@ type TreeNodeItemProps = { activeFilePath?: string | null; visibleLoadingPaths: Set; creatingInPath: string | null; + fileStatuses: Map; tree: FileTreeNode | null; onToggleExpand: (node: FileTreeNode) => void; onOpenFile: (path: string) => void; @@ -71,6 +75,22 @@ export function removeNodeFromTree(root: FileTreeNode, targetPath: string): File return { ...root, children: filtered.map((c) => removeNodeFromTree(c, targetPath)) }; } +function treeNodePaddingLeft(depth: number, isDir: boolean): string { + return `${depth * 12 + 8 + (isDir ? 0 : 20)}px`; +} + +function handleTreeNodeClick( + node: FileTreeNode, + onToggleExpand: (node: FileTreeNode) => void, + onOpenFile: (path: string) => void, +) { + if (node.is_dir) { + onToggleExpand(node); + return; + } + onOpenFile(node.path); +} + /** Expand/collapse chevron for directory nodes. */ function TreeNodeExpandChevron({ isLoading, isExpanded }: { isLoading: boolean; isExpanded: boolean }) { if (isLoading) return ; @@ -95,6 +115,35 @@ function TreeNodeFileIcon({ node, isExpanded, isActive }: { node: FileTreeNode; ); } +function getGitStatusTextClass(status: GitFileStatus): string { + switch (status) { + case 'added': + case 'untracked': + return 'text-emerald-600'; + case 'modified': + return 'text-yellow-600'; + default: + return ''; + } +} + +function getGitStatusDotClass(status: GitFileStatus): string { + switch (status) { + case 'added': + case 'untracked': + return 'bg-emerald-500'; + case 'modified': + return 'bg-yellow-500'; + default: + return 'bg-transparent'; + } +} + +function TreeNodeGitStatusDot({ status }: { status: GitFileStatus }) { + if (!status || (status !== 'added' && status !== 'untracked' && status !== 'modified')) return null; + return ; +} + /** Delete button shown on hover for file nodes. */ function TreeNodeDeleteButton({ node, tree, setTree, onDeleteFile, @@ -105,7 +154,7 @@ function TreeNodeDeleteButton({ onDeleteFile: (path: string) => Promise; }) { return ( -
    +