Conversation
fiftin
commented
Apr 19, 2026
fiftin
commented
- feat(secrets): add sync flag and sync paths table
- fix(secrets): layout
- feat(secrets): auto sync
![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: bc0ccade0c
ℹ️ 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".
| } | ||
|
|
||
| for i := range storages { | ||
| storages[i].SyncPaths, err = d.GetSecretStorageSyncPaths(storages[i].ID) |
There was a problem hiding this comment.
After this refactor, secret storages load sync_paths only from project__secret_storage__sync_path, but db/sql/migrations/v2.18.1.sql only creates the new table and never copies existing params.sync_paths data into it. In upgraded instances, previously configured sync mappings become empty in API/UI responses, so both manual and scheduled sync effectively stop until users manually recreate all paths.
Useful? React with 👍 / 👎.
| go schedulePool.Run() | ||
| go taskPool.Run() | ||
|
|
||
| secretStorageSyncScheduler.Start() |
There was a problem hiding this comment.
In active-active mode, this unconditional scheduler start runs on every node, while the rest of runService explicitly adds HA coordination only for schedules/websockets. Because GetSyncEnabledSecretStorages() reads from shared DB state, each node will call SyncSecrets for the same storage on the same interval, leading to duplicated external sync operations and race-prone concurrent writes in multi-node deployments.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Summary
- Wired the secret storage sync scheduler to the existing HA schedule deduplicator in
runService, so both schedule execution and secret-sync ticks can share the same cluster-wide lock behavior. cli/cmd/root.goL138-L141 - Extended
SecretStorageSyncSchedulerwith an optional tick deduplicator hook and added a lock gate at the start of each tick, preventing duplicate sync execution across active-active nodes when deduplication is available. services/server/secret_storage_sync_scheduler.goL16-L23 services/server/secret_storage_sync_scheduler.goL36-L40 services/server/secret_storage_sync_scheduler.goL68-L73 - Committed the changes on the current branch (
b3b10b0) and created a follow-up PR message via themake_prtool.
Testing
- ✅
go test ./services/server ⚠️ go test ./cli/cmd ./services/server(fails in this environment becauseapi/router.goexpects embeddedpublic/*assets that are not present during this run)
1 participant
