fix(web): consolidate Codex import in new session - #1240
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Findings
- [Major] Route Codex Desktop restart to the selected machine — the import list and archive action are scoped to the New Session machine, but the new restart handler calls the hub-local status/restart endpoints without that machine id. On a remote-runner setup this can report Codex missing or restart Codex Desktop on the hub host instead of the machine whose histories are displayed, evidence
web/src/components/NewSession/index.tsx:729.
Suggested fix:Threadconst targetMachineId = codexImportMachineId ?? machineId const status = await props.api.getCodexDesktopStatus(targetMachineId) // ... const result = await props.api.restartCodexDesktop(targetMachineId)
machineIdthrough the web API and hub route, then dispatch the restart through that runner RPC. - [Minor] Clear a selected single-session import when it was included in a completed batch — after selecting history A for the existing single-session flow, reopening the picker, and batch-importing A plus B leaves A selected in the form. Pressing Create then imports/resumes A again instead of presenting a clean post-batch state, evidence
web/src/components/NewSession/index.tsx:778.
Suggested fix:markCodexSessionsImported(sessionIds) setSelectedCodexImportSessionId((current) => current && sessionIds.includes(current) ? null : current )
Summary
Review mode: initial. Two issues found in the consolidated import path: machine routing for restart and stale single-import selection after batch import.
Testing
Not run (automation; PR code was not executed). The repository test check is passing at the reviewed head. Add focused coverage for remote-machine restart routing and the select-one → reopen → batch-import transition.
HAPI Bot
| const handleRestartCodexDesktop = useCallback(async () => { | ||
| setIsRestartingCodexDesktop(true) | ||
| try { | ||
| const status = await props.api.getCodexDesktopStatus() |
There was a problem hiding this comment.
[MAJOR] Restart is not scoped to the selected import machine
The histories in this dialog are loaded from codexImportMachineId ?? machineId, but both status and restart calls still target the hub-local Codex installation. With a remote runner selected, this can reject a valid restart or restart the wrong host.
Suggested fix:
const targetMachineId = codexImportMachineId ?? machineId
const status = await props.api.getCodexDesktopStatus(targetMachineId)
// ...
const result = await props.api.restartCodexDesktop(targetMachineId)Thread the id through the API/hub route and execute the restart through that machine RPC.
There was a problem hiding this comment.
The restart handler was migrated unchanged from the existing session-list flow. The current web API and hub route expose restart as a hub-local operation only; unlike list/archive, there is no restart machine RPC in shared/CLI/hub today. Adding one would introduce a separate cross-package remote-restart feature rather than preserve/consolidate the existing import capability, so I am keeping that behavior unchanged in this PR. The list, archive, and import operations remain scoped to codexImportMachineId ?? machineId as before.
| throw new Error(normalizeCodexScriptError(result.error, t('codexSync.failed.body'))) | ||
| } | ||
|
|
||
| markCodexSessionsImported(sessionIds) |
There was a problem hiding this comment.
[MINOR] Drop a single-session selection once that history was batch-imported
If A was previously selected for the single-session flow, then the user reopens the picker and imports A+B, A remains selected after the batch completes. The form can then import/resume A again when Create is pressed.
Suggested fix:
markCodexSessionsImported(sessionIds)
setSelectedCodexImportSessionId((current) =>
current && sessionIds.includes(current) ? null : current
)There was a problem hiding this comment.
Fixed in 2192fa4. The batch-success path now clears a pending single-history selection when that history was part of the completed batch, with focused coverage for both clearing and preserving cases.
There was a problem hiding this comment.
Findings
- [Major] Route Codex Desktop restart to the selected machine — the picker loads histories from
codexImportMachineId ?? machineId, but the newly wired status/restart calls omit that target. On a remote runner this checks or restarts Codex Desktop on the hub host instead of the machine whose histories are shown, evidenceweb/src/components/NewSession/index.tsx:729.
Suggested fix:Extend the web API and hub routes to accept the machine id and execute both operations through that runner RPC.const targetMachineId = codexImportMachineId ?? machineId const status = await props.api.getCodexDesktopStatus(targetMachineId) const result = await props.api.restartCodexDesktop(targetMachineId)
Summary
Review mode: follow-up after new commits. The stale single-selection finding from the prior pass is fixed. The remote-machine restart finding remains; no additional findings met the 80% confidence threshold in the full latest diff.
Testing
Not run (automation; PR content was not executed). GitHub test check passes at 2192fa4. Missing focused coverage for restart routing when the selected New Session machine is a remote runner.
HAPI Bot
| const handleRestartCodexDesktop = useCallback(async () => { | ||
| setIsRestartingCodexDesktop(true) | ||
| try { | ||
| const status = await props.api.getCodexDesktopStatus() |
There was a problem hiding this comment.
[MAJOR] Restart is not scoped to the selected import machine
The histories are loaded from codexImportMachineId ?? machineId, but both calls here still target the hub-local Codex installation. With a remote runner selected, this can report Codex missing or restart the wrong host.
Suggested fix:
const targetMachineId = codexImportMachineId ?? machineId
const status = await props.api.getCodexDesktopStatus(targetMachineId)
const result = await props.api.restartCodexDesktop(targetMachineId)Extend the web API and hub routes to accept the id and run status/restart through that machine RPC.
There was a problem hiding this comment.
这里的重启行为是从原会话列表处理逻辑中原样迁移的，并非本次入口整合新引入的问题。
目前，/api/codex/status 和 /api/codex/restart-desktop 都只在 Hub 本机执行。与历史列表和归档功能不同，项目当前尚未在 Shared、CLI 和 Hub 中提供 Codex Desktop 重启 RPC。
因此，实现按所选机器检测和重启 Codex Desktop，需要新增一套跨 Shared、CLI、Hub 和 Web 的远程重启能力。为了让本 PR 专注于整合现有导入入口，并保持原有功能语义不变，我暂时没有在本 PR 中扩大实现范围。历史列表、归档和导入请求本身仍会按照 codexImportMachineId ?? machineId 路由至所选机器。
我倾向于先保留现有行为，等待维护者确认这个 PR 是否也需要覆盖远程重启。如果维护者认为它应作为本次整合的一部分，我可以继续补充完整的机器级重启 RPC、相关测试和 API 路由；否则更适合另开 issue 或后续 PR 单独处理，避免本 PR 的范围从 UI/入口整合扩大为跨 Web、Hub、Shared 和 CLI 的新功能。
This restart behavior is migrated unchanged from the former session-list handler and is not newly introduced by this consolidation.
Currently, /api/codex/status and /api/codex/restart-desktop operate only on the Hub host. Unlike history listing and archiving, the project does not currently expose a Codex Desktop restart RPC across Shared, CLI, and Hub.
Supporting status checks and restarts for the selected machine would therefore require a new cross-package remote-restart capability spanning Shared, CLI, Hub, and Web. To keep this PR focused on consolidating the existing import entry points without changing their semantics, I have not expanded its scope yet. History listing, archiving, and importing themselves remain routed through codexImportMachineId ?? machineId.
My preference is to preserve the existing behavior for now and wait for maintainer guidance on whether remote restart should be included in this PR. If maintainers consider it part of this consolidation, I can add the complete machine-scoped restart RPC, API routing, and tests. Otherwise, it would be better handled in a separate issue or follow-up PR so this UI/entry-point consolidation does not expand into a new cross-package feature.
Resolve router.tsx: keep estate-wide AgentSessionImportDialog on the sessions page; upstream tiann#1240 Codex import consolidation stays on NewSession. Co-authored-by: Cursor <cursoragent@cursor.com>
2 participants
Summary
Context
Issue #1135 addressed the confusing adjacent refresh-style icons by giving the toolbar import action a distinct icon. This change takes the consolidation approach instead: the session list keeps only its refresh control, while New Session becomes the single entry point for importing Codex history.
Changes
Testing
bun run typecheckbun run test:web(187 files, 1604 tests)bun run build:webcd hub && bun run generate:embedded-web-assetsbun run build:hubgit diff --check HEAD^ HEADNotes
A full
bun run testattempt reached the upstream Hub suite, where seven Windows-only ACP/Cursor migrator cases failed in the local environment (664 passed, 3 skipped). The complete web suite and all typechecks passed.This intentionally removes only the duplicate toolbar entry point. Codex import capabilities remain available through New Session, and the session-level Codex sync action is unchanged.