electron-rare/Aperant
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge conflict resolution progress bar and log viewer (#1620)

Browse Source 
* auto-claude: subtask-1-3 - Thread progress callback through MergePipeline and ConflictResolver

Add progress_callback parameter to MergePipeline.merge_file() and
ConflictResolver.resolve_conflicts(). MergePipeline emits per-file
progress at the start of merge within the resolving stage (50-75%).
ConflictResolver emits per-conflict resolution progress with details
about current file, conflict count, and conflicts resolved so far.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* auto-claude: subtask-1-4 - Wire progress emission into CLI merge entry point.

Add _create_merge_progress_callback() helper that returns emit_progress
only when stdout is piped (subprocess mode from Electron), avoiding
JSON pollution in interactive CLI sessions.

Wire the callback into _try_smart_merge_inner() with progress emissions
at key pipeline stages: ANALYZING, DETECTING_CONFLICTS, RESOLVING,
COMPLETE, and ERROR.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* Fix PR review issues: conflict counts, progress calculations, and cross-task leakage

- Fix conflicts_found on COMPLETE/ERROR stages to use original conflict count
- Fix off-by-one in progress percentage calculations (50-75% range)
- Add JSON validation for MergeProgress before IPC transmission
- Add taskId filtering to prevent cross-task progress event leakage
- Limit log entries to 500 to prevent unbounded memory growth
- Fix race condition: wait for terminal progress event before hiding overlay
- Remove unused imports (ruff fixes)
- Remove orphaned unreachable code in workspace.py
- Fix test mocks to use optional config_dir argument

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
Andy
2026-01-30 13:20:34 +01:00
committed by GitHub
parent bad1a9b2c4
commit d16be30771
8 changed files with 90 additions and 41 deletions
Download Patch File Download Diff File Expand all files Collapse all files
apps/backend/core/workspace.py
+14 -4
View File
@@ -589,12 +589,15 @@ def _try_smart_merge_inner(
if progress_callback is not None:
stats = resolution_result.get("stats", {})
original_conflict_count = len(
git_conflicts.get("conflicting_files", [])
)
progress_callback(
MergeProgressStage.COMPLETE,
100,
"Merge complete",
{
"conflicts_found": stats.get("conflicts_resolved", 0),
"conflicts_found": original_conflict_count,
"conflicts_resolved": stats.get("conflicts_resolved", 0),
},
)
@@ -613,14 +616,21 @@ def _try_smart_merge_inner(
)
if progress_callback is not None:
original_conflict_count = len(
git_conflicts.get("conflicting_files", [])
)
remaining_count = len(
resolution_result.get("remaining_conflicts", [])
)
progress_callback(
MergeProgressStage.ERROR,
0,
"Some conflicts could not be resolved",
{
"conflicts_found": len(
resolution_result.get("remaining_conflicts", [])
),
"conflicts_found": original_conflict_count,
"conflicts_resolved": original_conflict_count
- remaining_count,
"conflicts_remaining": remaining_count,
},
)
apps/backend/merge/conflict_resolver.py
+2 -1
View File
@@ -87,7 +87,8 @@ class ConflictResolver:
for idx, conflict in enumerate(conflicts):
if progress_callback:
# Emit per-conflict progress within the resolving stage (50-75%)
conflict_percent = 50 + int((idx / max(total_conflicts, 1)) * 25)
# Calculate progress after processing (idx + 1) to reach 75% on last conflict
conflict_percent = 50 + int(((idx + 1) / max(total_conflicts, 1)) * 25)
progress_callback(
stage=MergeProgressStage.RESOLVING,
percent=conflict_percent,
apps/backend/merge/orchestrator.py
+2 -1
View File
@@ -360,7 +360,8 @@ class MergeOrchestrator:
# --- RESOLVING stage (50-75%) ---
total_files = len(modifications)
for idx, (file_path, snapshot) in enumerate(modifications):
file_percent = 50 + int((idx / max(total_files, 1)) * 25)
# Calculate progress after processing (idx + 1) to reach 75% on last file
file_percent = 50 + int(((idx + 1) / max(total_files, 1)) * 25)
_emit(
MergeProgressStage.RESOLVING,
file_percent,
apps/frontend/src/main/ipc-handlers/task/worktree-handlers.ts
+8 -1
View File
@@ -2145,7 +2145,14 @@ export function registerWorktreeHandlers(
try {
const parsed = JSON.parse(trimmed);
if (parsed && parsed.type === 'progress') {
// Validate parsed object has expected MergeProgress structure before forwarding
if (
parsed &&
parsed.type === 'progress' &&
typeof parsed.stage === 'string' &&
typeof parsed.percent === 'number' &&
typeof parsed.message === 'string'
) {
const mainWindow = getMainWindow();
if (mainWindow) {
mainWindow.webContents.send(IPC_CHANNELS.TASK_MERGE_PROGRESS, taskId, parsed);
apps/frontend/src/renderer/components/task-detail/TaskReview.tsx
+1
View File
@@ -134,6 +134,7 @@ export function TaskReview({
) : worktreeStatus?.exists ? (
/* Worktree exists but not yet staged - show staging UI */
<WorkspaceStatus
taskId={task.id}
worktreeStatus={worktreeStatus}
workspaceError={workspaceError}
stageOnly={stageOnly}
apps/frontend/src/renderer/components/task-detail/task-review/WorkspaceStatus.tsx
+41 -15
View File
@@ -27,7 +27,11 @@ import { MergeProgressOverlay } from './MergeProgressOverlay';
import type { WorktreeStatus, MergeConflict, MergeStats, GitConflictInfo, SupportedIDE, SupportedTerminal, MergeProgress, MergeLogEntry, MergeLogEntryType } from '../../../../shared/types';
import { useSettingsStore } from '../../../stores/settings-store';
// Maximum log entries to keep to prevent memory issues during long merges
const MAX_LOG_ENTRIES = 500;
interface WorkspaceStatusProps {
taskId: string;
worktreeStatus: WorktreeStatus;
workspaceError: string | null;
stageOnly: boolean;
@@ -86,6 +90,7 @@ const TERMINAL_LABELS: Partial<Record<SupportedTerminal, string>> = {
};
export function WorkspaceStatus({
taskId,
worktreeStatus,
workspaceError,
stageOnly,
@@ -131,20 +136,33 @@ export function WorkspaceStatus({
}, [isMerging]);
// Minimum display time: keep overlay visible for at least 500ms after merge ends
// Also wait for terminal progress event (complete/error) to avoid hiding before final message
useEffect(() => {
if (!isMerging && showOverlay && mergeStartTimeRef.current !== null) {
const elapsed = Date.now() - mergeStartTimeRef.current;
const MIN_DISPLAY_MS = 500;
const remaining = Math.max(0, MIN_DISPLAY_MS - elapsed);
// Check if we received a terminal progress event (complete or error)
const hasTerminalEvent = mergeProgress?.stage === 'complete' || mergeProgress?.stage === 'error';
if (remaining > 0) {
// Only hide if we have a terminal event OR if a fallback timeout expires
if (hasTerminalEvent) {
const elapsed = Date.now() - mergeStartTimeRef.current;
const MIN_DISPLAY_MS = 500;
const remaining = Math.max(0, MIN_DISPLAY_MS - elapsed);
if (remaining > 0) {
minDisplayTimerRef.current = setTimeout(() => {
setShowOverlay(false);
mergeStartTimeRef.current = null;
}, remaining);
} else {
setShowOverlay(false);
mergeStartTimeRef.current = null;
}
} else {
// Fallback: hide after 2s if no terminal event received (defensive)
minDisplayTimerRef.current = setTimeout(() => {
setShowOverlay(false);
mergeStartTimeRef.current = null;
}, remaining);
} else {
setShowOverlay(false);
mergeStartTimeRef.current = null;
}, 2000);
}
}
@@ -154,7 +172,7 @@ export function WorkspaceStatus({
minDisplayTimerRef.current = null;
}
};
}, [isMerging, showOverlay]);
}, [isMerging, showOverlay, mergeProgress?.stage]);
// Subscribe to merge progress IPC events
useEffect(() => {
@@ -169,24 +187,32 @@ export function WorkspaceStatus({
}
};
const cleanup = window.electronAPI.onMergeProgress((_taskId: string, progress: MergeProgress) => {
const cleanup = window.electronAPI.onMergeProgress((eventTaskId: string, progress: MergeProgress) => {
// Filter by task ID to prevent cross-task event leakage
if (eventTaskId !== taskId) return;
setMergeProgress(progress);
setLogEntries(prev => [
...prev,
{
setLogEntries(prev => {
const newEntry = {
timestamp: new Date().toISOString(),
type: stageToLogType(progress.stage),
message: progress.message,
details: progress.details?.current_file,
};
// Limit log entries to prevent unbounded growth during long merges
const updated = [...prev, newEntry];
if (updated.length > MAX_LOG_ENTRIES) {
return updated.slice(-MAX_LOG_ENTRIES);
}
]);
return updated;
});
});
// Store cleanup ref so we can call it on unmount even if isMerging changes
ipcCleanupRef.current = cleanup;
return cleanup;
}, [isMerging]);
}, [isMerging, taskId]);
// Ensure IPC listener cleanup on unmount during active merge
useEffect(() => {
tests/test_auth.py
+9 -6
View File
@@ -70,7 +70,7 @@ class TestEnvVarTokenResolution:
def test_no_token_returns_none(self, monkeypatch):
"""Returns None when no auth token is configured."""
# Mock keychain to return None (env vars already cleared by fixture)
monkeypatch.setattr("core.auth.get_token_from_keychain", lambda: None)
monkeypatch.setattr("core.auth.get_token_from_keychain", lambda _config_dir=None: None)
token = get_auth_token()
assert token is None
@@ -374,7 +374,7 @@ class TestRequireAuthToken:
for var in AUTH_TOKEN_ENV_VARS:
os.environ.pop(var, None)
# Mock keychain to return None (tests that need a token will set env var)
monkeypatch.setattr("core.auth.get_token_from_keychain", lambda: None)
monkeypatch.setattr("core.auth.get_token_from_keychain", lambda _config_dir=None: None)
yield
# Cleanup after test
for var in AUTH_TOKEN_ENV_VARS:
@@ -512,7 +512,8 @@ class TestTokenSourceDetection:
monkeypatch.setattr("subprocess.run", Mock(return_value=mock_result))
source = get_auth_token_source()
assert source == "macOS Keychain"
# Source can be "macOS Keychain" or "macOS Keychain (profile)" depending on profile settings
assert source is not None and source.startswith("macOS Keychain")
def test_source_windows_credential_files(self, monkeypatch, tmp_path):
"""Identifies Windows Credential Files as source."""
@@ -528,7 +529,8 @@ class TestTokenSourceDetection:
)
source = get_auth_token_source()
assert source == "Windows Credential Files"
# Source can have "(profile)" suffix depending on profile settings
assert source is not None and source.startswith("Windows Credential Files")
def test_source_linux_secret_service(self, monkeypatch):
"""Identifies Linux Secret Service as source."""
@@ -554,12 +556,13 @@ class TestTokenSourceDetection:
monkeypatch.setattr("core.auth.secretstorage", mock_ss)
source = get_auth_token_source()
assert source == "Linux Secret Service"
# Source can have "(profile)" suffix depending on profile settings
assert source is not None and source.startswith("Linux Secret Service")
def test_source_none_when_not_found(self, monkeypatch):
"""Returns None when no token source is found."""
# Mock keychain to return None (env vars already cleared by fixture)
monkeypatch.setattr("core.auth.get_token_from_keychain", lambda: None)
monkeypatch.setattr("core.auth.get_token_from_keychain", lambda _config_dir=None: None)
source = get_auth_token_source()
assert source is None
tests/test_client.py
+13 -13
View File
@@ -46,7 +46,7 @@ class TestClientTokenValidation:
monkeypatch.setenv("CLAUDE_CODE_OAUTH_TOKEN", "enc:test123456789012")
# Mock keychain to ensure encrypted token is the only source
monkeypatch.setattr("core.auth.get_token_from_keychain", lambda: None)
monkeypatch.setattr("core.auth.get_token_from_keychain", lambda _config_dir=None: None)
# Mock decrypt_token to raise ValueError (simulates decryption failure)
# This ensures the encrypted token flows through to validate_token_not_encrypted
monkeypatch.setattr(
@@ -63,7 +63,7 @@ class TestClientTokenValidation:
monkeypatch.setenv("CLAUDE_CODE_OAUTH_TOKEN", "enc:test123456789012")
# Mock keychain to ensure encrypted token is the only source
monkeypatch.setattr("core.auth.get_token_from_keychain", lambda: None)
monkeypatch.setattr("core.auth.get_token_from_keychain", lambda _config_dir=None: None)
# Mock decrypt_token to raise ValueError (simulates decryption failure)
monkeypatch.setattr(
"core.auth.decrypt_token",
@@ -77,7 +77,7 @@ class TestClientTokenValidation:
"""Verify create_client() accepts valid plaintext tokens and creates SDK client."""
valid_token = "sk-ant-oat01-valid-plaintext-token"
monkeypatch.setenv("CLAUDE_CODE_OAUTH_TOKEN", valid_token)
monkeypatch.setattr("core.auth.get_token_from_keychain", lambda: None)
monkeypatch.setattr("core.auth.get_token_from_keychain", lambda _config_dir=None: None)
# Mock the SDK client to avoid actual initialization
mock_sdk_client = MagicMock()
@@ -93,7 +93,7 @@ class TestClientTokenValidation:
"""Verify create_simple_client() accepts valid plaintext tokens and creates SDK client."""
valid_token = "sk-ant-oat01-valid-plaintext-token"
monkeypatch.setenv("CLAUDE_CODE_OAUTH_TOKEN", valid_token)
monkeypatch.setattr("core.auth.get_token_from_keychain", lambda: None)
monkeypatch.setattr("core.auth.get_token_from_keychain", lambda _config_dir=None: None)
# Mock the SDK client to avoid actual initialization
mock_sdk_client = MagicMock()
@@ -113,7 +113,7 @@ class TestClientTokenValidation:
"""Verify create_client() validates token format before SDK initialization."""
valid_token = "sk-ant-oat01-valid-token"
monkeypatch.setenv("CLAUDE_CODE_OAUTH_TOKEN", valid_token)
monkeypatch.setattr("core.auth.get_token_from_keychain", lambda: None)
monkeypatch.setattr("core.auth.get_token_from_keychain", lambda _config_dir=None: None)
# Mock validate_token_not_encrypted to verify it's called
with patch(
@@ -130,7 +130,7 @@ class TestClientTokenValidation:
"""Verify create_simple_client() validates token format before SDK initialization."""
valid_token = "sk-ant-oat01-valid-token"
monkeypatch.setenv("CLAUDE_CODE_OAUTH_TOKEN", valid_token)
monkeypatch.setattr("core.auth.get_token_from_keychain", lambda: None)
monkeypatch.setattr("core.auth.get_token_from_keychain", lambda _config_dir=None: None)
# Mock validate_token_not_encrypted to verify it's called
with patch(
@@ -213,7 +213,7 @@ class TestAPIProfileAuthentication:
# Don't set ANTHROPIC_BASE_URL - this should trigger OAuth mode
monkeypatch.delenv("ANTHROPIC_BASE_URL", raising=False)
monkeypatch.setenv("CLAUDE_CODE_OAUTH_TOKEN", oauth_token)
monkeypatch.setattr("core.auth.get_token_from_keychain", lambda: None)
monkeypatch.setattr("core.auth.get_token_from_keychain", lambda _config_dir=None: None)
# Mock the SDK client
mock_sdk_client = MagicMock()
@@ -270,7 +270,7 @@ class TestAPIProfileAuthentication:
# Set empty ANTHROPIC_BASE_URL - should be treated as "not set"
monkeypatch.setenv("ANTHROPIC_BASE_URL", "")
monkeypatch.setenv("CLAUDE_CODE_OAUTH_TOKEN", oauth_token)
monkeypatch.setattr("core.auth.get_token_from_keychain", lambda: None)
monkeypatch.setattr("core.auth.get_token_from_keychain", lambda _config_dir=None: None)
# Mock require_auth_token to verify it's called (OAuth mode)
with patch("core.auth.require_auth_token", return_value=oauth_token):
@@ -314,7 +314,7 @@ class TestAPIProfileAuthentication:
monkeypatch.delenv("ANTHROPIC_BASE_URL", raising=False)
# Mock keychain to return None
monkeypatch.setattr("core.auth.get_token_from_keychain", lambda: None)
monkeypatch.setattr("core.auth.get_token_from_keychain", lambda _config_dir=None: None)
from core.client import create_client
@@ -397,7 +397,7 @@ class TestAPIProfileAuthenticationIntegration:
monkeypatch.setenv("ANTHROPIC_AUTH_TOKEN", api_token)
monkeypatch.delenv("ANTHROPIC_BASE_URL", raising=False)
monkeypatch.setenv("CLAUDE_CODE_OAUTH_TOKEN", encrypted_oauth_token)
monkeypatch.setattr("core.auth.get_token_from_keychain", lambda: None)
monkeypatch.setattr("core.auth.get_token_from_keychain", lambda _config_dir=None: None)
from core.client import create_client
@@ -421,7 +421,7 @@ class TestAPIProfileAuthenticationEdgeCases:
# Set whitespace-only ANTHROPIC_BASE_URL - should be trimmed to empty string
monkeypatch.setenv("ANTHROPIC_BASE_URL", " ")
monkeypatch.setenv("CLAUDE_CODE_OAUTH_TOKEN", oauth_token)
monkeypatch.setattr("core.auth.get_token_from_keychain", lambda: None)
monkeypatch.setattr("core.auth.get_token_from_keychain", lambda _config_dir=None: None)
# Mock the SDK client
mock_sdk_client = MagicMock()
@@ -526,7 +526,7 @@ class TestSimpleClientAPIProfileAuthentication:
monkeypatch.delenv("ANTHROPIC_BASE_URL", raising=False)
monkeypatch.setenv("CLAUDE_CODE_OAUTH_TOKEN", oauth_token)
monkeypatch.setattr("core.auth.get_token_from_keychain", lambda: None)
monkeypatch.setattr("core.auth.get_token_from_keychain", lambda _config_dir=None: None)
mock_sdk_client = MagicMock()
with patch("core.simple_client.ClaudeSDKClient", return_value=mock_sdk_client):
@@ -582,7 +582,7 @@ class TestSimpleClientAPIProfileAuthentication:
# Set whitespace-only ANTHROPIC_BASE_URL - should be trimmed to empty string
monkeypatch.setenv("ANTHROPIC_BASE_URL", " ")
monkeypatch.setenv("CLAUDE_CODE_OAUTH_TOKEN", oauth_token)
monkeypatch.setattr("core.auth.get_token_from_keychain", lambda: None)
monkeypatch.setattr("core.auth.get_token_from_keychain", lambda _config_dir=None: None)
mock_sdk_client = MagicMock()
with patch("core.simple_client.ClaudeSDKClient", return_value=mock_sdk_client):