feat(session): support native derived sessions#136
Open
awsl233777 wants to merge 6 commits intomainfrom
Open
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR adds native/derived session status tracking, --output-dir control for conversions, import-derived-session CLI flow, and UI support (badges, import button, localized strings). Sessions and conversion results now include native availability, nativePath, derived metadata, and conversion origin fields. ChangesDerived Session Status and Import-to-Native Workflow
Sequence DiagramsequenceDiagram
actor User as User
participant UI as Web UI
participant API as CLI/API
participant FS as File System
User->>UI: Click "Convert Session"
UI->>API: POST convertSession(source, target, outputDir)
API->>FS: Determine outputDir (native or derived)
alt native
API->>FS: Write session to native dir (e.g., ~/.codex/sessions or ~/.claude/projects)
else derived
API->>FS: Write session to ~/.codexmate/sessions/derived/<target>
end
API->>API: Build/attach native-status metadata
API-->>UI: Return session summary with nativeAvailable, nativePath, convertedFrom
UI->>UI: Refresh list, show derived badge
User->>UI: Click "Import to Native" on derived session
UI->>API: POST importDerivedSessionToNative(sessionId)
API->>FS: Copy derived file -> native dir
alt exists
API-->>UI: Prompt confirm overwrite
User->>API: Confirm
API->>FS: Overwrite file
else created
API->>FS: Create file
end
API-->>UI: Return updated nativeAvailable, nativePath
UI->>UI: Update session detail, remove import action
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![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: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cli.js (1)
4724-4747:⚠️ Potential issue | 🟠 Major | ⚡ Quick winWrap all Claude discovery branches the same way.
Only the indexed branch on Lines 4724-4747 goes through
attachSessionNativeStatus(...). The fallback scan and derived-root scan later inlistClaudeSessions()still push raw summaries, so those sessions never exposenativeAvailable,nativeImportAvailable,nativePath, orderivedPath. The UI state will depend on how the session was discovered.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cli.js` around lines 4724 - 4747, In listClaudeSessions(), some discovery branches push raw summary objects directly (so they miss nativeAvailable/nativeImportAvailable/nativePath/derivedPath) while the indexed branch wraps its session with attachSessionNativeStatus before pushing; update the fallback scan and derived-root scan branches to wrap their session objects with attachSessionNativeStatus the same way (i.e., replace sessions.push(rawSummary) with sessions.push(attachSessionNativeStatus({...same fields...})) so all pushes go through attachSessionNativeStatus and expose nativeAvailable, nativeImportAvailable, nativePath, and derivedPath consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cli.js`:
- Around line 7112-7126: The loop always generates a derived ID via
buildDerivedSessionId(baseSessionId) even when outputDirMode === 'native', so
change the logic in the block that assigns derivedSessionId/outputPath/metaPath
to first check if outputDirMode === 'native' and in that case set
derivedSessionId = baseSessionId and set outputPath/metaPath to the native
filenames for baseSessionId (rather than calling buildDerivedSessionId); keep
the existing 8-attempt loop only for non-native modes (where
buildDerivedSessionId is invoked), and if outputDirMode === 'native' check if
the native slot exists and return the same error object when it does to surface
the conflict/abort behavior.
- Around line 7303-7334: The import-to-native flow performs
fs.copyFileSync(resolvedSourcePath, resolvedNativePath) then proceeds to write
meta via writeJsonAtomic and update Claude index via
ensureClaudeSessionsIndex/upsertClaudeSessionIndexEntry, but on any subsequent
error the new native file remains and can overwrite a working session; to fix,
make the operation rollback-safe by either copying to a temporary path and
atomically renaming into resolvedNativePath only after all meta/index writes
succeed, or capture and preserve the original file (if exists) before copy and
on error restore it (and remove the partial new file) and avoid leaving
inconsistent index entries; ensure this change touches the code around
ensureDir, fs.copyFileSync, getDerivedSessionMetaPath, readDerivedSessionMeta,
writeJsonAtomic, ensureClaudeSessionsIndex and upsertClaudeSessionIndexEntry so
failures clean up the copied file and revert any index updates.
- Around line 6469-6490: attachSessionNativeStatus currently only adds
native/derived status and omits conversion provenance used elsewhere; update
attachSessionNativeStatus to also include convertedFrom and convertedFromLabel
derived from the session meta by reading session.meta.convertedFrom (or
session.convertedFrom if present) and calling the existing
buildConvertedFromLabel(session.meta) helper so list/browse payloads carry the
same "Converted from …" fields added in readSessionDetail.
- Around line 6449-6465: In buildSessionNativeStatus: the logic sets nativePath
to resolveNativeSessionFilePath() fallback even when the session file is
actually native; change nativePath selection to prefer currentPath when the
session is already native (inNativePath === true) or when nativeExists &&
!isDerived && currentPath is present, so that nativePath resolves to currentPath
for real native sessions; update nativeAvailable/nativeImportAvailable logic
accordingly to still reflect availability but avoid pointing nativePath at the
resolveNativeSessionFilePath fallback when currentPath is the real native file.
In `@cli/session-convert.js`:
- Around line 89-100: The conversion currently writes only the .jsonl but not
the companion derived metadata, so create and persist a derived metadata file
next to the converted session after fs.writeFileSync(outputPath, jsonl,...).
Build a metadata object (include sessionId, cwd: extracted.cwd || '', derived:
true, convertedFrom: filePath (or extracted.source if available),
nativeAvailable: true/false as appropriate) and serialize it to JSON, then write
it to a sidecar file (e.g. same basename as outputPath with a .meta.json or
.metadata.json suffix) using ensureDir as needed and fs.writeFileSync; reference
readSessionMessages, buildTargetRecords, resolveDefaultOutputPath, outputPath
and extracted to locate where to add this.
- Around line 93-99: The existsSync/writeFileSync pair in
resolveDefaultOutputPath/cli session conversion is a TOCTOU race: replace the
check-then-write with an atomic create-or-fail operation by first ensuring the
target directory exists via ensureDir(path.dirname(outputPath)) and then
opening/writing the file using an exclusive-create flag (e.g., use
fs.open/fs.write or fs.writeFile with the 'wx' flag) so the write fails if the
file already exists; catch that specific error and call process.exit(1) with the
same conflict message instead of relying on fs.existsSync.
In `@web-ui/modules/app.methods.session-actions.mjs`:
- Around line 101-104: The fallback branch is mutating this.activeSession
unconditionally which can mark the wrong session after an async API return;
capture the target session identifier before initiating the async import check
(e.g., const targetSessionId = this.activeSession?.id) and when the response
arrives update the session by locating the session with that id (or verify
this.activeSession?.id === targetSessionId) before setting nativeAvailable and
nativePath; update the code paths that set nativeAvailable and nativePath
(referencing this.activeSession, nativeAvailable, nativePath, and
res.nativePath) to only mutate the session whose id matches the captured id.
In `@web-ui/modules/i18n.dict.mjs`:
- Around line 549-553: The zh dictionary contains English values for keys like
'sessions.derivedBadge', 'sessions.convertedFrom', 'sessions.nativeAvailable',
'common.yes', and 'common.no' (and the additional entries around lines 565-566);
update the zh translations to proper Chinese equivalents (e.g., Derived -> 派生,
Converted from {source} -> 转换自 {source}, Native Available: {value} ->
本地可用：{value}, Yes -> 是, No -> 否) by replacing the English strings in the zh
section for these keys so the UI is consistently localized.
In `@web-ui/partials/index/panel-sessions.html`:
- Around line 235-239: The convert button currently only disables based on
activeSession and isSessionConvertAvailable(activeSession) but not when a
conversion is already running; update the disable logic so the button (class
btn-session-export) is disabled when
sessionConverting[getSessionExportKey(activeSession)] is true as well, or add an
early guard in convertSession to return immediately if
sessionConverting[getSessionExportKey(activeSession)] is true; reference
convertSession, activeSession, sessionConverting, and getSessionExportKey to
locate and apply the change.
---
Outside diff comments:
In `@cli.js`:
- Around line 4724-4747: In listClaudeSessions(), some discovery branches push
raw summary objects directly (so they miss
nativeAvailable/nativeImportAvailable/nativePath/derivedPath) while the indexed
branch wraps its session with attachSessionNativeStatus before pushing; update
the fallback scan and derived-root scan branches to wrap their session objects
with attachSessionNativeStatus the same way (i.e., replace
sessions.push(rawSummary) with sessions.push(attachSessionNativeStatus({...same
fields...})) so all pushes go through attachSessionNativeStatus and expose
nativeAvailable, nativeImportAvailable, nativePath, and derivedPath
consistently.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 257d5859-6f03-4b8e-8842-7144abbfc093
📒 Files selected for processing (14)
cli.jscli/session-convert-args.jscli/session-convert.jstests/e2e/test-session-convert-derived.jstests/unit/session-header-actions-layout.test.mjstests/unit/session-resume-command.test.mjstests/unit/web-ui-behavior-parity.test.mjsweb-ui/app.jsweb-ui/modules/app.methods.codex-config.mjsweb-ui/modules/app.methods.session-actions.mjsweb-ui/modules/i18n.dict.mjsweb-ui/partials/index/panel-sessions.htmlweb-ui/session-helpers.mjsweb-ui/styles/sessions-toolbar-trash.css
📜 Review details
🔇 Additional comments (6)
web-ui/styles/sessions-toolbar-trash.css (1)
226-238: Derived badge style is clean and consistent.The new badge integrates well with the existing session chip/badge visual system.
cli/session-convert-args.js (1)
28-28:--output-dirparsing/validation looks solid.Good defaulting, normalization, and explicit allowlist handling.
Also applies to: 44-45, 55-56
web-ui/session-helpers.mjs (1)
428-445: Session detail hydration extension looks good.The added native/derived metadata propagation is coherent and fits the new UI state requirements.
web-ui/modules/app.methods.codex-config.mjs (1)
83-89: Per-session convert availability guard is well implemented.Nice addition to prevent duplicate conversion actions and invalid source/target flows.
tests/unit/session-resume-command.test.mjs (1)
23-41: New coverage for derived/native resume behavior is valuable.This test closes an important regression gap around warning/import logic.
web-ui/app.js (1)
200-200:sessionImportingNativestate addition is appropriate.This integrates cleanly with the new import action flow and loading guards.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
web-ui/modules/app.methods.session-actions.mjs (1)
108-111:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate the imported session, not whichever session is active later.
After the awaits above, Line 108 mutates
this.activeSessionunconditionally. If the user switches selection before the import resolves, the wrong session can be marked as native. This is the same race previously flagged on this method.🔒 Safer state-targeted update
- } else if (this.activeSession) { - this.activeSession.nativeAvailable = true; - this.activeSession.nativePath = res.nativePath || this.activeSession.nativePath; + } else if (session && typeof session === 'object') { + session.nativeAvailable = true; + session.nativePath = res.nativePath || session.nativePath; + if (this.activeSession === session) { + this.activeSession.nativeAvailable = true; + this.activeSession.nativePath = res.nativePath || this.activeSession.nativePath; + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web-ui/modules/app.methods.session-actions.mjs` around lines 108 - 111, Before awaiting the import result, capture the specific session reference or its id (e.g., importedSession or importedSessionId) and after the await, update that stored session instance (or locate it in this.sessions by id) instead of mutating this.activeSession; replace the unconditional writes to this.activeSession.nativeAvailable and this.activeSession.nativePath with updates targeted to the captured session (and guard for it being present), so a user switching activeSession during the async import won’t cause the wrong session to be marked native.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@web-ui/modules/app.methods.session-actions.mjs`:
- Around line 31-43: The two helpers handle missing session.nativeAvailable
inconsistently: change isSessionNativeAvailable to treat nativeAvailable as
available only when explicitly true (e.g., return session.nativeAvailable ===
true after validating session object) so its behavior aligns with
isImportToNativeAvailable (which already blocks import only when nativeAvailable
=== true); update the logic in isSessionNativeAvailable (function name)
accordingly so undefined no longer counts as "native available".
- Around line 440-448: Replace the hardcoded Chinese toast messages inside the
copy/resume flow with the i18n keys added in the PR: use the nativeOk variable
and call the translation helper (e.g., this.$t or the project's i18n.t) instead
of the literal strings in both the immediate-ok branch and the
navigator.clipboard success branch; update the two showMessage calls (the ones
using nativeOk ? '已复制' : '已复制，但该会话不在原生目录，恢复可能失败') to use something like
this.$t('session.copied') and this.$t('session.copied_not_native') (keeping the
same success/info severity) so English users receive localized text.
---
Duplicate comments:
In `@web-ui/modules/app.methods.session-actions.mjs`:
- Around line 108-111: Before awaiting the import result, capture the specific
session reference or its id (e.g., importedSession or importedSessionId) and
after the await, update that stored session instance (or locate it in
this.sessions by id) instead of mutating this.activeSession; replace the
unconditional writes to this.activeSession.nativeAvailable and
this.activeSession.nativePath with updates targeted to the captured session (and
guard for it being present), so a user switching activeSession during the async
import won’t cause the wrong session to be marked native.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ecb5d578-a21a-42a3-af80-35b61d250279
📒 Files selected for processing (3)
tests/unit/session-resume-command.test.mjsweb-ui/modules/app.methods.session-actions.mjsweb-ui/modules/i18n.dict.mjs
Comment on lines
+31
to
+43
| isSessionNativeAvailable(session) { | ||
| if (!session || typeof session !== 'object') return false; | ||
| if (session.nativeAvailable === false) return false; | ||
| return true; | ||
| }, | ||
|
|
||
| isImportToNativeAvailable(session) { | ||
| if (!this.isDerivedSession(session)) return false; | ||
| if (session.nativeAvailable === true) return false; | ||
| if (session.nativeImportAvailable === false) return false; | ||
| const source = String(session.source || '').trim().toLowerCase(); | ||
| return source === 'codex' || source === 'claude'; | ||
| }, |
Contributor
There was a problem hiding this comment.
Align undefined nativeAvailable handling across these helpers.
Line 33 treats a missing flag as “native available”, while Line 39 only blocks import when the flag is explicitly true. A derived session with nativeAvailable omitted can therefore show the normal resume tooltip and still offer “Import to Native” at the same time.
🔧 Suggested consistency fix
isSessionNativeAvailable(session) {
if (!session || typeof session !== 'object') return false;
- if (session.nativeAvailable === false) return false;
- return true;
+ if (this.isDerivedSession(session)) {
+ return session.nativeAvailable === true;
+ }
+ return session.nativeAvailable !== false;
},
isImportToNativeAvailable(session) {
if (!this.isDerivedSession(session)) return false;
- if (session.nativeAvailable === true) return false;
+ if (this.isSessionNativeAvailable(session)) return false;
if (session.nativeImportAvailable === false) return false;
const source = String(session.source || '').trim().toLowerCase();
return source === 'codex' || source === 'claude';
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| isSessionNativeAvailable(session) { | |
| if (!session || typeof session !== 'object') return false; | |
| if (session.nativeAvailable === false) return false; | |
| return true; | |
| }, | |
| isImportToNativeAvailable(session) { | |
| if (!this.isDerivedSession(session)) return false; | |
| if (session.nativeAvailable === true) return false; | |
| if (session.nativeImportAvailable === false) return false; | |
| const source = String(session.source || '').trim().toLowerCase(); | |
| return source === 'codex' || source === 'claude'; | |
| }, | |
| isSessionNativeAvailable(session) { | |
| if (!session || typeof session !== 'object') return false; | |
| if (this.isDerivedSession(session)) { | |
| return session.nativeAvailable === true; | |
| } | |
| return session.nativeAvailable !== false; | |
| }, | |
| isImportToNativeAvailable(session) { | |
| if (!this.isDerivedSession(session)) return false; | |
| if (this.isSessionNativeAvailable(session)) return false; | |
| if (session.nativeImportAvailable === false) return false; | |
| const source = String(session.source || '').trim().toLowerCase(); | |
| return source === 'codex' || source === 'claude'; | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web-ui/modules/app.methods.session-actions.mjs` around lines 31 - 43, The two
helpers handle missing session.nativeAvailable inconsistently: change
isSessionNativeAvailable to treat nativeAvailable as available only when
explicitly true (e.g., return session.nativeAvailable === true after validating
session object) so its behavior aligns with isImportToNativeAvailable (which
already blocks import only when nativeAvailable === true); update the logic in
isSessionNativeAvailable (function name) accordingly so undefined no longer
counts as "native available".
Comment on lines
+440
to
+448
| const nativeOk = !(session && session.nativeAvailable === false); | ||
| if (ok) { | ||
| this.showMessage('已复制', 'success'); | ||
| this.showMessage(nativeOk ? '已复制' : '已复制，但该会话不在原生目录，恢复可能失败', nativeOk ? 'success' : 'info'); | ||
| return; | ||
| } | ||
| try { | ||
| if (navigator.clipboard && window.isSecureContext) { | ||
| await navigator.clipboard.writeText(command); | ||
| this.showMessage('已复制', 'success'); | ||
| this.showMessage(nativeOk ? '已复制' : '已复制，但该会话不在原生目录，恢复可能失败', nativeOk ? 'success' : 'info'); |
Contributor
There was a problem hiding this comment.
Localize the new resume-warning toast.
Lines 442 and 448 hardcode the new success/warning text in Chinese, so English users will get a mixed-language toast on this path even though this PR already adds i18n entries for the same feature.
🌐 Suggested localization approach
async copyResumeCommand(session) {
+ const tr = (key, fallback, params = null) => (typeof this.t === 'function' ? this.t(key, params) : fallback);
if (!this.isResumeCommandAvailable(session)) {
this.showMessage('不支持此操作', 'error');
return;
}
const command = this.buildResumeCommand(session);
const ok = this.fallbackCopyText(command);
const nativeOk = !(session && session.nativeAvailable === false);
+ const copiedMessage = tr('toast.copy.ok', 'Copied');
+ const nativeWarning = tr('sessions.resume.nativeUnavailableTitle', 'Session not in native directory, resume may fail');
if (ok) {
- this.showMessage(nativeOk ? '已复制' : '已复制，但该会话不在原生目录，恢复可能失败', nativeOk ? 'success' : 'info');
+ this.showMessage(nativeOk ? copiedMessage : `${copiedMessage} · ${nativeWarning}`, nativeOk ? 'success' : 'info');
return;
}
try {
if (navigator.clipboard && window.isSecureContext) {
await navigator.clipboard.writeText(command);
- this.showMessage(nativeOk ? '已复制' : '已复制，但该会话不在原生目录，恢复可能失败', nativeOk ? 'success' : 'info');
+ this.showMessage(nativeOk ? copiedMessage : `${copiedMessage} · ${nativeWarning}`, nativeOk ? 'success' : 'info');
return;
}
} catch (_) {}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web-ui/modules/app.methods.session-actions.mjs` around lines 440 - 448,
Replace the hardcoded Chinese toast messages inside the copy/resume flow with
the i18n keys added in the PR: use the nativeOk variable and call the
translation helper (e.g., this.$t or the project's i18n.t) instead of the
literal strings in both the immediate-ok branch and the navigator.clipboard
success branch; update the two showMessage calls (the ones using nativeOk ?
'已复制' : '已复制，但该会话不在原生目录，恢复可能失败') to use something like this.$t('session.copied')
and this.$t('session.copied_not_native') (keeping the same success/info
severity) so English users receive localized text.
Collaborator
Author
|
@coderabbitai re-review |
Contributor
|
Tip For best results, initiate chat on the files or code changes.
[review] |
Collaborator
Author
|
@coderabbitai re-review |
Contributor
|
Tip For best results, initiate chat on the files or code changes.
[review] |
Collaborator
Author
|
@coderabbitai re-review |
Contributor
|
Tip For best results, initiate chat on the files or code changes.
[review] |
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
1 participant
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
nativeAvailable,nativePath,derivedPath, and import availability metadata.outputDir=derived/--output-dir derivedpreserves the legacy codexmate-derived directory behavior.import-derived-sessionAPI and Web UI “Import to Native” flow for legacy derived sessions, including conflict confirmation, rollback-safe overwrite/import handling, and resume warning when a derived session is not natively available.Issues
Closes #131
Closes #132
Closes #133
Closes #134
Closes #135
Verification
node --check cli.jsnode --check cli/session-convert.jsnode --check cli/session-convert-args.jsnode --check web-ui/modules/app.methods.session-actions.mjsnode --check web-ui/modules/app.methods.codex-config.mjsnode --check web-ui/modules/i18n.dict.mjsnode --check web-ui/session-helpers.mjsnode --check tests/e2e/test-session-convert-derived.jsnode --check tests/unit/session-convert.test.mjsnode --check tests/unit/session-resume-command.test.mjsnode tests/unit/run.mjs— 524 tests passednode tests/e2e/run.js— completed with exit code 0npm run lint— lint passed for 186 filesgit diff --check