Open
Conversation
Dailin521
previously requested changes
Apr 17, 2026
Owner
Dailin521
left a comment
There was a problem hiding this comment.
Thanks for the patch. The .NET side looks fine, but there is still one blocking issue on the JS path.
Blocking:
- In
src/session-files.js,restoreFileMtime()readssnapshot.mtimeMs, but the WindowsapplySessionChanges()path passes the collected change object, which only hasoriginalMtimeMs. That means the mtime restore becomes a no-op on Windows for the main sync path. - I pulled this branch locally and re-ran
npm test; the new testrunSync preserves rollout modified time after rewriting provider metadatafails for exactly this reason.
Please fix the field mismatch on the JS side and re-run the test suite. After that this PR should be in much better shape.
Dailin521
requested changes
Apr 17, 2026
Owner
Dailin521
left a comment
There was a problem hiding this comment.
感谢提交。.NET 这边看起来没问题,但 JS 路径目前还有一个阻塞点。
阻塞问题:
src/session-files.js里的restoreFileMtime()读取的是snapshot.mtimeMs,但 Windows 下applySessionChanges()传进去的是收集阶段的 change 对象,这个对象只有originalMtimeMs,没有mtimeMs。结果就是主同步路径下的 mtime 恢复在 Windows 上实际没有生效。- 我已经把这个分支拉到本地重新跑过
npm test,新增的runSync preserves rollout modified time after rewriting provider metadata用例会直接失败,问题和这里一致。
建议先修正 JS 侧字段不一致的问题,并重新跑完整测试;这个点修完后,这个 PR 会更接近可合并状态。
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.
摘要
rename方式覆盖 rollout 文件,改为原地回写mtime背景
当前
codex-provider sync在同步 provider 时,会重写~/.codex/sessions和~/.codex/archived_sessions下的 rollout 文件。
虽然 provider 同步本身是正确的,但这个过程会刷新文件时间。对于依赖 rollout 文件时间做启动扫描或活跃会话判断
的工具来说,这会带来副作用:历史会话可能会被误判成“刚刚活跃过的会话”。
我这边实际遇到的现象是:
codex-provider sync后~/.codex/sessions的工具会把大量历史 Codex session 误加载出来修改内容
CLI / Node 实现
rename覆盖 rollout 文件mtimemtime恢复Desktop / .NET 实现
LastWriteTimeUtc验证
npm test说明
这次改动是一个尽量小的修复,只处理 rollout 文件重写带来的时间戳副作用,不改变以下行为: