-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
fix(ACS-175): Resolve integrations freeze and improve rate limit handling #839
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 7 commits
b03e446
b20c1bb
27ae640
56a6bed
a37e072
f9fdd8d
8328066
da9b707
e033f51
0f1a7ec
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,6 +10,7 @@ import type { TerminalProcess, WindowGetter } from './types'; | |
| import { IPC_CHANNELS } from '../../shared/constants'; | ||
| import { getClaudeProfileManager } from '../claude-profile-manager'; | ||
| import { readSettingsFile } from '../settings-utils'; | ||
| import { debugLog, debugError } from '../../shared/utils/debug-logger'; | ||
| import type { SupportedTerminal } from '../../shared/types/settings'; | ||
|
|
||
| /** | ||
|
|
@@ -84,7 +85,7 @@ export function spawnPtyProcess( | |
|
|
||
| const shellArgs = process.platform === 'win32' ? [] : ['-l']; | ||
|
|
||
| console.warn('[PtyManager] Spawning shell:', shell, shellArgs, '(preferred:', preferredTerminal || 'system', ')'); | ||
| debugLog('[PtyManager] Spawning shell:', shell, shellArgs, '(preferred:', preferredTerminal || 'system', ')'); | ||
|
|
||
| // Create a clean environment without DEBUG to prevent Claude Code from | ||
| // enabling debug mode when the Electron app is run in development mode. | ||
|
|
@@ -137,7 +138,7 @@ export function setupPtyHandlers( | |
|
|
||
| // Handle terminal exit | ||
| ptyProcess.onExit(({ exitCode }) => { | ||
| console.warn('[PtyManager] Terminal exited:', id, 'code:', exitCode); | ||
| debugLog('[PtyManager] Terminal exited:', id, 'code:', exitCode); | ||
|
|
||
| const win = getWindow(); | ||
| if (win) { | ||
|
|
@@ -151,11 +152,67 @@ export function setupPtyHandlers( | |
| }); | ||
| } | ||
|
|
||
| /** | ||
| * Constants for chunked write behavior | ||
| * CHUNKED_WRITE_THRESHOLD: Data larger than this (bytes) will be written in chunks | ||
| * CHUNK_SIZE: Size of each chunk - smaller chunks yield to event loop more frequently | ||
| */ | ||
| const CHUNKED_WRITE_THRESHOLD = 1000; | ||
| const CHUNK_SIZE = 100; | ||
|
|
||
| /** | ||
| * Write data to a PTY process | ||
| * Uses setImmediate to prevent blocking the event loop on large writes | ||
| */ | ||
| export function writeToPty(terminal: TerminalProcess, data: string): void { | ||
| terminal.pty.write(data); | ||
| debugLog('[PtyManager:writeToPty] About to write to pty, data length:', data.length); | ||
|
|
||
| // For large commands, write in chunks to prevent blocking | ||
| if (data.length > CHUNKED_WRITE_THRESHOLD) { | ||
| debugLog('[PtyManager:writeToPty] Large write detected, using chunked write'); | ||
| let offset = 0; | ||
| let chunkNum = 0; | ||
|
|
||
| const writeChunk = () => { | ||
|
||
| // Check if terminal is still valid before writing | ||
| if (!terminal.pty) { | ||
| debugError('[PtyManager:writeToPty] Terminal PTY no longer valid, aborting chunked write'); | ||
| return; | ||
| } | ||
|
|
||
| if (offset >= data.length) { | ||
| debugLog('[PtyManager:writeToPty] Chunked write completed, total chunks:', chunkNum); | ||
| return; | ||
| } | ||
|
|
||
| const chunk = data.slice(offset, offset + CHUNK_SIZE); | ||
| chunkNum++; | ||
| debugLog('[PtyManager:writeToPty] Writing chunk', chunkNum, 'offset:', offset, 'size:', chunk.length); | ||
| try { | ||
| terminal.pty.write(chunk); | ||
| debugLog('[PtyManager:writeToPty] Chunk', chunkNum, 'written'); | ||
| offset += CHUNK_SIZE; | ||
| // Use setImmediate to yield to the event loop between chunks | ||
| setImmediate(writeChunk); | ||
| } catch (error) { | ||
| debugError('[PtyManager:writeToPty] Chunked write FAILED at chunk', chunkNum, 'offset', offset, ':', error); | ||
| // Don't rethrow - match non-chunked behavior for consistency (fire-and-forget semantics) | ||
| } | ||
| }; | ||
coderabbitai[bot] marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| // Start the chunked write after yielding | ||
| debugLog('[PtyManager:writeToPty] Scheduling first chunk...'); | ||
| setImmediate(writeChunk); | ||
| debugLog('[PtyManager:writeToPty] First chunk scheduled, returning'); | ||
coderabbitai[bot] marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| } else { | ||
| try { | ||
| terminal.pty.write(data); | ||
| debugLog('[PtyManager:writeToPty] Write completed successfully'); | ||
| } catch (error) { | ||
| debugError('[PtyManager:writeToPty] Write FAILED:', error); | ||
| // Don't rethrow - fire-and-forget terminal write semantics | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /** | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🏁 Script executed:
Repository: AndyMik90/Auto-Claude
Length of output: 171
🏁 Script executed:
Repository: AndyMik90/Auto-Claude
Length of output: 3092
🏁 Script executed:
Repository: AndyMik90/Auto-Claude
Length of output: 158
🏁 Script executed:
Repository: AndyMik90/Auto-Claude
Length of output: 9408
🏁 Script executed:
Repository: AndyMik90/Auto-Claude
Length of output: 4038
The chunked write approach is sound, though the rationale for specific chunk size could be clearer.
The constants are well-extracted and documented. However, the logic is intentionally designed to yield to the event loop via
setImmediatebetween chunks—this is not a latency concern but the intended behavior to prevent blocking. CHUNK_SIZE of 100 bytes is conservative by design for this purpose, and the 1000-byte threshold ensures small writes bypass chunking entirely. While performance testing could validate whether the chunk size is optimal for actual throughput, it is not necessary for correctness, as the current values are reasonable for a terminal I/O operation.🤖 Prompt for AI Agents