Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 32 additions & 1 deletion internal/api/handlers/management/auth_files.go
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,12 @@ func (h *Handler) listAuthFilesFromDisk(c *gin.Context) {
emailValue := gjson.GetBytes(data, "email").String()
fileData["type"] = typeValue
fileData["email"] = emailValue
if pv := gjson.GetBytes(data, "priority"); pv.Exists() {
fileData["priority"] = int(pv.Int())
}
Comment on lines +335 to +337

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Ignore invalid disk priority values instead of coercing to 0

When the auth manager is unavailable, the fallback path now emits priority for any file that merely has a priority key, but gjson.Int() coerces non-numeric values (for example "priority":"high" or "priority":null) to 0. This makes the API report a concrete priority that was never configured and can change sort/order behavior in degraded mode, while the normal in-memory path only includes priority when integer parsing succeeds.

Useful? React with 👍 / 👎.

if nv := gjson.GetBytes(data, "note"); nv.Exists() && strings.TrimSpace(nv.String()) != "" {
fileData["note"] = strings.TrimSpace(nv.String())
}
Comment on lines +338 to +340
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The expression strings.TrimSpace(nv.String()) is evaluated twice here, once in the condition and again in the assignment. This is slightly inefficient. You can improve this by storing the trimmed string in a variable and using it in both places. This will also make the code a bit more readable.

if nv := gjson.GetBytes(data, "note"); nv.Exists() {
	if trimmed := strings.TrimSpace(nv.String()); trimmed != "" {
		fileData["note"] = trimmed
	}
}

}

files = append(files, fileData)
Expand Down Expand Up @@ -415,6 +421,18 @@ func (h *Handler) buildAuthFileEntry(auth *coreauth.Auth) gin.H {
if claims := extractCodexIDTokenClaims(auth); claims != nil {
entry["id_token"] = claims
}
// Expose priority from Attributes (set by synthesizer from JSON "priority" field).
if p := strings.TrimSpace(authAttribute(auth, "priority")); p != "" {
if parsed, err := strconv.Atoi(p); err == nil {
entry["priority"] = parsed
}
}
// Expose note from Metadata.
if note, ok := auth.Metadata["note"].(string); ok {
if trimmed := strings.TrimSpace(note); trimmed != "" {
entry["note"] = trimmed
}
}
Comment on lines +430 to +435
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

There's an inconsistency in how the note field is accessed compared to the priority field. The synthesizer stores both priority and note in auth.Attributes, but here note is being read from auth.Metadata. While this might work in some cases because Metadata is also populated from the file, it's better to have a single, consistent source for these synthesized values.

For consistency and correctness, note should be read from auth.Attributes, just like priority. This also has the benefit of removing a redundant strings.TrimSpace call, as the synthesizer already handles trimming.

	// Expose note from Attributes (set by synthesizer from JSON "note" field).
	if note := authAttribute(auth, "note"); note != "" {
		entry["note"] = note
	}

return entry
}

Expand Down Expand Up @@ -839,7 +857,7 @@ func (h *Handler) PatchAuthFileStatus(c *gin.Context) {
c.JSON(http.StatusOK, gin.H{"status": "ok", "disabled": *req.Disabled})
}

// PatchAuthFileFields updates editable fields (prefix, proxy_url, priority) of an auth file.
// PatchAuthFileFields updates editable fields (prefix, proxy_url, priority, note) of an auth file.
func (h *Handler) PatchAuthFileFields(c *gin.Context) {
if h.authManager == nil {
c.JSON(http.StatusServiceUnavailable, gin.H{"error": "core auth manager unavailable"})
Expand All @@ -851,6 +869,7 @@ func (h *Handler) PatchAuthFileFields(c *gin.Context) {
Prefix *string `json:"prefix"`
ProxyURL *string `json:"proxy_url"`
Priority *int `json:"priority"`
Note *string `json:"note"`
}
if err := c.ShouldBindJSON(&req); err != nil {
c.JSON(http.StatusBadRequest, gin.H{"error": "invalid request body"})
Expand Down Expand Up @@ -904,6 +923,18 @@ func (h *Handler) PatchAuthFileFields(c *gin.Context) {
}
changed = true
}
if req.Note != nil {
if targetAuth.Metadata == nil {
targetAuth.Metadata = make(map[string]any)
}
trimmedNote := strings.TrimSpace(*req.Note)
if trimmedNote == "" {
delete(targetAuth.Metadata, "note")
} else {
targetAuth.Metadata["note"] = trimmedNote
}
changed = true
}
Comment on lines +926 to +937
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This block for handling req.Note is very similar to the block for req.Priority just above it. This introduces code duplication, specifically the check if targetAuth.Metadata == nil.

Consider refactoring to reduce this duplication and improve maintainability. You could combine the logic by first checking if either field needs to be updated, then initializing the metadata map if needed, and then handling each field.

For example:

	if req.Priority != nil || req.Note != nil {
		if targetAuth.Metadata == nil {
			targetAuth.Metadata = make(map[string]any)
		}

		if req.Priority != nil {
			if *req.Priority == 0 {
				delete(targetAuth.Metadata, "priority")
			} else {
				targetAuth.Metadata["priority"] = *req.Priority
			}
		}
		if req.Note != nil {
			trimmedNote := strings.TrimSpace(*req.Note)
			if trimmedNote == "" {
				delete(targetAuth.Metadata, "note")
			} else {
				targetAuth.Metadata["note"] = trimmedNote
			}
		}
		changed = true
	}


if !changed {
c.JSON(http.StatusBadRequest, gin.H{"error": "no fields to update"})
Expand Down
8 changes: 8 additions & 0 deletions internal/watcher/synthesizer/file.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,14 @@ func synthesizeFileAuths(ctx *SynthesisContext, fullPath string, data []byte) []
}
}
}
// Read note from auth file.
if rawNote, ok := metadata["note"]; ok {
if note, isStr := rawNote.(string); isStr {
if trimmed := strings.TrimSpace(note); trimmed != "" {
a.Attributes["note"] = trimmed
}
}
}
ApplyAuthExcludedModelsMeta(a, cfg, perAccountExcluded, "oauth")
// For codex auth files, extract plan_type from the JWT id_token.
if provider == "codex" {
Expand Down
Loading