fix(management): preserve auth metadata and Codex priority on save#3843
fix(management): preserve auth metadata and Codex priority on save#3843drwpls wants to merge 4 commits into
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces metadata merging capabilities when updating auth file statuses and adds support for updating the priority of Codex keys. Specifically, it updates the PatchAuthFileStatus handler and the file token store to merge existing on-disk metadata with incoming request metadata, preventing the loss of existing fields. It also adds corresponding unit tests. The review feedback suggests extending this metadata merging logic to the auth.Storage != nil case in the file token store to prevent potential data loss during token saves, and simplifying the merging logic in the handler by removing redundant conditional branches.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
![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: a6ca9ea607
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
The token store previously re-merged on-disk metadata on every save. That preserved fields omitted from a partial update, but it also resurrected keys a caller intentionally removed: clearing all custom headers via PATCH /auth-files/fields would reappear after the next save/reload. Field preservation already happens at the handler layer, where intent is known: mergeAuthFileStatusMetadata reads the on-disk metadata and merges it under the in-memory record and the request before persisting. The store now writes exactly the metadata it is given (only backfilling type and setting disabled), so deletions stick while existing fields are still preserved by the handler. Also layer the status-patch merge as disk -> in-memory -> request so an in-memory update (e.g. a token refresh not yet flushed) is not overwritten by the on-disk baseline. Tests: - store writes metadata verbatim and drops keys absent from the update - clearing all headers via the fields patch persists to disk - existing status-patch preservation still holds
drwpls
force-pushed
the
pr/preserve-auth-metadata
branch
from
a6ca9ea to
7eee71f
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7eee71f236
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…patch The scheduler derives an account's priority from Attributes["priority"], not Metadata. When the status patch merged a priority preserved from disk into the record's metadata, it did not mirror it into attributes, so re-enabling an account whose in-memory attributes lacked the priority scheduled it at 0 until the next reload. Sync the merged priority into attributes (as the fields patch already does). Test: status patch on a partial record syncs the disk priority into attributes.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f18c00232e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Extend the status-patch repair beyond priority: when the merge pulls metadata off disk into a partial in-memory record, mirror every runtime-affecting root (custom headers, proxy_url, prefix, priority, note, websockets) into the runtime fields/attributes via syncAuthFileMetadataFields. Otherwise a re-enabled credential could run without its configured headers or proxy until a later file reload, since executors read those from attributes/Auth fields. Test: status patch on a partial record syncs priority, proxy_url and custom headers from the merged metadata.
|
@hkfires bro can you have a look |
1 participant
Summary
When updating an auth file via the management API (status/fields patch), the save path previously serialized only the in-memory metadata payload. Any field that existed only on disk — and was not part of the update — was silently dropped. For Codex accounts this notably lost the account
priorityand subscription metadata on a simple enable/disable toggle.This PR makes the save merge the existing on-disk metadata with the incoming update, so fields the caller did not send are preserved.
Behavior
typeis backfilled only when missing/unknown, and thedisabledflag is always written.priority) instead of overwriting the record wholesale.Changes
sdk/auth/filestore.go— addmergeAuthMetadataForSave(path, metadata, provider, disabled); the token storeSavenow merges with on-disk metadata before writing.internal/api/handlers/management/auth_files.go—mergeAuthFileStatusMetadata/readAuthMetadataForStatusPatchpreserve existing metadata and Codex priority on a status patch.internal/api/handlers/management/config_lists.go— keep metadata intact on the related delete path.prioritysurvives a partial-metadata save.Testing
go build ./...go test ./internal/api/handlers/management/... ./sdk/auth/...— all passgofmt/go vetclean