Repository navigation
Conversation
|
You have reached your Codex usage limits for security reviews. Please try again later. |
![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: dec3058038
ℹ️ 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".
|
You have reached your Codex usage limits for security reviews. Please try again later. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 456d53ebc6
ℹ️ 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".
|
You have reached your Codex usage limits for security reviews. Please try again later. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 75955cf147
ℹ️ 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".
|
You have reached your Codex usage limits for security reviews. Please try again later. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0d02255705
ℹ️ 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".
|
You have reached your Codex usage limits for security reviews. Please try again later. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0b8d1f16f5
ℹ️ 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".
|
You have reached your Codex usage limits for security reviews. Please try again later. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7b77c6d892
ℹ️ 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".
|
You have reached your Codex usage limits for security reviews. Please try again later. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1fd4a6229d
ℹ️ 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".
| Files.deleteIfExists(preserved); | ||
| Files.createLink(preserved, target); |
There was a problem hiding this comment.
When two threads or server processes save the same resolved YAML concurrently, both transactions use this single fixed preserved-inode path, so one can delete or relink the other transaction's backup while its subsequent copy and move operations are still running. The interleaved operations can then copy into the wrong inode, lose the destination's metadata, throw after partially completing a save, or produce inconsistent contents; use destination-scoped cross-process synchronization or transaction-unique sidecars with ownership validation.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Acknowledged — this is valid and is not fixed on current head 1fd4a6229d0b6559ac4f662ad09413b3e5ac8457. The fixed .advancedcore-preserved-inode sidecar can collide across concurrent writers, so this thread should remain open until the transaction is serialized per resolved destination or uses uniquely owned sidecars. I’m requesting a fresh Codex review of the current head now to catch any additional persistence issues before the next patch.
|
|
||
| private static void commitThroughPreservedInode(Path temp, Path target, Path preserved) throws IOException { | ||
| Files.deleteIfExists(preserved); | ||
| Files.createLink(preserved, target); |
There was a problem hiding this comment.
When an existing YAML resides on a POSIX-view filesystem that does not support hard links, such as some network or virtual mounts, every save reaches this call and fails even though creating the sibling temporary file and replacing the destination may be supported. Because createLink has no fallback, both ServerData and reward persistence stop working after the file exists; fall back to metadata-preserving replacement or the compatible in-place path when hard-link creation is unsupported.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Acknowledged — this is valid and is not fixed on current head 1fd4a6229d0b6559ac4f662ad09413b3e5ac8457. Existing POSIX files currently depend on Files.createLink(...); mounts that expose POSIX attributes but reject hard links need a safe fallback rather than failing the save. I’m leaving this thread open and requesting a fresh Codex review of the current head now so any other filesystem compatibility issues are surfaced before the next patch.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1fd4a6229d
ℹ️ 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".
|
|
||
| boolean committed = false; | ||
| try { | ||
| data.save(temp.toFile()); |
There was a problem hiding this comment.
When the host loses power after moveReplacement(temp, target) but before copyCompleteContents forces the preserved inode, data.save(...) has only closed the temporary file and has not forced its contents to stable storage. The rename can therefore survive while some YAML data blocks do not, leaving the live path empty or partial; on the next save, recoverPreservedInode copies that damaged path over the known-good preserved inode. Force the serialized temporary file before the first move, and durably sync the containing directory where supported.
Useful? React with 👍 / 👎.
| if (!targetOwner.equals(tempAcl.getOwner())) { | ||
| tempAcl.setOwner(targetOwner); | ||
| } | ||
| return true; |
There was a problem hiding this comment.
On an ACL-capable non-POSIX filesystem such as NTFS, this successful branch copies only the ACL and owner and then replaces the destination inode. Existing DOS attributes such as hidden/system/archive and user-defined attributes or alternate data streams are consequently discarded after the first save, whereas the previous in-place writer retained them. Preserve the remaining supported attribute views or use the metadata-bearing inode commit path for these destinations.
Useful? React with 👍 / 👎.
Summary
Harden AdvancedCore YAML writes while preserving existing configuration paths and save entry points.
AtomicYamlWriterto serialize into a sibling temporary file before replacing the destinationFilesManager.editFile(...)asynchronous but route its write through the atomic replacement helperServerData.saveData()so its existing immediate-save setters retain their behavior without truncating the last good file during a failed serializationServerDatakeys and public methodsTests
This PR changes persistence mechanics only; it does not rename or migrate configuration keys.