fix(frontend): prevent "Render frame was disposed" crash (ACS-211) #918
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add safeSendToRenderer helper that validates frame state before IPC sends. Prevents app crash when renderer frames are disposed during heavy agent output. Fixes AndyMik90#211 Signed-off-by: StillKnotKnown <[email protected]>
Contributor
|
Warning Rate limit exceeded@StillKnotKnown has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 16 minutes and 17 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughAdds a new Changes
Sequence Diagram(s)sequenceDiagram
participant AgentManager
participant MainProcess as Main
participant Utils as safeSendToRenderer
participant BrowserWindow
participant Renderer
AgentManager->>Main: emit event (e.g., roadmap-progress)
Main->>Utils: safeSendToRenderer(getMainWindow, channel, ...args)
Utils->>BrowserWindow: getMainWindow()
alt BrowserWindow null or destroyed
Utils-->>Main: return false (maybe warn/cooldown)
else BrowserWindow present
Utils->>BrowserWindow: webContents.isDestroyed()?
alt webContents destroyed
Utils-->>Main: return false (warn/cooldown)
else webContents ok
Utils->>Renderer: webContents.send(channel, ...args)
Utils-->>Main: return true
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Summary of ChangesHello @StillKnotKnown, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the stability of the Electron frontend by addressing a common crash related to inter-process communication. By introducing a new, robust utility for sending messages to the renderer, the application can now gracefully handle scenarios where renderer frames are no longer active, preventing unexpected terminations and improving overall user experience, especially during concurrent operations. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces a robust fix for the 'Render frame was disposed' crash by adding a safeSendToRenderer helper function. The new utility is well-implemented with checks for disposed windows and webContents, and it includes a sensible cooldown for warnings to prevent log spam. The refactoring of existing IPC handlers in agent-events-handlers.ts, ideation-handlers.ts, and roadmap-handlers.ts to use this new utility is done correctly and simplifies the code. The addition of a comprehensive test suite for safeSendToRenderer in utils.test.ts is excellent and covers numerous edge cases, ensuring the reliability of the fix. I have one minor suggestion to improve the maintainability of the new utility function by reducing some code duplication. Overall, this is a high-quality contribution that effectively addresses a critical stability issue.
Comment on lines
40
to
89
| export function safeSendToRenderer( | ||
| getMainWindow: () => BrowserWindow | null, | ||
| channel: string, | ||
| ...args: unknown[] | ||
| ): boolean { | ||
| try { | ||
| const mainWindow = getMainWindow(); | ||
|
|
||
| if (!mainWindow) { | ||
| return false; | ||
| } | ||
|
|
||
| // Check if window or webContents is destroyed | ||
| // isDestroyed() returns true if the window has been closed and destroyed | ||
| if (mainWindow.isDestroyed()) { | ||
| if (!recentlyWarnedChannels.has(channel)) { | ||
| console.warn(`[safeSendToRenderer] Skipping send to destroyed window: ${channel}`); | ||
| recentlyWarnedChannels.add(channel); | ||
| } | ||
| return false; | ||
| } | ||
|
|
||
| // Check if webContents is destroyed (can happen independently of window) | ||
| if (!mainWindow.webContents || mainWindow.webContents.isDestroyed()) { | ||
| if (!recentlyWarnedChannels.has(channel)) { | ||
| console.warn(`[safeSendToRenderer] Skipping send to destroyed webContents: ${channel}`); | ||
| recentlyWarnedChannels.add(channel); | ||
| } | ||
| return false; | ||
| } | ||
|
|
||
| // All checks passed - safe to send | ||
| mainWindow.webContents.send(channel, ...args); | ||
| return true; | ||
| } catch (error) { | ||
| // Catch any disposal errors that might occur between our checks and the actual send | ||
| const errorMessage = error instanceof Error ? error.message : String(error); | ||
|
|
||
| // Only log disposal errors once per channel to avoid log spam | ||
| if (errorMessage.includes("disposed") || errorMessage.includes("destroyed")) { | ||
| if (!recentlyWarnedChannels.has(channel)) { | ||
| console.warn(`[safeSendToRenderer] Frame disposed, skipping send: ${channel}`); | ||
| recentlyWarnedChannels.add(channel); | ||
| } | ||
| } else { | ||
| console.error(`[safeSendToRenderer] Error sending to renderer:`, error); | ||
| } | ||
| return false; | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
The logic for logging a warning only once per channel is duplicated in three places within this function (lines 55-58, 64-67, and 80-83). To improve maintainability and adhere to the DRY (Don't Repeat Yourself) principle, you could extract this logic into a small helper function.
For example, you could define a warnOnce helper and use it in the checks:
const warnOnce = (message: string) => {
if (!recentlyWarnedChannels.has(channel)) {
console.warn(`[safeSendToRenderer] ${message}: ${channel}`);
recentlyWarnedChannels.add(channel);
}
};
// ... inside the checks
if (mainWindow.isDestroyed()) {
warnOnce('Skipping send to destroyed window');
return false;
}![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/frontend/src/main/ipc-handlers/utils.ts (1)
94-116:parseEnvFile: consider\r\nsplitting explicitly.
split("\n")+trim()will usually handle\r\n, but using/\r?\n/is a low-cost hardening if these env files are commonly edited on Windows.
🤖 Fix all issues with AI agents
In @apps/frontend/src/main/ipc-handlers/utils.ts:
- Around line 7-17: The cooldown logic currently uses a module-level setInterval
clearing recentlyWarnedChannels (a Set) which leaks timers and gives
inconsistent per-channel cooldowns; replace recentlyWarnedChannels: Set<string>
with a Map<string, number> storing last-warn timestamps and remove the
setInterval entirely, keep WARN_COOLDOWN_MS = 5000, and change any checks to
compare Date.now() - (map.get(channel) || 0) >= WARN_COOLDOWN_MS before allowing
a new warning and then set map.set(channel, Date.now()); optionally prune
entries when reading if you want to free memory, but do not introduce timers.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
apps/frontend/src/main/__tests__/ipc-handlers.test.tsapps/frontend/src/main/__tests__/utils.test.tsapps/frontend/src/main/ipc-handlers/agent-events-handlers.tsapps/frontend/src/main/ipc-handlers/ideation-handlers.tsapps/frontend/src/main/ipc-handlers/roadmap-handlers.tsapps/frontend/src/main/ipc-handlers/utils.ts
🧰 Additional context used
📓 Path-based instructions (3)
apps/frontend/src/**/*.{ts,tsx,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always use i18n translation keys for all user-facing text in the frontend instead of hardcoded strings
Files:
apps/frontend/src/main/__tests__/utils.test.tsapps/frontend/src/main/ipc-handlers/agent-events-handlers.tsapps/frontend/src/main/ipc-handlers/utils.tsapps/frontend/src/main/ipc-handlers/roadmap-handlers.tsapps/frontend/src/main/__tests__/ipc-handlers.test.tsapps/frontend/src/main/ipc-handlers/ideation-handlers.ts
apps/frontend/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
useTranslation()hook with namespace prefixes (e.g., 'navigation:items.key') for accessing translation strings in React components
Files:
apps/frontend/src/main/__tests__/utils.test.tsapps/frontend/src/main/ipc-handlers/agent-events-handlers.tsapps/frontend/src/main/ipc-handlers/utils.tsapps/frontend/src/main/ipc-handlers/roadmap-handlers.tsapps/frontend/src/main/__tests__/ipc-handlers.test.tsapps/frontend/src/main/ipc-handlers/ideation-handlers.ts
apps/frontend/**/*.{ts,tsx}
⚙️ CodeRabbit configuration file
apps/frontend/**/*.{ts,tsx}: Review React patterns and TypeScript type safety.
Check for proper state management and component composition.
Files:
apps/frontend/src/main/__tests__/utils.test.tsapps/frontend/src/main/ipc-handlers/agent-events-handlers.tsapps/frontend/src/main/ipc-handlers/utils.tsapps/frontend/src/main/ipc-handlers/roadmap-handlers.tsapps/frontend/src/main/__tests__/ipc-handlers.test.tsapps/frontend/src/main/ipc-handlers/ideation-handlers.ts
🧠 Learnings (1)
📚 Learning: 2025-12-30T16:38:36.314Z
Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T16:38:36.314Z
Learning: Applies to apps/backend/.env* : Enable Electron MCP for E2E testing by setting `ELECTRON_MCP_ENABLED=true` in `.env` and starting the Electron app with `npm run dev`
Applied to files:
apps/frontend/src/main/__tests__/ipc-handlers.test.ts
🧬 Code graph analysis (5)
apps/frontend/src/main/__tests__/utils.test.ts (2)
apps/frontend/src/__mocks__/electron.ts (1)
BrowserWindow(64-98)apps/frontend/src/main/ipc-handlers/utils.ts (1)
safeSendToRenderer(40-89)
apps/frontend/src/main/ipc-handlers/agent-events-handlers.ts (7)
apps/frontend/src/shared/types/task.ts (2)
Task(243-263)TaskStatus(8-8)apps/frontend/src/main/ipc-handlers/utils.ts (1)
safeSendToRenderer(40-89)apps/frontend/src/shared/constants/ipc.ts (1)
IPC_CHANNELS(6-517)apps/frontend/src/shared/types/terminal.ts (1)
SDKRateLimitInfo(87-122)apps/frontend/src/main/ipc-handlers/task/plan-file-utils.ts (2)
getPlanPath(68-72)persistPlanStatusSync(160-186)apps/frontend/src/main/worktree-paths.ts (1)
findTaskWorktree(50-78)apps/frontend/src/shared/constants/config.ts (1)
getSpecsDir(106-109)
apps/frontend/src/main/ipc-handlers/utils.ts (1)
apps/frontend/src/__mocks__/electron.ts (1)
BrowserWindow(64-98)
apps/frontend/src/main/__tests__/ipc-handlers.test.ts (1)
apps/frontend/src/main/ipc-handlers/index.ts (1)
setupIpcHandlers(46-122)
apps/frontend/src/main/ipc-handlers/ideation-handlers.ts (2)
apps/frontend/src/main/ipc-handlers/ideation/task-converter.ts (1)
convertIdeaToTask(181-266)apps/frontend/src/main/ipc-handlers/utils.ts (1)
safeSendToRenderer(40-89)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: CodeQL (python)
- GitHub Check: CodeQL (javascript-typescript)
🔇 Additional comments (15)
apps/frontend/src/main/__tests__/ipc-handlers.test.ts (2)
182-190: Good: test window mocks now match safe-send destruction checks.
173-232: Movevi.resetModules()to the start ofbeforeEach, or re-import/rebindipcMainafter the reset.The current placement of
vi.resetModules()at the end ofbeforeEach(after importing electron) causes a module instance mismatch. The electron mock is imported before reset, saving anipcMainreference. Whenvi.resetModules()runs and subsequent tests importsetupIpcHandlers, the electron module is re-instantiated with a differentipcMainobject. Handlers get registered on this new instance, but tests invoke methods on the old reference, resulting in "No handler for channel" errors. Either callvi.resetModules()first inbeforeEach, or re-import and rebindipcMainafter the reset to ensure consistency.apps/frontend/src/main/ipc-handlers/ideation-handlers.ts (2)
58-114: LGTM: ideation events now usesafeSendToRendererto avoid disposed-frame crashes.
59-69: Verify generation handlers don’t still send unsafely.These pass
getMainWindow()(a window snapshot) intostartIdeationGeneration/refreshIdeationSession/stopIdeationGeneration. If those internals callwebContents.senddirectly, the original crash can still happen. Consider passinggetMainWindowthrough, or usingsafeSendToRendererinside those handlers too.apps/frontend/src/main/__tests__/utils.test.ts (1)
191-232: Add assertions for the “log spam prevention” requirement (cooldown).Right now the “warning cooldown behavior” tests don’t verify that warnings are actually suppressed (e.g.,
console.warncalled once per channel per cooldown). Considervi.spyOn(console, "warn")+vi.useFakeTimers()and advancing time past 5000ms to assert the cooldown resets.apps/frontend/src/main/ipc-handlers/agent-events-handlers.ts (1)
114-135: LGTM: all high-volume agent → renderer sends now go throughsafeSendToRenderer.Also applies to: 271-356
apps/frontend/src/main/ipc-handlers/roadmap-handlers.ts (9)
1-28: LGTM!Imports are well-organized and the addition of
safeSendToRendererfrom./utilsaligns with the PR objective to centralize safe renderer communication.
33-59: LGTM!The function handles settings retrieval with proper error handling and fallbacks to defaults. Synchronous reads are acceptable here for configuration access in the main process.
72-217: LGTM!The handler correctly uses the request-response pattern (
ipcMain.handle) for fetching data, with comprehensive snake_case to camelCase transformation and appropriate fallback defaults for optional fields.
220-227: LGTM!Simple and correct implementation for querying generation status.
229-289: LGTM!Good use of
safeSendToRendererwith the getter function pattern, which ensures the latest window state is retrieved on each send. The early return for null window and error handling for missing projects are appropriate.
291-344: LGTM!Consistent implementation pattern with the generate handler, correctly using
safeSendToRendererfor renderer communication.
346-360: LGTM!The new stop handler correctly combines the request-response pattern (returning success status) with an event notification (
ROADMAP_STOPPED) for UI updates. UsingsafeSendToRendererensures graceful handling if the window is disposed between the stop request and the notification.
366-621: LGTM!These handlers correctly use the request-response pattern (
ipcMain.handle) wheresafeSendToRendererisn't needed since results are returned directly to the caller. The file operations and error handling are appropriate for the main process context.
627-637: Key fix implementation looks good.These agent event handlers are the critical path for the "Render frame was disposed" crash. Previously, events from
agentManagerwould have been sent directly tomainWindow.webContents.send, which could fail if the renderer frame was disposed. UsingsafeSendToRendererhere ensures graceful handling when windows are closed during agent operations.
…dd \r\n support - Replace setInterval-based cooldown with timestamp Map approach to avoid timer leaks - Add isWithinCooldown() and recordWarning() helper functions - Optionally prune old entries when Map exceeds 100 entries - Update parseEnvFile to handle Windows \r\n line endings with /\r?\n/ regex Signed-off-by: StillKnotKnown <[email protected]>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @apps/frontend/src/main/ipc-handlers/utils.ts:
- Around line 30-38: The pruning only deletes expired entries and may not reduce
warnTimestamps below the threshold if many entries are still within
WARN_COOLDOWN_MS; update the block handling warnTimestamps (the if
(warnTimestamps.size > 100) { ... } logic) to first remove expired entries
as-is, and then, if warnTimestamps.size still exceeds 100, remove the oldest
entries (iterate over warnTimestamps.keys() or entries() which preserve
insertion order and delete until warnTimestamps.size <= 100) to enforce a hard
cap.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
apps/frontend/src/main/ipc-handlers/utils.ts
🧰 Additional context used
📓 Path-based instructions (3)
apps/frontend/src/**/*.{ts,tsx,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always use i18n translation keys for all user-facing text in the frontend instead of hardcoded strings
Files:
apps/frontend/src/main/ipc-handlers/utils.ts
apps/frontend/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
useTranslation()hook with namespace prefixes (e.g., 'navigation:items.key') for accessing translation strings in React components
Files:
apps/frontend/src/main/ipc-handlers/utils.ts
apps/frontend/**/*.{ts,tsx}
⚙️ CodeRabbit configuration file
apps/frontend/**/*.{ts,tsx}: Review React patterns and TypeScript type safety.
Check for proper state management and component composition.
Files:
apps/frontend/src/main/ipc-handlers/utils.ts
🧬 Code graph analysis (1)
apps/frontend/src/main/ipc-handlers/utils.ts (1)
apps/frontend/src/__mocks__/electron.ts (1)
BrowserWindow(64-98)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: CodeQL (python)
- GitHub Check: CodeQL (javascript-typescript)
🔇 Additional comments (3)
apps/frontend/src/main/ipc-handlers/utils.ts (3)
5-6: LGTM! Type-only import is correct.Proper use of
import typefor the BrowserWindow type, which will be erased at compile time and has no runtime overhead.
63-112: Excellent implementation addressing the disposal crash.The function correctly handles the TOCTOU race condition between state checks and the actual send operation through the try-catch wrapper (lines 97-111). The multi-layered validation (window null check, isDestroyed checks, and error handling) ensures robust disposal detection.
Key strengths:
- Dependency injection via
getMainWindowcallback enables testing- Per-channel cooldown prevents log spam effectively
- Boolean return allows callers to handle failures appropriately
- Catches both pre-send disposal (checks) and during-send disposal (try-catch)
Minor consideration: Callers should handle the
falsereturn value appropriately (e.g., queuing messages, logging, or dropping). The silent failure is by design but should be documented in consumer code.
119-120: Good addition of cross-platform line ending support.The regex pattern
/\r?\n/correctly handles both Unix (\n) and Windows (\r\n) line endings, preventing potential issues where Windows files would leave carriage return characters in parsed values.
- First remove expired entries (outside cooldown period) - If still over 100 entries, remove oldest by insertion order - Ensures Map never exceeds 100 entries even during heavy load Signed-off-by: StillKnotKnown <[email protected]>
StillKnotKnown
added
bug
AndyMik90
reviewed
Jan 11, 2026
Owner
AndyMik90
left a comment
AndyMik90
left a comment
There was a problem hiding this comment.
🤖 Auto Claude PR Review
🟠 Follow-up Review: Needs Revision
🟠 Needs revision - 2 unresolved finding(s) from previous review.
Resolution Status
- ✅ Resolved: 2 previous findings addressed
- ❌ Unresolved: 2 previous findings remain
- 🆕 New Issues: 2 new findings in recent changes
Finding Validation
- 🔍 Dismissed as False Positives: 0 findings were re-investigated and found to be incorrect
- ✓ Confirmed Valid: 2 findings verified as genuine issues
- 👤 Needs Human Review: 0 findings require manual verification
🚨 Blocking Issues
- quality: [UNRESOLVED] Incomplete refactoring - insights-handlers.ts still uses direct webContents.send
- test: Missing test coverage for pruning logic and memory cap enforcement
Verdict
CI Status: ✅ All 16 checks passing - CI does not block this PR.
2 of 4 previous findings resolved:
- ✅ setInterval replaced with timestamp-based cooldown (a9e76ced8d08)
- ✅ Duplicated warning pattern refactored into helper functions (072ec682df76)
2 findings remain unresolved (confirmed valid):
- 🔴 [MEDIUM] insights-handlers.ts still uses direct webContents.send instead of safeSendToRenderer (654dfde0668a) - This is the same pattern being fixed elsewhere, should be included for consistency
- 🟡 [LOW] Tests don't verify console.warn suppression (330188f49065) - The cooldown behavior is the core feature but isn't tested
1 new medium finding:
- 🔴 [MEDIUM] No tests for the new 100-entry pruning logic (NEW-001) - Important memory management code without test coverage
Action required: Apply safeSendToRenderer to insights-handlers.ts to complete the refactoring. Consider adding tests for the pruning logic and console.warn suppression to ensure the cooldown mechanism works as intended.
Review Process
Agents invoked: resolution-verifier, new-code-reviewer, finding-validator
This is an AI-generated follow-up review using parallel specialist analysis with finding validation.
Findings (4 selected of 4 total)
🟡 [654dfde0668a] [MEDIUM] [UNRESOLVED] Incomplete refactoring - insights-handlers.ts still uses direct webContents.send
📁 apps/frontend/src/main/ipc-handlers/insights-handlers.ts:284
The safeSendToRenderer utility was applied to agent-events-handlers.ts, ideation-handlers.ts, and roadmap-handlers.ts, but insights-handlers.ts (which has high-frequency streaming events like 'stream-chunk') still uses direct mainWindow.webContents.send() calls. These streaming events fire frequently and are susceptible to the same 'Render frame was disposed' crash this PR aims to fix.
Resolution note: insightsService.on('stream-chunk', (projectId: string, chunk: unknown) => {
const mainWindow = getMainWindow();
if (mainWindow) {
mainWindow.webContents.send(IPC_CHANNELS.INSIGHTS_STREAM_CHUNK, projectId, chunk);
}
});
Suggested fix:
Add import { safeSendToRenderer } from './utils'; and refactor event handlers to use safeSendToRenderer(getMainWindow, channel, ...args) pattern like the other handlers
🔵 [330188f49065] [LOW] [UNRESOLVED] Warning cooldown tests don't verify log suppression
📁 apps/frontend/src/main/__tests__/utils.test.ts:191
The 'warning cooldown behavior' tests verify that safeSendToRenderer returns false for destroyed windows, but they don't verify that console.warn is called only once (first time) and suppressed for subsequent calls. The core functionality of the cooldown mechanism (log spam prevention) is not explicitly tested.
Resolution note: it("returns false for multiple consecutive calls to destroyed windows", () => {
// ... setup ...
const result1 = safeSendToRenderer(getMainWindow, "test-channel", "data1");
const result2 = safeSendToRenderer(getMainWindow, "test-channel", "data2");
expect(result1).toBe(false);
expect(result2).toBe(false);
// No console.warn verification
});
Suggested fix:
Add a spy on console.warn and verify it's called exactly once for multiple consecutive calls to the same destroyed channel
🟡 [NEW-001] [MEDIUM] Missing test coverage for pruning logic and memory cap enforcement
📁 apps/frontend/src/main/__tests__/utils.test.ts:191
The test file has tests for 'warning cooldown behavior' but does NOT test the hard cap of 100 entries pruning logic introduced in commit 7d0b964. The pruning logic at lines 31-53 of utils.ts is completely untested. This could hide bugs in the two-phase pruning approach (remove expired entries first, then remove oldest).
Suggested fix:
Add test cases: (1) adding 101+ unique channels triggers pruning, (2) expired entries are removed first, (3) Map never exceeds 100 entries after recordWarning(), (4) newest entries preserved over oldest.
🔵 [NEW-002] [LOW] Missing test coverage for parseEnvFile with Windows line endings
📁 apps/frontend/src/main/__tests__/utils.test.ts:1
Commit a6027f3 adds \r\n support to parseEnvFile() but there are no tests for parseEnvFile() at all in utils.test.ts. The file only tests safeSendToRenderer, leaving parseEnvFile completely untested including the new Windows line ending support.
Suggested fix:
Add tests for parseEnvFile covering: Unix line endings, Windows line endings, mixed endings, empty lines, comments, and quoted values.
This review was generated by Auto Claude.
- Add _clearWarnTimestampsForTest() helper to clear module-level Map - Call clear in parent beforeEach to ensure clean state for each test - Simplify pruning tests to avoid complex cooldown timing issues - All 28 tests now pass
AndyMik90
reviewed
Jan 11, 2026
Owner
AndyMik90
left a comment
AndyMik90
left a comment
There was a problem hiding this comment.
🤖 Auto Claude PR Review
🟠 Follow-up Review: Needs Revision
🟠 Needs revision - 4 unresolved finding(s) from previous review.
Resolution Status
- ✅ Resolved: 0 previous findings addressed
- ❌ Unresolved: 4 previous findings remain
- 🆕 New Issues: 2 new findings in recent changes
Finding Validation
- 🔍 Dismissed as False Positives: 0 findings were re-investigated and found to be incorrect
- ✓ Confirmed Valid: 4 findings verified as genuine issues
- 👤 Needs Human Review: 0 findings require manual verification
🚨 Blocking Issues
- quality: [UNRESOLVED] [UNRESOLVED] Incomplete refactoring - insights-handlers.ts still uses direct webContents.send
- test: [UNRESOLVED] Missing test coverage for pruning logic and memory cap enforcement
- quality: Inconsistent safeSendToRenderer usage in generation-handlers.ts
Verdict
CI Status: ✅ All 15 checks passing - not blocking.
Blocking Issues (MEDIUM severity, confirmed valid):
- 654dfde0668a - insights-handlers.ts still uses 6 direct webContents.send() calls instead of safeSendToRenderer. This is the core bug the PR aims to fix, but this file was missed.
- NEW-003 - generation-handlers.ts also has 5 direct webContents.send() calls, creating additional exposure to the crash bug.
Non-blocking Issues (LOW severity):
- 330188f49065 - Warning cooldown tests don't verify console.warn suppression
- NEW-001 - No tests for pruning logic (100-entry cap)
- NEW-002 - No tests for parseEnvFile() including Windows line endings
- NEW-004 - Missing vi.resetModules() in test setup
Summary: The PR correctly refactored agent-events-handlers.ts, ideation-handlers.ts, and roadmap-handlers.ts to use safeSendToRenderer. However, insights-handlers.ts and generation-handlers.ts were missed and still have direct webContents.send() calls vulnerable to the 'Render frame was disposed' crash. These MEDIUM severity incomplete fix issues require revision before merge.
Review Process
Agents invoked: resolution-verifier, new-code-reviewer, finding-validator
This is an AI-generated follow-up review using parallel specialist analysis with finding validation.
Findings (6 selected of 6 total)
🟡 [654dfde0668a] [MEDIUM] [UNRESOLVED] [UNRESOLVED] Incomplete refactoring - insights-handlers.ts still uses direct webContents.send
📁 apps/frontend/src/main/ipc-handlers/insights-handlers.ts:284
The safeSendToRenderer utility was applied to agent-events-handlers.ts, ideation-handlers.ts, and roadmap-handlers.ts, but insights-handlers.ts (which has high-frequency streaming events like 'stream-chunk') still uses direct mainWindow.webContents.send() calls. These streaming events fire frequently and are susceptible to the same 'Render frame was disposed' crash this PR aims to fix.
Resolution note: insightsService.on('stream-chunk', (projectId: string, chunk: unknown) => {
const mainWindow = getMainWindow();
if (mainWindow) {
mainWindow.webContents.send(IPC_CHANNELS.INSIGHTS_STREAM_CHUNK, projectId, chunk);
}
});
Resolution note: mainWindow.webContents.send(IPC_CHANNELS.INSIGHTS_ERROR, projectId, 'Project not found'); // Line 40
mainWindow.webContents.send(IPC_CHANNELS.INSIGHTS_STREAM_CHUNK, projectId, chunk); // Line 287
Suggested fix:
Add import { safeSendToRenderer } from './utils'; and refactor event handlers to use safeSendToRenderer(getMainWindow, channel, ...args) pattern like the other handlers
🔵 [330188f49065] [LOW] [UNRESOLVED] [UNRESOLVED] Warning cooldown tests don't verify log suppression
📁 apps/frontend/src/main/__tests__/utils.test.ts:191
The 'warning cooldown behavior' tests verify that safeSendToRenderer returns false for destroyed windows, but they don't verify that console.warn is called only once (first time) and suppressed for subsequent calls. The core functionality of the cooldown mechanism (log spam prevention) is not explicitly tested.
Resolution note: it("returns false for multiple consecutive calls to destroyed windows", () => {
// ... setup ...
const result1 = safeSendToRenderer(getMainWindow, "test-channel", "data1");
const result2 = safeSendToRenderer(getMainWindow, "test-channel", "data2");
expect(result1).toBe(false);
expect(result2).toBe(false);
// No console.warn verification
});
Resolution note: expect(result1).toBe(false);
expect(result2).toBe(false);
expect(result3).toBe(false);
// No vi.spyOn(console, 'warn') to verify log suppression
Suggested fix:
Add a spy on console.warn and verify it's called exactly once for multiple consecutive calls to the same destroyed channel
🟡 [NEW-001] [MEDIUM] [UNRESOLVED] Missing test coverage for pruning logic and memory cap enforcement
📁 apps/frontend/src/main/__tests__/utils.test.ts:191
The test file has tests for 'warning cooldown behavior' but does NOT test the hard cap of 100 entries pruning logic introduced in commit 7d0b964. The pruning logic at lines 31-53 of utils.ts is completely untested. This could hide bugs in the two-phase pruning approach (remove expired entries first, then remove oldest).
Resolution note: // utils.test.ts has 253 lines testing safeSendToRenderer
// No tests for recordWarning() function or pruning logic at utils.ts lines 28-54
Suggested fix:
Add test cases: (1) adding 101+ unique channels triggers pruning, (2) expired entries are removed first, (3) Map never exceeds 100 entries after recordWarning(), (4) newest entries preserved over oldest.
🔵 [NEW-002] [LOW] [UNRESOLVED] Missing test coverage for parseEnvFile with Windows line endings
📁 apps/frontend/src/main/__tests__/utils.test.ts:1
Commit a6027f3 adds \r\n support to parseEnvFile() but there are no tests for parseEnvFile() at all in utils.test.ts. The file only tests safeSendToRenderer, leaving parseEnvFile completely untested including the new Windows line ending support.
Resolution note: // grep 'parseEnvFile' in tests returns no matches
// utils.test.ts only tests safeSendToRenderer
Suggested fix:
Add tests for parseEnvFile covering: Unix line endings, Windows line endings, mixed endings, empty lines, comments, and quoted values.
🟡 [NEW-003] [MEDIUM] Inconsistent safeSendToRenderer usage in generation-handlers.ts
📁 apps/frontend/src/main/ipc-handlers/ideation/generation-handlers.ts:77
The ideation/generation-handlers.ts file still uses direct webContents.send() calls instead of safeSendToRenderer. While ideation-handlers.ts correctly uses safeSendToRenderer for agent event forwarding, the underlying generation-handlers.ts file has 5 direct webContents.send() calls that bypass the safe wrapper. This creates inconsistency where some ideation IPC messages could still trigger 'Render frame was disposed' errors.
Suggested fix:
Import safeSendToRenderer from '../utils' and replace all direct webContents.send() calls with safeSendToRenderer(getMainWindow, ...).
🔵 [NEW-004] [LOW] Module state isolation issue in test setup
📁 apps/frontend/src/main/__tests__/utils.test.ts:18
The test uses dynamic import to get a fresh module state for each test, but vi.resetModules() is not called before the import. The warnTimestamps Map state may persist between test cases, potentially causing flaky tests when testing cooldown behavior.
Suggested fix:
Add vi.resetModules() before the dynamic import in beforeEach to ensure each test gets a fresh module state.
This review was generated by Auto Claude.
Addresses finding NEW-003 from follow-up review: - Import safeSendToRenderer from '../utils' - Replace all 5 direct webContents.send() calls with safeSendToRenderer - Add getMainWindow wrapper for each function This ensures ideation generation IPC messages are protected from "Render frame was disposed" crashes. Related: ACS-211
AndyMik90
approved these changes
Jan 12, 2026
VDT-91
added a commit
to VDT-91/Auto-Claude
that referenced
this pull request
Jan 12, 2026
…ndyMik90#918) * fix(frontend): prevent "Render frame was disposed" crash (ACS-211) Add safeSendToRenderer helper that validates frame state before IPC sends. Prevents app crash when renderer frames are disposed during heavy agent output. Fixes AndyMik90#211 Signed-off-by: StillKnotKnown <[email protected]> * refactor(utils): replace setInterval with timestamp-based cooldown, add \r\n support - Replace setInterval-based cooldown with timestamp Map approach to avoid timer leaks - Add isWithinCooldown() and recordWarning() helper functions - Optionally prune old entries when Map exceeds 100 entries - Update parseEnvFile to handle Windows \r\n line endings with /\r?\n/ regex Signed-off-by: StillKnotKnown <[email protected]> * refactor(utils): enforce hard cap of 100 entries in pruning logic - First remove expired entries (outside cooldown period) - If still over 100 entries, remove oldest by insertion order - Ensures Map never exceeds 100 entries even during heavy load Signed-off-by: StillKnotKnown <[email protected]> * test: fix module-level state issue in pruning tests - Add _clearWarnTimestampsForTest() helper to clear module-level Map - Call clear in parent beforeEach to ensure clean state for each test - Simplify pruning tests to avoid complex cooldown timing issues - All 28 tests now pass * fix: update generation-handlers.ts to use safeSendToRenderer Addresses finding NEW-003 from follow-up review: - Import safeSendToRenderer from '../utils' - Replace all 5 direct webContents.send() calls with safeSendToRenderer - Add getMainWindow wrapper for each function This ensures ideation generation IPC messages are protected from "Render frame was disposed" crashes. Related: ACS-211 --------- Signed-off-by: StillKnotKnown <[email protected]> Co-authored-by: StillKnotKnown <[email protected]>
This was referenced Jan 12, 2026
aleksanderem
added a commit
to aleksanderem/Auto-Claude
that referenced
this pull request
Jan 13, 2026
… verification (#3) * Fix/2.7.2 hotfixes (#577) * feat(terminal): respect preferred terminal setting for Windows PTY shell Adds Windows shell selection in the embedded PTY terminal based on the user's preferredTerminal setting from onboarding/settings. On Windows, the terminal preference (PowerShell, Windows Terminal, CMD) now maps to the appropriate shell executable when spawning PTY processes. This ensures the embedded terminal matches user expectations when they select their preferred terminal during setup. - Adds WINDOWS_SHELL_PATHS mapping for powershell, windowsterminal, cmd - Implements getWindowsShell() to find first available shell executable - Falls back to COMSPEC/cmd.exe for 'system' or unknown terminals - Reads preferredTerminal from user settings on each spawn * fix(ci): cache pip wheels to speed up Intel Mac builds The real_ladybug package has no pre-built wheel for macOS x86_64 (Intel), requiring Rust compilation from source on every build. This caused builds to take 5-10+ minutes. Changes: - Remove --no-cache-dir from pip install so wheels get cached - Add pip wheel cache to GitHub Actions cache for all platforms - Include requirements.txt hash in cache keys for proper invalidation - Fix restore-keys to avoid falling back to incompatible old caches After this fix, subsequent Intel Mac builds will use the cached compiled wheel instead of rebuilding from source each time. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]> * # 🔥 hotfix(electron): restore app functionality on Windows broken by GPU cache errors (#569) ## 📋 Critical Issue | Severity | Impact | Affected Users | |----------|--------|----------------| | 🔴 **CRITICAL** | 🚫 **Non-functional** | 🪟 **Windows users** | On Windows systems, the Electron app failed to create GPU shader and program caches due to filesystem permission errors (**Error 0x5: Access Denied**). This prevented users from initiating the autonomous coding phase, rendering the application **non-functional** for its primary purpose. --- ## 🔍 Root Cause Analysis ### The Problem Chromium's GPU process attempts to create persistent shader caches in the following locations: %LOCALAPPDATA%\auto-claude-ui\GPUCache\ %LOCALAPPDATA%\auto-claude-ui\ShaderCache\ ### Why It Fails | Factor | Description | |--------|-------------| | 🦠 **Antivirus** | Real-time scanning blocks cache directory creation | | 🛡️ **Windows Defender** | Protection policies deny write access | | ☁️ **Sync Software** | OneDrive/Dropbox interferes with AppData folders | | 🔐 **Permissions** | Insufficient rights in default Electron cache paths | ### Error Console Output ❌ ERROR:net\disk_cache\cache_util_win.cc:25] Unable to move the cache: Zugriff verweigert (0x5) ❌ ERROR:gpu\ipc\host\gpu_disk_cache.cc:724] Gpu Cache Creation failed: -2 ❌ ERROR:net\disk_cache\disk_cache.cc:236] Unable to create cache --- ## ✅ Solution Implemented ### 1️⃣ GPU Shader Disk Cache Disabled app.commandLine.appendSwitch('disable-gpu-shader-disk-cache'); - ⚡ Prevents Chromium from writing shader caches to disk - ✅ GPU acceleration remains fully functional - 🎯 Zero performance impact on typical usage ### 2️⃣ GPU Program Disk Cache Disabled app.commandLine.appendSwitch('disable-gpu-program-cache'); - 🚫 Prevents compiled GPU program caching issues - 🔒 Eliminates permission-related failures ### 3️⃣ Startup Cache Clearing session.defaultSession.clearCache() .then(() => console.log('[main] Cleared cache on startup')) .catch((err) => console.warn('[main] Failed to clear cache:', err)); - 🧹 Clears stale session cache on initialization - 🔧 Prevents errors from corrupted cache artifacts - ⚠️ Includes error handling for robustness --- ## 📝 Technical Changes ### Files Modified | File | Changes | |------|---------| | apps/frontend/src/main/index.ts | +13 lines (cache fixes) | ### Platform Gating ✅ **Windows Only** (process.platform === 'win32') ✅ macOS & Linux behavior unchanged --- ## 🎯 Impact Assessment | Aspect | Status | Details | |--------|--------|---------| | 🎮 **GPU Acceleration** | ✅ **PRESERVED** | Hardware rendering fully functional | | 🤖 **Agent Functionality** | ✅ **RESTORED** | Coding phase now works on Windows | | 🖥️ **Console Errors** | ✅ **ELIMINATED** | Clean startup on all Windows systems | | ⚡ **Performance** | ✅ **NO IMPACT** | Typical usage unaffected | | 🔙 **Compatibility** | ✅ **MAINTAINED** | No breaking changes | --- ## 🧪 Testing ### Test Environments | Platform | Antivirus | Result | |----------|-----------|--------| | Windows 10 | Windows Defender | ✅ Pass | | Windows 11 | Real-time scanning | ✅ Pass | ### Test Scenarios ✅ Application starts without cache errors ✅ Agent initialization completes successfully ✅ Coding phase executes without GPU failures ✅ GPU acceleration functional (hardware rendering active) --- ## 📦 Meta Information | Field | Value | |-------|-------| | 📍 **Component** | apps/frontend/src/main/index.ts | | 🪟 **Platform** | Windows (win32) - platform-gated | | 🔥 **Type** | Hotfix (critical functionality restoration) | --- ## 🔄 Backwards Compatibility | Check | Status | |-------|--------| | Breaking Changes | ❌ None | | User Data Migration | ❌ Not required | | Settings Impact | ❌ Unaffected | | Workflow Changes | ❌ None required | --- *This hotfix restores critical functionality for Windows users while maintaining full compatibility with macOS and Linux platforms. GPU acceleration remains fully functional — only disk-based caching is disabled.* Co-authored-by: sniggl <[email protected]> * ci(release): add CHANGELOG.md validation and fix release workflow The release workflow was failing with "GitHub Releases requires a tag" when triggered via workflow_dispatch because no tag existed. Changes: - prepare-release.yml: Validates CHANGELOG.md has entry for version BEFORE creating tag (fails early with clear error message) - release.yml: Uses CHANGELOG.md content instead of release-drafter for release notes; fixes workflow_dispatch to be dry-run only - bump-version.js: Warns if CHANGELOG.md missing entry for new version - RELEASE.md: Updated documentation for new changelog-first workflow This ensures releases are only created when CHANGELOG.md is properly updated, preventing incomplete releases and giving better release notes. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]> * fix(merge): handle Windows CRLF line endings in regex fallback The merge conflict layer was failing on Windows when tree-sitter was unavailable. The regex-based fallback used split("\n") which doesn't handle CRLF line endings, and findall() returned tuples for JS/TS patterns breaking function detection. Changes: - Normalize line endings (CRLF → LF) before parsing in regex_analyzer.py - Use splitlines() instead of split("\n") in file_merger.py - Fix tuple extraction from findall() for JS/TS function patterns - Normalize line endings before tree-sitter parsing for consistent byte positions All 111 merge tests pass. These changes are cross-platform safe and maintain compatibility with macOS and Linux. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]> * 2.7.2 release --------- Co-authored-by: Claude Opus 4.5 <[email protected]> Co-authored-by: sniggl <[email protected]> Co-authored-by: sniggl <[email protected]> * ci: force workflow re-index for tag trigger * fix(ci): fix YAML syntax error in release.yml multiline string * docs: update stable download links to v2.7.2 (#579) * Fix Windows UTF-8 encoding errors in spec file reading (#744) * Fix Windows UTF-8 encoding errors in spec file reading Fixes UnicodeDecodeError on Windows when reading spec files with non-ASCII characters. Python's read_text() defaults to cp1252 on Windows instead of UTF-8, causing crashes when spec.md contains Unicode characters. Changes: - spec_document_validator.py: Add encoding='utf-8' to spec.md reading - compaction.py: Add encoding='utf-8' to phase output file reading - validation_strategy.py: Add encoding='utf-8' to requirements/pyproject reading - agent_runner.py: Add encoding='utf-8' to prompt file reading Impact: - Fixes spec generation crashes on Windows with Unicode content - No impact on Unix/Mac (already default to UTF-8) - Prevents charmap codec errors during spec validation Tested on Windows 10 with Auto-Claude v2.7.3 * Add error handling for file read operations Implements Gemini Code Assist review suggestions to improve robustness: 1. spec_document_validator.py: - Wrap read_text() in try-except to handle corrupted UTF-8 files - Provide clear error message when spec.md is unreadable 2. validation_strategy.py: - Silently skip unreadable requirements.txt/pyproject.toml files - Prevents crashes during project type detection This defensive programming prevents crashes from: - Corrupted or invalid UTF-8 files - Permission errors (OSError) - Other file system issues Addresses code review feedback from @gemini-code-assist * Fix Windows UTF-8 encoding errors in project analyzer - Add encoding='utf-8' to file reads in analyzer.py (load/save profile) - Add encoding='utf-8' to file reads in config_parser.py (read_json, read_text) - Add encoding='utf-8' to file reads in stack_detector.py (YAML files) This fixes the 'charmap' codec error that was blocking roadmap generation on Windows: Warning: Could not load security profile: 'charmap' codec can't decode byte 0x8d Same root cause as spec file reading - Windows defaults to cp1252 instead of UTF-8. * Revert "Fix Windows UTF-8 encoding errors in project analyzer" This reverts commit 41deed0aa388d8599371db16f822d9278464f09b. --------- Co-authored-by: TamerineSky <[email protected]> * Revert "Fix Windows UTF-8 encoding errors in spec file reading (#744)" (#770) This reverts commit a01634d54809e9f4f1d6507d5bcbf55fdff68981. * feat: Add workspace mode preference (ISOLATED/DIRECT) - Frontend: Add workspace mode setting in General Settings UI with dropdown - Frontend: Map workspaceMode TypeScript field to WORKSPACE_MODE env variable - Frontend: Read WORKSPACE_MODE from .env back to workspaceMode for UI - Backend: Read WORKSPACE_MODE from project .env in choose_workspace() - Backend: Respect user preference before defaulting to safety checks This allows users to configure their preferred workspace mode (isolated worktree vs direct) through the UI, persisted to .auto-claude/.env, and respected by the backend agent. * feat: Add Playwright screenshot collection after QA sessions Backend changes: - Add collect_qa_screenshots() to scan for recent .png files after QA - Add save_qa_screenshots_to_plan() to save paths to implementation_plan.json - Call screenshot collection in QA reviewer after session completes - Screenshots are saved to qa_signoff.screenshots field Frontend changes: - Add screenshots?: string[] field to QAReport TypeScript type This enables capturing and storing Playwright screenshots during QA validation. Next step: Map screenshots to Task.qaReport and display in UI. * feat: Map qa_signoff to Task.qaReport with screenshots - Import QAReport type in project-store - Extract qa_signoff from implementation_plan.json when loading tasks - Map qa_signoff.status to qaReport.status (approved -> passed, rejected -> failed) - Map qa_signoff.issues_found to qaReport.issues array - Map qa_signoff.screenshots to qaReport.screenshots array - Convert timestamp to Date object This enables frontend to display QA results and Playwright screenshots in task UI. * feat: Enable Claude to see and verify Playwright screenshots during QA CRITICAL CHANGE: QA agent can now visually verify screenshots Backend changes: - playwright/executor.py: Return base64-encoded screenshot in tool response - playwright/sdk_integration.py: Format screenshot as image content block for Claude - qa_reviewer.md prompt: Explicit instructions to ANALYZE visual content How it works now: 1. QA agent uses playwright_screenshot tool 2. Tool returns base64-encoded image + metadata 3. SDK formats it as image content block 4. Claude SEES the screenshot and can analyze visual content 5. If Claude detects visual bugs → reports them as issues 6. QA fixer fixes the issues 7. Loop continues until approved Before this change: Screenshots were taken but Claude never saw them After this change: Claude can verify UI matches acceptance criteria visually This closes the verification loop - screenshots are not just captured for later viewing, but actively analyzed by the QA agent during validation. * feat: add screenshot support to QA feedback Users can now paste/drag screenshots in QA feedback textarea to show visual problems. Frontend changes: - QAFeedbackSection: Added image paste/drag-drop with useImageUpload hook - Shows image previews with thumbnails and count indicator - Visual feedback on paste success (green border flash) - Added feedbackImages state to useTaskDetail hook - Updated submitReview to accept ImageAttachment[] parameter Backend changes: - IPC handler saves images to qa-feedback-screenshots/ directory - References screenshots in QA_FIX_REQUEST.md with instructions - QA fixer loads and displays images as visual content blocks - Claude sees screenshots and can analyze visual problems Complete flow: 1. User pastes screenshot (Cmd+V or drag-drop) 2. Image saved as base64 ImageAttachment 3. On "Request Changes", sent via IPC to backend 4. Backend saves to qa-feedback-screenshots/ 5. QA_FIX_REQUEST.md lists screenshot paths 6. QA fixer loads and sends as image blocks to Claude 7. Claude sees screenshots visually and fixes problems This mirrors the Playwright screenshot pattern - Claude gets visual context, not just file paths. * security: Add input validation and sanitization for screenshot uploads Address security concerns from code review: 1. Path Traversal Prevention: - Sanitize filenames with path.basename() - Validate only safe characters (alphanumeric, dash, underscore, dot) - Resolve and validate paths before reading screenshots 2. Resource Limits: - Enforce 10MB max size per image - Log rejected files with size/reason 3. Separation of Concerns: - Exclude user feedback screenshots from agent screenshot collection - Only collect Playwright/agent-generated screenshots in collect_qa_screenshots() 4. UX Improvement: - Allow 'Request Changes' with images only (no text required) These changes prevent: - Directory traversal attacks (../../etc/passwd) - Arbitrary file overwrites - Disk exhaustion via unbounded uploads - Mixing user and agent screenshots * fix: Include user feedback text with screenshots for full context Extract feedback text from QA_FIX_REQUEST.md and include it in the content blocks sent to Claude along with the screenshots. This ensures the QA fixer sees both the text description and visual context together. Changes: - Modified load_qa_feedback_screenshots() to return tuple of (feedback_text, image_blocks) - Extract feedback section from markdown with regex - Include feedback text in prompt before image blocks - Provide clear context that screenshots show visual problems * feat: Update app icon to new design - Converted WebP images from appiconset to PNG format - Generated new icon.icns using iconutil - Updated icon.png (1024x1024) and icon-256.png - Packaged and verified new icon in Mac build * feat: Display QA report screenshots in task detail UI - Created QAReportSection component to display Playwright screenshots - Shows screenshots in grid with zoom functionality - Positioned between workspace status and QA feedback section - Screenshots are clickable to view full-size in dialog - Only displays when QA has run and screenshots exist * feat: Add Playwright headless/headed mode toggle - Added playwrightHeadless setting to ProjectEnvConfig (default: true) - Backend: Read PLAYWRIGHT_HEADLESS env var in executor (default: true) - Frontend: Added UI toggle in project settings Workspace section - .env file integration for PLAYWRIGHT_HEADLESS setting - Allows users to run Playwright in headed mode for debugging * fix: Add ImageAttachment parameter to submitReview type - Added ImageAttachment import to ipc.ts - Updated submitReview signature to include images parameter - Fixes TypeScript compilation error * feat: implement automatic MCP tool discovery via SERVER_TOOL_REGISTRY Replaced hardcoded if/elif chains in _get_mcp_tools_for_servers() with a registry pattern for automatic tool discovery. Changes: - Added SERVER_TOOL_REGISTRY dict mapping server names to tool lists - _get_mcp_tools_for_servers() now uses simple dict lookup - Exported SERVER_TOOL_REGISTRY for external use - Added ADDING_MCP_SERVERS.md documentation Benefits: - Adding new MCP servers requires just 2 steps (define tools, add to registry) - No more manual if/elif modification for each new server - DRY principle - tool mappings defined once - Easier maintenance and debugging All existing servers (context7, linear, graphiti, electron, puppeteer, playwright) work unchanged. Verified via comprehensive tests. Fixes the original issue where Playwright tools weren't automatically available when the server was added to agent configs. * fix(ACS-175): Resolve integrations freeze and improve rate limit handling (#839) * fix(ACS-175): Resolve integrations freeze and improve rate limit handling * fix(i18n): replace hardcoded toast strings with translation keys Addresses CodeRabbit review feedback on PR #839: - OAuthStep.tsx: use t() for all toast messages - RateLimitModal.tsx: use t() for toast messages - SDKRateLimitModal.tsx: add useTranslation hook, use t() for toasts - Add toast translation keys to en/fr onboarding.json and common.json * fix: replace console.log with debugLog in IntegrationSettings Addresses CodeRabbit review feedback - use project's debug logging utility for consistent and toggle-able logging in production. * fix: replace console.log with debugLog in main process files Addresses Auto-Claude PR Review feedback: - terminal-handlers.ts: 14 console.log → debugLog - pty-manager.ts: 10 console.log → debugLog/debugError - terminal-manager.ts: 4 console.log → debugLog/debugError Also fixes: - Extract magic numbers to CHUNKED_WRITE_THRESHOLD/CHUNK_SIZE constants - Add terminal validity check before chunked writes - Consistent error handling (no rethrow for fire-and-forget semantics) * fix: address Auto Claude PR review findings - console.error→debugError + i18n - Replace 8 remaining console.error/warn calls with debugError/debugLog: - terminal-handlers.ts: lines 59, 426, 660, 671 - terminal-manager.ts: lines 88, 320 - pty-manager.ts: lines 88, 141 - Fixed duplicate logging in exception handler - Add comprehensive i18n for SDKRateLimitModal.tsx (~20 strings): - Added rateLimit.sdk.* keys with swap notifications, buttons, labels - EN + FR translations in common.json - Add comprehensive i18n for OAuthStep.tsx (~15 strings): - Added oauth.badges.*, oauth.buttons.*, oauth.labels.* keys - EN + FR translations in onboarding.json All MEDIUM severity findings resolved except race condition (deferred). * fix: address Auto Claude PR review findings - race condition + console.error - Fix race condition in chunked PTY writes by serializing writes per terminal using Promise chaining (prevents interleaving of concurrent large writes) - Fix missing 't' in useEffect dependency array in OAuthStep.tsx - Convert all remaining console.error calls to debugError for consistency: - IntegrationSettings.tsx (9 occurrences) - RateLimitModal.tsx (3 occurrences) - SDKRateLimitModal.tsx (4 occurrences) Co-Authored-By: Claude Opus 4.5 <[email protected]> Signed-off-by: Black Circle Sentinel <[email protected]> --------- Signed-off-by: Black Circle Sentinel <[email protected]> Co-authored-by: Andy <[email protected]> Co-authored-by: Adam Slaker <[email protected]> Co-authored-by: Claude Opus 4.5 <[email protected]> * fix: Playwright headed mode by default + enforce relative screenshot paths Changes: 1. Headless mode: - Changed default from headless=true to headless=false (HEADED/visible) - Browser now opens visibly by default for debugging - Set PLAYWRIGHT_HEADLESS=true in .env to run headless 2. Screenshot paths: - Added prominent warnings in QA prompt about using relative paths - Updated tool description to enforce relative paths - Updated tool input schema description with clear warning - Relative paths are saved in spec dir for persistence - Absolute paths in /tmp are lost after cleanup Why: - User reported browser wasn't visible during QA testing - Screenshto zapisywał się w /tmp i znikał po cleanup - Agent needs clear instruction to use relative paths only Now: - Browser opens visibly for easy debugging - Screenshots persist in spec directory - Clear warnings prevent agent from using /tmp paths * fix: Load PLAYWRIGHT_HEADLESS from .env dynamically in QA loop Problem: When user changes PLAYWRIGHT_HEADLESS in UI, backend doesn't see it because os.environ is static (loaded once at process start). Solution: 1. Added PLAYWRIGHT_HEADLESS to mcp_keys in load_project_mcp_config() 2. QA loop now loads mcp_config and sets os.environ["PLAYWRIGHT_HEADLESS"] BEFORE creating Playwright browser 3. This allows UI changes to take effect immediately without backend restart Flow: - User changes setting in UI → saved to .auto-claude/.env - QA loop starts → loads .env file dynamically - Sets os.environ["PLAYWRIGHT_HEADLESS"] = value from .env - Playwright executor reads os.environ → uses fresh value Result: UI changes to headed/headless mode now work immediately! * docs: Add AUTO-CLAUDE BUILT-IN CAPABILITIES section to planner prompt Problem: Planner doesn't know about Auto-Claude's built-in tools and creates unnecessary subtasks to install them (e.g., 'npx playwright'). Solution: Added prominent section at the top of planner.md explaining: - Playwright: Already installed, available to QA agents, don't use npx - Context7: Built-in docs lookup - Graphiti: Built-in knowledge graph memory - Linear: Built-in PM integration Also updated examples: - Changed 'npx playwright test' → 'QA agent with built-in Playwright' - Updated e2e_tests example to reference built-in Playwright Result: Planner now understands these tools exist and focuses subtasks on actual feature implementation, not on setting up Auto-Claude infrastructure. This prevents planner from: - Creating 'install Playwright' subtasks - Trying to use 'npx playwright' commands - Assuming tools need to be configured in the project * fix: Add mcp__playwright__ prefix to Playwright tool names Problem: Agent tries to use 'mcp__playwright__playwright_navigate' but gets permission error because PLAYWRIGHT_TOOLS had names without MCP prefix. Root cause: Playwright was converted to SDK MCP server (commit dd108b9) but tool names in PLAYWRIGHT_TOOLS weren't updated to include the mcp__playwright__ prefix that SDK automatically adds. Comparison: - Context7: mcp__context7__resolve-library-id ✓ - Linear: mcp__linear-server__list_teams ✓ - Playwright: playwright_navigate ✗ (missing prefix!) Solution: Updated all PLAYWRIGHT_TOOLS to include mcp__playwright__ prefix: - playwright_navigate → mcp__playwright__playwright_navigate - playwright_screenshot → mcp__playwright__playwright_screenshot - etc. This matches the naming convention used by all other MCP servers. Result: QA agent can now actually use Playwright tools without permission errors. * feat: Use wildcard MCP permissions for automatic tool discovery Problem: Manual tool name mapping required in two places: 1. TOOL_NAME constants (e.g., PLAYWRIGHT_TOOLS list) 2. Security permissions (hardcoded if/else for each server) This caused permission errors when: - Tool names didn't match exactly - New tools were added to MCP servers - Servers were renamed or updated Solution: Wildcard MCP permissions! Before (MANUAL): - List every tool: CONTEXT7_TOOLS, LINEAR_TOOLS, etc. - Add permissions for each tool individually - Update both places when adding new tools After (AUTOMATIC): - If server in required_servers → grant mcp__<server>__*(*) permission - All tools from that server work immediately - No manual tool name listing needed Changes: 1. Replaced individual tool permissions with wildcard pattern 2. Removed browser_tools_permissions logic (now automatic) 3. Removed unused imports (CONTEXT7_TOOLS, LINEAR_TOOLS, etc.) Example permissions generated: - mcp__context7__*(*) → all Context7 tools - mcp__linear__*(*) → all Linear tools - mcp__playwright__*(*) → all Playwright tools - mcp__graphiti__*(*) → all Graphiti tools Result: - ✅ Permission errors eliminated - ✅ New MCP tools work immediately - ✅ Cleaner code (removed 30+ lines of manual mapping) - ✅ True automatic tool discovery * fix(frontend): strip ANSI escape codes from roadmap/ideation progress messages (ACS-219) (#933) * fix(frontend): strip ANSI escape codes from roadmap/ideation progress messages (ACS-219) - Create ansi-sanitizer.ts utility with stripAnsiCodes() function - Apply to roadmap and ideation progress handlers in agent-queue.ts - Add 34 comprehensive test cases covering ANSI escape patterns - Fixes raw escape codes (e.g., \x1b[90m) displaying in UI * fix(frontend): extract first line before truncating roadmap progress messages Make roadmap progress message construction consistent with ideation by extracting the first line (split by newline) before applying the 200 character truncation. This ensures multi-line log output doesn't display partial lines in the UI. * refactor(frontend): extract repeated status message formatting to helper Add formatStatusMessage() helper function to centralize the sanitize+truncate pattern used in progress message handlers. This improves maintainability by: - Adding STATUS_MESSAGE_MAX_LENGTH constant (200) - Encapsulating ANSI stripping, line extraction, and truncation - Replacing 4 duplicated call sites with single helper Refactors lines 442, 461, 704, 718 to use formatStatusMessage(log). --------- Co-authored-by: StillKnotKnown <[email protected]> * fix(frontend): prevent "Render frame was disposed" crash (ACS-211) (#918) * fix(frontend): prevent "Render frame was disposed" crash (ACS-211) Add safeSendToRenderer helper that validates frame state before IPC sends. Prevents app crash when renderer frames are disposed during heavy agent output. Fixes #211 Signed-off-by: StillKnotKnown <[email protected]> * refactor(utils): replace setInterval with timestamp-based cooldown, add \r\n support - Replace setInterval-based cooldown with timestamp Map approach to avoid timer leaks - Add isWithinCooldown() and recordWarning() helper functions - Optionally prune old entries when Map exceeds 100 entries - Update parseEnvFile to handle Windows \r\n line endings with /\r?\n/ regex Signed-off-by: StillKnotKnown <[email protected]> * refactor(utils): enforce hard cap of 100 entries in pruning logic - First remove expired entries (outside cooldown period) - If still over 100 entries, remove oldest by insertion order - Ensures Map never exceeds 100 entries even during heavy load Signed-off-by: StillKnotKnown <[email protected]> * test: fix module-level state issue in pruning tests - Add _clearWarnTimestampsForTest() helper to clear module-level Map - Call clear in parent beforeEach to ensure clean state for each test - Simplify pruning tests to avoid complex cooldown timing issues - All 28 tests now pass * fix: update generation-handlers.ts to use safeSendToRenderer Addresses finding NEW-003 from follow-up review: - Import safeSendToRenderer from '../utils' - Replace all 5 direct webContents.send() calls with safeSendToRenderer - Add getMainWindow wrapper for each function This ensures ideation generation IPC messages are protected from "Render frame was disposed" crashes. Related: ACS-211 --------- Signed-off-by: StillKnotKnown <[email protected]> Co-authored-by: StillKnotKnown <[email protected]> * fix(core): implement atomic JSON writes to prevent file corruption (ACS-209) (#915) * fix(core): implement atomic JSON writes to prevent file corruption (ACS-209) - Add write_json_atomic() utility using temp file + os.replace() - Add async_save() to ImplementationPlan for non-blocking I/O - Update planner.py to use async_save() in async context - Add 'error' status to TaskStatus for corrupted files - Enhance ProjectStore error handling to surface parse errors - Add recovery CLI utility (cli/recovery.py) for detecting/fixing corrupted files This fixes JSON parse errors that caused tasks to be silently skipped when implementation_plan.json was corrupted during crashes/interrupts. * fix(i18n): add error column to task status constants and translations - Add 'error' to TASK_STATUS_COLUMNS array - Add 'error' label and color to TASK_STATUS_LABELS and TASK_STATUS_COLORS - Add 'columns.error' translation key to en/tasks.json and fr/tasks.json This fixes TypeScript errors in KanbanBoard.tsx where the Record type was missing the 'error' status that was added to TaskStatus type. * refactor: improve code quality and add i18n support for error messages Backend improvements: - Fix duplicate JSON check in recovery.py (remove redundant plan_file check) - Update find_specs_dir return type to Path (always returns valid path) - Replace deprecated asyncio.get_event_loop() with asyncio.get_running_loop() - Use functools.partial for cleaner async_save implementation Frontend improvements: - Add TaskErrorInfo type for structured error information - Update project-store.ts to use errorInfo for i18n-compatible error messages - Tighten typing of TASK_STATUS_LABELS and TASK_STATUS_COLORS to use TaskStatusColumn - Add error column to KanbanBoard grouped tasks initialization - Add en/fr errors.json translation files for parse error messages * docs: add errors.json to i18n translation namespaces - Document new errors.json namespace for error messages - Add example showing interpolation/substitution pattern for dynamic error content * refactor: typing improvements, i18n fixes, and error UX enhancements Backend typing: - Add explicit return type hint (-> None) to main() in recovery.py - Add explicit return type hint (-> None) to async_save() in plan.py - Make recovery.py exit with code 1 when corrupted files are detected Error handling improvements: - Include specId in errorInfo meta for better error context - Cap error message length at 500 characters to prevent bloat - Update error translation keys to include specId substitution Frontend improvements: - Conditionally add reviewReason/errorInfo to task objects only when defined - Add 'error' case to KanbanBoard empty state with AlertCircle icon - Fix hardcoded "Refreshing..."/"Refresh Tasks" strings to use i18n i18n additions: - Add emptyError/emptyErrorHint to en/fr tasks.json - Add refreshing/refreshTasks to en/fr translation files * refactor: code quality improvements - Remove error truncation in recovery.py (show full error for debugging) - Extract duplicate timestamp/status update logic to _update_timestamps_and_status() helper - Fix MD031 in CLAUDE.md (add blank line before fenced code block) * refactor: code quality improvements - Remove error truncation in recovery.py (show full error for debugging) - Extract duplicate timestamp/status update logic to _update_timestamps_and_status() helper - Fix MD031 in CLAUDE.md (add blank line before fenced code block) * refactor: naming and typing improvements - Update docstring examples in recovery.py (--fix -> --delete) - Rename delete_corrupted_file to backup_corrupted_file for clarity - Add return type annotation -> None to save() method * fix: add error status handling to TaskCard - Add 'error' case to getStatusBadgeVariant() returning 'destructive' - Add 'error' case to getStatusLabel() returning t('columns.error') - Start/stop buttons already exclude error tasks (backlog/in_progress only) * fix: address PR review feedback Backend changes: - recovery.py: Change [DELETE] to [BACKUP] in output message to match operation - recovery.py: Replace startswith with is_relative_to for proper path validation - recovery.py: Handle existing .corrupted backup files with unique timestamp suffix - file_utils.py: Add Iterator[IO[str]] return type annotation to atomic_write Frontend changes: - KanbanBoard.tsx: Use column-error CSS class instead of inline border-t-destructive - globals.css: Add .column-error rule with destructive color for consistency - tasks.json: Add top-level refreshTasks translation key for en/fr locales * fix: address follow-up review findings Backend changes: - file_utils.py: Handle binary mode correctly (encoding=None for 'b' in mode) - file_utils.py: Change return type to Iterator[IO] for broader type support - file_utils.py: Add docstring note about binary mode support - plan.py: Fix async_save to restore state on write failure (captures timestamps) Frontend changes: - project-store.ts: Add 'error' to statusMap to preserve error status - project-store.ts: Add early return for 'error' status like 'done'/'pr_created' - task-store.ts: Allow recovery from error status to backlog/in_progress - task-store.ts: Allow transitions to error without completion validation * fix: address follow-up review findings Backend changes: - file_utils.py: Handle binary mode correctly (encoding=None for 'b' in mode) - file_utils.py: Change return type to Iterator[IO] for broader type support - file_utils.py: Add docstring note about binary mode support - plan.py: Fix async_save to restore state on write failure (captures timestamps) Frontend changes: - project-store.ts: Add 'error' to statusMap to preserve error status - project-store.ts: Add early return for 'error' status like 'done'/'pr_created' - task-store.ts: Allow recovery from error status to backlog/in_progress - task-store.ts: Allow transitions to error without completion validation * fix: address third follow-up review findings Backend changes: - recovery.py: Change spec_dir.glob to spec_dir.rglob for recursive JSON scan - file_utils.py: Fix fd leak when os.fdopen raises (close fd and unlink tmp_path) - plan.py: Capture full state with to_dict() for robust rollback in async_save * fix: address final review quality suggestions Backend changes: - plan.py: Add NOTE comment about rollback fields maintenance - file_utils.py: Add logging.warning for temp file cleanup failures * fix: address final review quality suggestions Backend changes: - plan.py: Add NOTE comment about rollback fields maintenance - file_utils.py: Add logging.warning for temp file cleanup failures * fix: address final review quality suggestions Backend changes: - plan.py: Add NOTE comment about rollback fields maintenance - file_utils.py: Add logging.warning for temp file cleanup failures * fix: include stack trace in temp file cleanup failure logging Add exc_info=True to logging.warning call when temp file cleanup fails. This captures the full stack trace for better postmortem debugging of orphaned temp files. --------- Co-authored-by: StillKnotKnown <[email protected]> Co-authored-by: Andy <[email protected]> * fix(workspace): auto-rebase spec branch when behind before merge (#945) (#946) * fix(workspace): auto-rebase spec branch when behind before merge (ACS-224) When the "Merge with AI" button is clicked and the spec branch is behind the target branch, the system now automatically rebases before attempting to merge. Previously, the merge preview correctly detected the "behind" state but the actual merge process would fail with conflict errors. Changes: - Enhanced _check_git_conflicts() to detect needs_rebase and commits_behind - Added _rebase_spec_branch() to automatically rebase spec branch - Integrated rebase logic into _try_smart_merge_inner() before conflict resolution - Added 10 comprehensive tests for rebase detection and execution * fix(workspace): correct rebase invocation and use run_git helper Fixes issues in _rebase_spec_branch function: - Use run_git() instead of subprocess.run for allowlist compliance - Fix fragile git rebase arg order - use standard invocation - Remove misleading --strategy-option=theirs (not actually used) - Return False when conflicts abort (no ref movement occurred) - Verify branch actually moved after successful rebase Also updates test to check both .git/rebase-merge and .git/rebase-apply directories for robust git version compatibility. * fix: address PR review findings for rebase function Fixes 7 issues identified in PR review: HIGH: - Save and restore original branch after rebase (prevents leaving repo on spec branch) MEDIUM: - Return True when branch is already up-to-date (success, not failure) - Fix conflict detection to parse git status codes properly (not 'U' substring) - Check rebase abort result for inconsistent state LOW: - Remove unused variables git_dir and git_backup from test - Deduplicate git_conflicts.get('commits_behind', 0) call - Rename test to accurately describe what it tests * fix: address follow-up PR review findings Additional fixes from code review: workspace.py: - Remove duplicate "UD" from conflict status codes tuple - Add returncode check for original_branch_result with proper validation - Guard finally block restore with original_branch validity check - Replace subprocess.run with run_git for rev-list call - Add error logging when rev-list fails test_workspace.py: - Remove duplicate test test_rebase_handles_nonexistent_branch_gracefully (redundant with test_rebase_spec_branch_invalid_branch) - Strengthen merge assertion from "is not None" to "is True" * fix: replace remaining subprocess.run with run_git and add comprehensive tests workspace.py: - Replace all subprocess.run calls in _check_git_conflicts with run_git for consistent error handling, timeout handling, and centralized logging test_workspace.py: - Add test_rebase_spec_branch_already_up_to_date to verify function returns True when spec branch is already current - Add test_check_git_conflicts_handles_detached_head to verify graceful handling of detached HEAD state - Add test_check_git_conflicts_handles_corrupted_repo to verify graceful handling of corrupted .git directory with proper cleanup - Fix test_check_git_conflicts_no_commits_behind to checkout main before assertion (was comparing spec to itself, always returning 0) Test count: 42 -> 45 (+3 new tests) * fix: remove unused tempfile import from test * fix(workspace): final PR review fixes for ACS-224 rebase functionality This commit addresses the final batch of PR review findings for the rebase functionality added in ACS-224. All 45 tests pass. Changes: - NEW-001: Added branch restoration failure tracking and logging in finally block (noted limitation: cannot return False from finally block) - NEW-002: Added abort_failed tracking and checks before returning True to propagate abort failures to caller - NEW-004: Added state verification assertions to test_rebase_spec_branch_invalid_branch to verify current branch, no rebase state directories, and clean git status - LOGIC-002: Wrapped int() conversion in try-except to handle malformed rev-list output gracefully - LOGIC-003: Simplified redundant condition from checking both needs_rebase and commits_behind > 0 to just checking needs_rebase (which implies > 0) Files modified: - apps/backend/core/workspace.py: Added abort_failed tracking and error handling - tests/test_workspace.py: Added repo state verification assertions Related: ACS-224 * fix(workspace): fix CodeQL warnings for unreachable code and unused variable CodeQL detected unreachable code and unused variable issues in the rebase function. The abort_failed flag was only set when rebase failed, but was only checked in the success path (making it unreachable). Fixed by: - Removing abort_failed variable and its unreachable checks - Immediately returning False when abort fails (cannot safely continue) - Simplifying the finally block comment to reflect actual behavior All 45 tests pass. Related: ACS-224 --------- Co-authored-by: StillKnotKnown <[email protected]> * New/relase (#949) * chore: bump version to 2.7.3 * chore: update CHANGELOG for version 2.7.3, highlighting new features, improvements, and bug fixes --------- Co-authored-by: Test User <[email protected]> * fix: address CodeQL and PR review findings - Fix shell command injection vulnerability in cli-tool-manager.ts by using cmd.exe with argument array instead of string interpolation - Fix unused variables in test files (PRDetail.test.tsx, ReviewStatusTree.test.tsx) - Remove unused 'issues' destructuring in GitLabIssues.tsx - Fix unused 'merge_base' variable in workspace.py - Remove INVESTIGATION.md (temporary investigation document) Co-Authored-By: Claude Opus 4.5 <[email protected]> * Release v2.7.3 (#951) * docs: Add Git Flow branching strategy to CONTRIBUTING.md - Add comprehensive branching strategy documentation - Explain main, develop, feature, fix, release, and hotfix branches - Clarify that all PRs should target develop (not main) - Add release process documentation for maintainers - Update PR process to branch from develop - Expand table of contents with new sections * Feature/apps restructure v2.7.2 (#138) * refactor: restructure project to Apps/frontend and Apps/backend - Move auto-claude-ui to Apps/frontend with feature-based architecture - Move auto-claude to Apps/backend - Switch from pnpm to npm for frontend - Update Node.js requirement to v24.12.0 LTS - Add pre-commit hooks for lint, typecheck, and security audit - Add commit-msg hook for conventional commits - Fix CommonJS compatibility issues (postcss.config, postinstall scripts) - Update README with comprehensive setup and contribution guidelines - Configure ESLint to ignore .cjs files - 0 npm vulnerabilities Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com> * feat(refactor): clean code and move to npm * feat(refactor): clean code and move to npm * chore: update to v2.7.0, remove Docker deps (LadybugDB is embedded) * feat: v2.8.0 - update workflows and configs for Apps/ structure, npm * fix: resolve Python lint errors (F401, I001) * fix: update test paths for Apps/backend structure * fix: add missing facade files and update paths for Apps/backend structure - Fix ruff lint error I001 in auto_claude_tools.py - Create missing facade files to match upstream (agent, ci_discovery, critique, etc.) - Update test paths from auto-claude/ to Apps/backend/ - Update .pre-commit-config.yaml paths for Apps/ structure - Add pytest to pre-commit hooks (skip slow/integration/Windows-incompatible tests) - Fix Unicode encoding in test_agent_architecture.py for Windows Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com> * feat: improve readme * fix: new path * fix: correct release workflow and docs for Apps/ restructure - Fix ARM64 macOS build: pnpm → npm, auto-claude-ui → Apps/frontend - Fix artifact upload paths in release.yml - Update Node.js version to 24 for consistency - Update CLI-USAGE.md with Apps/backend paths - Update RELEASE.md with Apps/frontend/package.json paths 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]> * refactor: rename Apps/ to apps/ and fix backend path resolution - Rename Apps/ folder to apps/ for consistency with JS/Node conventions - Update all path references across CI/CD workflows, docs, and config files - Fix frontend Python path resolver to look for 'backend' instead of 'auto-claude' - Update path-resolver.ts to correctly find apps/backend in development mode This completes the Apps restructure from PR #122 and prepares for v2.8.0 release. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]> * fix(electron): correct preload script path from .js to .mjs electron-vite builds the preload script as ESM (index.mjs) but the main process was looking for CommonJS (index.js). This caused the preload to fail silently, making the app fall back to browser mock mode with fake data and non-functional IPC handlers. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]> * - Introduced `dev:debug` script to enable debugging during development. - Added `dev:mcp` script for running the frontend in MCP mode. These enhancements streamline the development process for frontend developers. * refactor(memory): make Graphiti memory mandatory and remove Docker dependency Memory is now a core component of Auto Claude rather than optional: - Python 3.12+ is required for the backend (not just memory layer) - Graphiti is enabled by default in .env.example - Removed all FalkorDB/Docker references (migrated to embedded LadybugDB) - Deleted guides/DOCKER-SETUP.md and docker-handlers.ts - Updated onboarding UI to remove "optional" language - Updated all documentation to reflect LadybugDB architecture 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]> * feat: add cross-platform Windows support for npm scripts - Add scripts/install-backend.js for cross-platform Python venv setup - Auto-detects Python 3.12 (py -3.12 on Windows, python3.12 on Unix) - Handles platform-specific venv paths - Add scripts/test-backend.js for cross-platform pytest execution - Update package.json to use Node.js scripts instead of shell commands - Update CONTRIBUTING.md with correct paths and instructions: - apps/backend/ and apps/frontend/ paths - Python 3.12 requirement (memory system now required) - Platform-specific install commands (winget, brew, apt) - npm instead of pnpm - Quick Start section with npm run install:all 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]> * remove doc * fix(frontend): correct Ollama detector script path after apps restructure The Ollama status check was failing because memory-handlers.ts was looking for ollama_model_detector.py at auto-claude/ but the script is now at apps/backend/ after the directory restructure. This caused "Ollama not running" to display even when Ollama was actually running and accessible. * chore: bump version to 2.7.2 Downgrade version from 2.8.0 to 2.7.2 as the Apps/ restructure is better suited as a patch release rather than a minor release. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]> * chore: update package-lock.json for Windows compatibility 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]> * docs(contributing): add hotfix workflow and update paths for apps/ structure Add Git Flow hotfix workflow documentation with step-by-step guide and ASCII diagram showing the branching strategy. Update all paths from auto-claude/auto-claude-ui to apps/backend/apps/frontend and migrate package manager references from pnpm to npm to match the new project structure. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]> * fix(ci): remove duplicate ARM64 build from Intel runner The Intel runner was building both x64 and arm64 architectures, while a separate ARM64 runner also builds arm64 natively. This caused duplicate ARM64 builds, wasting CI resources. Now each runner builds only its native architecture: - Intel runner: x64 only - ARM64 runner: arm64 only 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]> --------- Co-authored-by: Alex Madera <[email protected]> Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com> Co-authored-by: Claude Opus 4.5 <[email protected]> * Feat: Ollama download progress tracking with new apps structure (#141) * feat(ollama): add real-time download progress tracking for model downloads Implement comprehensive download progress tracking with: - NDJSON parsing for streaming progress data from Ollama API - Real-time speed calculation (MB/s, KB/s, B/s) with useRef for delta tracking - Time remaining estimation based on download speed - Animated progress bars in OllamaModelSelector component - IPC event streaming from main process to renderer - Proper listener management with cleanup functions Changes: - memory-handlers.ts: Parse NDJSON from Ollama stderr, emit progress events - OllamaModelSelector.tsx: Display progress bars with speed and time remaining - project-api.ts: Implement onDownloadProgress listener with cleanup - ipc.ts types: Define onDownloadProgress listener interface - infrastructure-mock.ts: Add mock implementation for browser testing This allows users to see real-time feedback when downloading Ollama models, including percentage complete, current download speed, and estimated time remaining. * test: add focused test coverage for Ollama download progress feature Add unit tests for the critical paths of the real-time download progress tracking: - Progress calculation tests (52 tests): Speed/time/percentage calculations with comprehensive edge case coverage (zero speeds, NaN, Infinity, large numbers) - NDJSON parser tests (33 tests): Streaming JSON parsing from Ollama, buffer management for incomplete lines, error handling All 562 unit tests passing with clean dependencies. Tests focus on critical mathematical logic and data processing - the most important paths that need verification. Test coverage: ✅ Speed calculation and formatting (B/s, KB/s, MB/s) ✅ Time remaining calculations (seconds, minutes, hours) ✅ Percentage clamping (0-100%) ✅ NDJSON streaming with partial line buffering ✅ Invalid JSON handling ✅ Real Ollama API responses ✅ Multi-chunk streaming scenarios * docs: add comprehensive JSDoc docstrings for Ollama download progress feature - Enhanced OllamaModelSelector component with detailed JSDoc * Documented component props, behavior, and usage examples * Added docstrings to internal functions (checkInstalledModels, handleDownload, handleSelect) * Explained progress tracking algorithm and useRef usage - Improved memory-handlers.ts documentation * Added docstring to main registerMemoryHandlers function * Documented all Ollama-related IPC handlers (check-status, list-embedding-models, pull-model) * Added JSDoc to executeOllamaDetector helper function * Documented interface types (OllamaStatus, OllamaModel, OllamaEmbeddingModel, OllamaPullResult) * Explained NDJSON parsing and progress event structure - Enhanced test file documentation * Added docstrings to NDJSON parser test utilities with algorithm explanation * Documented all calculation functions (speed, time, percentage) * Added detailed comments on formatting and bounds-checking logic - Improved overall code maintainability * Docstring coverage now meets 80%+ threshold for code review * Clear explanation of progress tracking implementation details * Better context for future maintainers working with download streaming * feat: add batch task creation and management CLI commands - Handle batch task creation from JSON files - Show status of all specs in project - Cleanup tool for completed specs - Full integration with new apps/backend structure - Compatible with implementation_plan.json workflow * test: add batch task test file and testing checklist - batch_test.json: Sample tasks for testing batch creation - TESTING_CHECKLIST.md: Comprehensive testing guide for Ollama and batch tasks - Includes UI testing steps, CLI testing steps, and edge cases - Ready for manual and automated testing * chore: update package-lock.json to match v2.7.2 * test: update checklist with verification results and architecture validation * docs: add comprehensive implementation summary for Ollama + Batch features * docs: add comprehensive Phase 2 testing guide with checklists and procedures * docs: add NEXT_STEPS guide for Phase 2 testing * fix: resolve merge conflict in project-api.ts from Ollama feature cherry-pick * fix: remove duplicate Ollama check status handler registration * test: update checklist with Phase 2 bug findings and fixes --------- Co-authored-by: ray <[email protected]> * fix: resolve Python environment race condition (#142) Implemented promise queue pattern in PythonEnvManager to handle concurrent initialization requests. Previously, multiple simultaneous requests (e.g., startup + merge) would fail with "Already initializing" error. Also fixed parsePythonCommand() to handle file paths with spaces by checking file existence before splitting on whitespace. Changes: - Added initializationPromise field to queue concurrent requests - Split initialize() into public and private _doInitialize() - Enhanced parsePythonCommand() with existsSync() check Co-authored-by: Joris Slagter <[email protected]> * fix: remove legacy path from auto-claude source detection (#148) Removes the legacy 'auto-claude' path from the possiblePaths array in agent-process.ts. This path was from before the monorepo restructure (v2.7.2) and is no longer needed. The legacy path was causing spec_runner.py to be looked up at the wrong location: - OLD (wrong): /path/to/auto-claude/auto-claude/runners/spec_runner.py - NEW (correct): /path/to/apps/backend/runners/spec_runner.py This aligns with the new monorepo structure where all backend code lives in apps/backend/. Fixes #147 Co-authored-by: Joris Slagter <[email protected]> * Fix/linear 400 error * fix: Linear API authentication and GraphQL types - Remove Bearer prefix from Authorization header (Linear API keys are sent directly) - Change GraphQL variable types from String! to ID! for teamId and issue IDs - Improve error handling to show detailed Linear API error messages 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]> * fix: Radix Select empty value error in Linear import modal Use '__all__' sentinel value instead of empty string for "All projects" option, as Radix Select does not allow empty string values. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]> * feat: add CodeRabbit configuration file Introduce a new .coderabbit.yaml file to configure CodeRabbit settings, including review profiles, automatic review options, path filters, and specific instructions for different file types. This enhances the code review process by providing tailored guidelines for Python, TypeScript, and test files. * fix: correct GraphQL types for Linear team queries Linear API uses different types for different queries: - team(id:) expects String! - issues(filter: { team: { id: { eq: } } }) expects ID! 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]> * fix: refresh task list after Linear import Call loadTasks() after successful Linear import to update the kanban board without requiring a page reload. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]> * cleanup * cleanup * fix: address CodeRabbit review comments for Linear integration - Fix unsafe JSON parsing: check response.ok before parsing JSON to handle non-JSON error responses (e.g., 503 from proxy) gracefully - Use ID! type instead of String! for teamId in LINEAR_GET_PROJECTS query for GraphQL type consistency - Remove debug console.log (ESLint config only allows warn/error) - Refresh task list on partial import success (imported > 0) instead of requiring full success - Fix pre-existing TypeScript and lint issues blocking commit 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]> * version sync logic * lints for develop branch * chore: update CI workflow to include develop branch - Modified the CI configuration to trigger on pushes and pull requests to both main and develop branches, enhancing the workflow for development and integration processes. * fix: update project directory auto-detection for apps/backend structure The project directory auto-detection was checking for the old `auto-claude/` directory name but needed to check for `apps/backend/`. When running from `apps/backend/`, the directory name is `backend` not `auto-claude`, so the check would fail and `project_dir` would incorrectly remain as `apps/backend/` instead of resolving to the project root (2 levels up). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]> * fix: use GraphQL variables instead of string interpolation in LINEAR_GET_ISSUES Replace direct string interpolation of teamId and linearProjectId with proper GraphQL variables. This prevents potential query syntax errors if IDs contain special characters like double quotes, and aligns with the variable-based approach used elsewhere in the file. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]> * fix(ui): correct logging level and await loadTasks on import complete - Change console.warn to console.log for import success messages (warn is incorrect severity for normal completion) - Make onImportComplete callback async and await loadTasks() to prevent potential unhandled promise rejections Applies CodeRabbit review feedback across 3 LinearTaskImportModal usages. * fix(hooks): use POSIX-compliant find instead of bash glob The pre-commit hook uses #!/bin/sh but had bash-specific ** glob pattern for staging ruff-formatted files. The ** pattern only works in bash with globstar enabled - in POSIX sh it expands literally and won't match subdirectories, causing formatted files in nested directories to not be staged. --------- Co-authored-by: Claude Opus 4.5 <[email protected]> * fix(task): stop running process when task status changes away from in_progress When a user drags a running task back to Planning (or any other column), the process was not being stopped, leaving a "ghost" process that prevented deletion with "Cannot delete a running task" error. Now the task process is automatically killed when status changes away from in_progress, ensuring the process state stays in sync with the UI. * feat: Add UI scale feature with 75-200% range (#125) * feat: add UI scale feature * refactor: extract UI scale bounds to shared constants * fix: duplicated import * fix: hide status badge when execution phase badge is showing (#154) * fix: analyzer Python compatibility and settings integration Fixes project index analyzer failing with TypeError on Python type hints. Changes: - Added 'from __future__ import annotations' to all analysis modules - Fixed project discovery to support new analyzer JSON format - Read Python path directly from settings.json instead of pythonEnvManager - Added stderr/stdout logging for analyzer debugging Resolves 'Discovered 0 files' and 'TypeError: unsupported operand type' issues. * auto-claude: subtask-1-1 - Hide status badge when execution phase badge is showing When a task has an active execution (planning, coding, etc.), the execution phase badge already displays the correct state with a spinner. The status badge was also rendering, causing duplicate/confusing badges (e.g., both "Planning" and "Pending" showing at the same time). This fix wraps the status badge in a conditional that only renders when there's no active execution, eliminating the redundant badge display. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]> * fix(ipc): remove unused pythonEnvManager parameter and fix ES6 import Address CodeRabbit review feedback: - Remove unused pythonEnvManager parameter from registerProjectContextHandlers and registerContextHandlers (the code reads Python path directly from settings.json instead) - Replace require('electron').app with proper ES6 import for consistency 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]> * chore(lint): fix import sorting in analysis module Run ruff --fix to resolve I001 lint errors after merging develop. All 23 files in apps/backend/analysis/ now have properly sorted imports. --------- Co-authored-by: Joris Slagter <[email protected]> Co-authored-by: Claude Opus 4.5 <[email protected]> * fix/PRs from old main setup to apps structure (#185) * fix(core): add task persistence, terminal handling, and HTTP 300 fixes Consolidated bug fixes from PRs #168, #170, #171: - Task persistence (#168): Scan worktrees for tasks on app restart to prevent loss of in-progress work and wasted API credits. Tasks in .worktrees/*/specs are now loaded and deduplicated with main. - Terminal buttons (#170): Fix "Open Terminal" buttons silently failing on macOS by properly awaiting createTerminal() Promise. Added useTerminalHandler hook with loading states and error display. - HTTP 300 errors (#171): Handle branch/tag name collisions that cause update failures. Added validation script to prevent conflicts before releases and user-friendly error messages with manual download links. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]> * fix(platform): add path resolution, spaces handling, and XDG support This commit consolidates multiple bug fixes from community PRs: - PR #187: Path resolution fix - Update path detection to find apps/backend instead of legacy auto-claude directory after v2.7.2 restructure - PR #182/#155: Python path spaces fix - Improve parsePythonCommand() to handle quoted paths and paths containing spaces without splitting - PR #161: Ollama detection fix - Add new apps structure paths for ollama_model_detector.py script discovery - PR #160: AppImage support - Add XDG Base Directory compliant paths for Linux sandboxed environments (AppImage, Flatpak, Snap). New files: - config-paths.ts: XDG path utilities - fs-utils.ts: Filesystem utilities with fallback support - PR #159: gh CLI PATH fix - Add getAugmentedEnv() utility to include common binary locations (Homebrew, snap, local) in PATH for child processes. Fixes gh CLI not found when app launched from Finder/Dock. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]> * fix: address CodeRabbit/Cursor review comments on PR #185 Fixes from code review: - http-client.ts: Use GITHUB_CONFIG instead of hardcoded owner in HTTP 300 error message - validate-release.js: Fix substring matching bug in branch detection that could cause false positives (e.g., v2.7 matching v2.7.2) - bump-version.js: Remove unnecessary try-catch wrapper (exec() already exits on failure) - execution-handlers.ts: Capture original subtask status before mutation for accurate logging - fs-utils.ts: Add error handling to safeWriteFile with proper logging Dismissed as trivial/not applicable: - config-paths.ts: Exhaustive switch check (over-engineering) - env-utils.ts: PATH priority documentation (existing comments sufficient) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]> * fix: address additional CodeRabbit review comments (round 2) Fixes from second round of code review: - fs-utils.ts: Wrap test file cleanup in try-catch for Windows file locking - fs-utils.ts: Add error handling to safeReadFile for consistency with safeWriteFile - http-client.ts: Use GITHUB_CONFIG in fetchJson (missed in first round) - validate-release.js: Exclude symbolic refs (origin/HEAD -> origin/main) from branch check - python-detector.ts: Return cleanPath instead of pythonPath for empty input edge case Dismissed as trivial/not applicable: - execution-handlers.ts: Redundant checkSubtasksCompletion call (micro-optimization) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]> --------- Co-authored-by: Claude Opus 4.5 <[email protected]> * feat/beta-release (#190) * chore: update README version to 2.7.1 Updated the version badge and download links in the README to reflect t…
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Base Branch
developbranch (required for all feature/fix PRs)Description
ACS-211 - Electron app crashes with "Render frame was disposed" error when multiple agents run simultaneously. The crash occurs because the main process continues sending IPC messages to renderer frames that have been disposed.
Added
safeSendToRenderer()helper function that:Updated all critical IPC handlers:
agent-events-handlers.ts(agent logs, errors, progress)ideation-handlers.ts(ideation events)roadmap-handlers.ts(roadmap events)Related Issue
Closes #917
Type of Change
Area
Commit Message Format
Follow conventional commits:
<type>: <subject>Types: feat, fix, docs, style, refactor, test, chore
Example:
feat: add user authentication systemChecklist
developbranchCI/Testing Requirements
Screenshots
N/A - Non-UI change
Feature Toggle
Breaking Changes
Breaking: No
Details: None - this is a defensive fix that only adds safety checks
Summary by CodeRabbit
Bug Fixes
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.