fix(acp,log,main): revert codex-acp 0.10.0, fix log persistence, refactor index.ts#1368
Merged
fix(acp,log,main): revert codex-acp 0.10.0, fix log persistence, refactor index.ts#1368
Conversation
added 5 commits
March 17, 2026 18:37
Users reported issues with [email protected] introduced in #1316. Revert to the previous stable version 0.9.5. Closes #1367
…ransport `log.initialize()` only patches the renderer process via preload. Add `Object.assign(console, log.functions)` to explicitly redirect main-process console.log/warn/error through electron-log, ensuring all log output is persisted to daily log files on disk.
…C from index.ts - Extract tray business logic to src/process/tray.ts - Extract deep link parsing and handling to src/process/deepLink.ts - Extract WebUI config resolution to src/process/webuiConfig.ts - Move ipcBridge.application.isDevToolsOpened/openDevTools providers to src/process/bridge/applicationBridge.ts where they belong - Add unit tests for all extracted modules (41 new tests) - Update vitest coverage config to include new source files - Reduce index.ts from ~1060 lines to ~660 lines
- Add src/process/mainWindowLifecycle.ts with bindMainWindowReferences, showAndFocusMainWindow, and showOrCreateMainWindow helpers - Consolidate 4 scattered setXxxMainWindow calls into bindMainWindowReferences - Eliminate duplicated show/restore/focus logic across second-instance, open-url, and activate handlers - Remove getAllWindows() fallback that incorrectly treated hidden utility windows (e.g. HTML-to-PDF converter) as reusable main windows - Add unit tests for mainWindowLifecycle module
- Replace relative imports with path aliases (@/extensions, @process/bridge) - Fix Prettier formatting (indentation, trailing commas)
kaizhou-lab
changed the title
There was a problem hiding this comment.
Pull request overview
This PR reverts the Codex ACP bridge version to address reported regressions, fixes main-process log persistence by redirecting console to electron-log, and reduces src/index.ts complexity by extracting tray, deep link, WebUI config, and main-window lifecycle logic into dedicated modules.
Changes:
- Revert
CODEX_ACP_BRIDGE_VERSIONfrom0.10.0back to0.9.5. - Ensure main-process
console.*output is persisted by explicitly assigningconsolemethods toelectron-logfunctions. - Refactor main-process business logic into
src/process/*modules and add unit tests for the extracted behavior.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| vitest.config.ts | Adds extracted process modules to coverage include list. |
| tests/unit/webuiConfig.test.ts | Unit tests for WebUI config parsing/resolution and auto-restore. |
| tests/unit/tray.test.ts | Unit tests for tray lifecycle, menu building, and error handling. |
| tests/unit/mainWindowLifecycle.test.ts | Unit tests for main-window binding and show-or-create behavior. |
| tests/unit/deepLink.test.ts | Unit tests for deep link parsing, queuing, and IPC emission behavior. |
| tests/unit/configureConsoleLog.test.ts | Unit tests validating electron-log setup and console redirection. |
| src/utils/configureConsoleLog.ts | Redirects main-process console output to electron-log for persistence. |
| src/types/acpTypes.ts | Reverts Codex ACP bridge version to 0.9.5. |
| src/process/webuiConfig.ts | New module for WebUI config parsing and preference-based restore. |
| src/process/tray.ts | New module encapsulating tray state, creation, and menu building. |
| src/process/mainWindowLifecycle.ts | New helpers for binding main-window references and focusing/creating windows. |
| src/process/deepLink.ts | New module for deep link parsing and pending URL handling. |
| src/process/bridge/applicationBridge.ts | Moves DevTools IPC providers into the application bridge with window ref binding. |
| src/index.ts | Refactors entrypoint to use extracted modules and simplifies main-process flow. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+62
to
+79
| const wasOpen = mainWindowRef.webContents.isDevToolsOpened(); | ||
|
|
||
| if (wasOpen) { | ||
| mainWindowRef.webContents.closeDevTools(); | ||
| return Promise.resolve(false); | ||
| } else { | ||
| return new Promise((resolve) => { | ||
| const onOpened = () => { | ||
| mainWindowRef!.webContents.off('devtools-opened', onOpened); | ||
| resolve(true); | ||
| }; | ||
|
|
||
| mainWindowRef!.webContents.once('devtools-opened', onOpened); | ||
| mainWindowRef!.webContents.openDevTools(); | ||
|
|
||
| setTimeout(() => { | ||
| mainWindowRef!.webContents.off('devtools-opened', onOpened); | ||
| const isNowOpen = mainWindowRef!.webContents.isDevToolsOpened(); |
Collaborator
Author
There was a problem hiding this comment.
Good catch. Fixed in 9ada160 — captured a local const win = mainWindowRef before async work and added win.isDestroyed() guard in the timeout fallback.
Comment on lines
+41
to
+45
| vi.doUnmock('electron-log/main'); | ||
| // Restore original console functions | ||
| console.log = originalConsoleLog; | ||
| console.warn = originalConsoleWarn; | ||
| console.error = originalConsoleError; |
Collaborator
Author
There was a problem hiding this comment.
Good catch. Fixed in 9ada160 — now saving/restoring all keys from mockLog.functions (including info and debug) instead of only the three originally captured.
added 3 commits
March 17, 2026 18:58
- Capture local window ref in openDevTools to prevent stale access after window destruction during async callbacks - Save/restore all console methods in configureConsoleLog test to prevent mock leakage across test suites
Re-register all module mocks before applying per-test overrides to prevent stale mock references across vi.resetModules() calls.
IceyLiu
approved these changes
Mar 17, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
CODEX_ACP_BRIDGE_VERSIONfrom0.10.0back to0.9.5due to user-reported issues (fix(acp): bump bridge versions and fix Windows UTF-8 encoding for npx #1316)src/index.tsfrom ~1060 lines to ~630 lines by extracting business logic:src/process/tray.ts— system tray logicsrc/process/deepLink.ts— deep link parsing and handlingsrc/process/webuiConfig.ts— WebUI config resolutionsrc/process/mainWindowLifecycle.ts— main window lifecycle helperssrc/process/bridge/applicationBridge.tsCLAUDE_ACP_BRIDGE_VERSION(0.21.0) remains unchanged.Closes #1367