fix(transfer): resolve imported thread id on Windows and surface import failures - #701
fix(transfer): resolve imported thread id on Windows and surface import failures#701JMak-Security wants to merge 1 commit into
Conversation
…ures `/codex:transfer` reported failure on every attempt on Windows even when the Claude session imported successfully, because the post-import ledger lookup in `external_agent_session_imports.json` could never match: - Codex writes `source_path` via Rust's `fs::canonicalize`, which on Windows emits a verbatim extended-length path (`\?\C:\...`), while the plugin compared it against `fs.realpathSync()`, which never carries that prefix. The strict equality was therefore always false on Windows. - `content_sha256` was recomputed from the source after import, but the source is the live Claude transcript that keeps growing during the import, so the hash no longer matched the imported snapshot — breaking the lookup on every platform for the common case of transferring the running session. Prefer the authoritative `externalAgentConfig/import/completed` notification, whose `successes[].target` is the imported thread id, and fall back to the ledger with both comparisons relaxed: paths are normalized (verbatim prefix stripped, case-insensitive on Windows) and `content_sha256` is demoted from a requirement to a preference. On failure, surface `failures[]` from the notification instead of a generic message. Adds regression tests for path normalization and completion/failure parsing. Fixes openai#618 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3fa57011cb
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const threadId = | ||
| importedThreadIdFromCompletion(completion, options.sourcePath) ?? | ||
| importedThreadIdForSource(options.sourcePath); |
There was a problem hiding this comment.
Reject failed re-imports before reading the ledger
When this source was imported previously and a later import completes without a success target, the unconditional ledger fallback can return the earlier thread. Because importedThreadIdForSource now accepts a path match even when the current content hash differs, a changed transcript whose new import fails is reported as successfully imported while the user is given a stale thread that lacks the latest session content; only use the ledger when the completion lacks result data, not when it explicitly contains no matching success.
Useful? React with 👍 / 👎.
What
/codex:transferreported failure on every attempt on Windows even though theClaude session was imported successfully — the imported thread existed and was
resumable, but the user was told it failed and never got the thread id.
Root cause (per #618): the post-import lookup in
external_agent_session_imports.jsoncould never match the record:
source_pathvia Rust'sfs::canonicalize, which on Windows emits a verbatim extended-length path(
\?\C:\...). The plugin compared it againstfs.realpathSync(), which nevercarries that prefix, so the strict equality was always false on Windows.
content_sha256was recomputed from the sourceafter import, but the source is the live Claude transcript that keeps growing
during the import. The recomputed hash no longer matches the imported snapshot —
breaking the lookup on every platform when transferring the running session
(the normal case).
Fix
Prefer the authoritative
externalAgentConfig/import/completednotification, whosesuccesses[].targetis the imported thread id, and keep the ledger as a fallbackwith both comparisons relaxed:
requestExternalAgentSessionImportnow returns the completionparams.importedThreadIdFromCompletionresolves the thread id from the notification.normalizeImportPath: strip the verbatim\?\/\?\UNC\prefix, case-insensitive on Windows) before comparison.
content_sha256is demoted from a requirement to a preference.failures[]from the notification is surfaced instead of a generic"did not record an imported thread" message.
Tests
Adds
tests/codex-transfer.test.mjscovering verbatim/UNC path normalization,completion-target resolution (matched + fallback), and failure-detail extraction.
npm testandnpm run buildpass.Fixes #618