Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2daedb306c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| conversationHistory, | ||
| model | ||
| ); | ||
| await runSessionClaude(sessionManager, sessionId, continuePrompt, { mode: 'continue', model }, claudeCodeManager); |
There was a problem hiding this comment.
When the selected Claude panel is still starting or already working, runSessionClaude rejects the request, but continueConversation has already persisted the prompt to conversation history and session output. The UI therefore reports failure while history and later context compaction treat an unsent prompt as delivered; the removed isSessionRunning check previously prevented this case. Validate/send first, or roll back/defer persistence until the terminal accepts the prompt.
Useful? React with 👍 / 👎.
| const resumeId = startFresh ? undefined : previousState?.agentSessionId | ||
| ?? (previousState?.hasClaudeSessionId ? panel?.id : undefined) | ||
| ?? (!panel ? dbSession?.claude_session_id : undefined); |
There was a problem hiding this comment.
After a fresh launch fails, the persisted panel still contains the locally generated agentSessionId, even though Claude may never have created that conversation. The next sessions:continue call treats the mere presence of that ID as proof that it is resumable and launches Claude with --resume, so failures such as a missing/broken executable can leave all subsequent retries targeting a nonexistent session. Only resume IDs confirmed by hasClaudeSessionId/successful startup, or clear the allocated ID on failure.
AGENTS.md reference: AGENTS.md:L7-L8
Useful? React with 👍 / 👎.
| const executable = config?.claudeExecutablePath | ||
| ?? (project?.wsl_enabled ? 'claude' : await this.getCliExecutablePath()); |
There was a problem hiding this comment.
For a WSL project on Windows with the global custom executable configured to a host path such as C:\Users\…\claude.exe, the nullish-coalescing order selects that host path despite the comment and passes it to the WSL bash -lc command. Bash cannot execute the Windows path, so all migrated session commands fail for this otherwise supported configuration; WSL projects should select a distro-local executable (or explicitly translate a compatible path) before considering the host override.
AGENTS.md reference: AGENTS.md:L7-L8
Useful? React with 👍 / 👎.
| await terminalPanelManager.initializeTerminal(panel, session.worktreePath, context?.commandRunner.wslContext ?? null); | ||
| if (!terminalPanelManager.isCommandBoundTerminal(panel.id)) throw new Error('Claude exited during startup'); | ||
| await sessionManager.updateSession(sessionId, { status: 'running', run_started_at: 'CURRENT_TIMESTAMP' }); |
There was a problem hiding this comment.
Every migrated start/continue now spawns through TerminalPanelManager, which never emits the claudeCodeManager spawned or exit events consumed in events.ts. Consequently these sessions no longer run the existing execution-tracking startup, PR-cache invalidation, final git-status refresh, or session-summary append performed by those listeners. Wire equivalent behavior to the new terminal lifecycle events so changing process ownership does not silently disable these session features.
Useful? React with 👍 / 👎.
1 participant
Session commands still called deprecated virtual-session wrappers, which bypassed the persisted terminal lifecycle. This PR migrates those live callers to real owned terminal panels, fixes optional Redis queue configuration, and removes obsolete direct runtime dependencies.
autoretains Claude's default model.Closes #620
Closes #621
Closes #622
Includes current main through v2.4.106 (
99791ac), preserving the pane name reservations and creation-failure handling from #643.Validation on final head
713e94e: Code Quality passed, including Linux main/frontend tests and Electron smoke checks, plus the macOS and Windows main suites. React Doctor also passed. Local verification passed 61 focused tests after the main merge, fullpnpm lint, workspacepnpm typecheck, andpnpm build:main, including sandboxed preload verification. Shell argument preservation executes a real POSIX shell in a regression test; PowerShell/WSL construction is covered by unit tests.