perf: harden sync planning and journal throughput - #76
Conversation
![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: c2e0646fc7
ℹ️ 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".
|
@codex 重新审查 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f487720a43
ℹ️ 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".
Two regressions from this branch's sync hardening. A rollout that grows while its content digest is being folded threw CoreWritePlanStaleException out of CollectSessionChangesAsync. The skipLockedReads catch filters only match busy and unreadable I/O errors, so an ordinary unchecked sync aborted entirely instead of skipping the file an active Codex session is appending to. Report it as a locked rollout when skipLockedReads is set, matching the documented partial-success contract, and keep throwing on the strict path where a stale hint must not be cached. Refreshing the backup inventory ran unguarded right after the journal reached committed. Because the outer handler rethrows once transactionCommitted is set, a failed metadata.json write reported a fully durable sync as failed and skipped automatic backup pruning. Degrade it to a warning on both the sync and restore paths, in Core and in the Node service. Adds a scan-phase test seam so the mid-scan mutation is exercised deterministically, and covers both fixes on both implementations.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 54fc74263c
ℹ️ 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".
Second review round on the backup inventory work, plus the folder-open false error. Rollback paths left metadata.json stale. RolledBackAsync and RecoveryRequiredAsync append journal records into the retained backup after its inventory was written, and GetBackupDirectorySize trusts the recorded sizeBytes unconditionally, so status and pruning read a size and file count that no longer match disk. Both rollback exits now refresh the inventory, swallowing any failure so the original sync error and its rollback details reach the caller unchanged. The Node CLI dropped restore's backupInventoryWarning, silently hiding a warning the service already produced. OpenPath treated a null Process as a failure. With UseShellExecute the shell may satisfy the request through a process it already owns - Explorer reusing an open window is the common case - and then returns no handle even though the path opened, so the folder opened while the GUI reported "Unable to open". A genuine failure still surfaces as Win32Exception, and OpenBackupFolder now reports it the same way OpenLogFolder does instead of escaping to the message loop.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 62c4bf287f
ℹ️ 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".
The chmod(0o444) fault only works when the suite runs as a user that Unix permission bits actually constrain. As root the atomic metadata rewrite succeeds, backupInventoryWarning stays undefined and the test fails, which is what happens in the repository container. runRestore now accepts a faultInjector and threads it into the post-restore refresh, matching how runSync already exposes one, so the test drives the failure through a deterministic seam.
1 participant
Summary
Validation
dotnet test CodexProviderSync.sln --configuration Release --no-build— 380 passed, 1 skippednpm test— 208 passeddalalignmentSafety
src/cli.jsremains an unrelated local working-tree change and is intentionally excluded