feat(claude): add per-account 1M context toggle with global model whitelist#2247
feat(claude): add per-account 1M context toggle with global model whitelist#2247Minidoracat wants to merge 5 commits intorouter-for-me:mainfrom
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refactors the handling of Claude's 1M context beta header, moving away from a hardcoded approach that caused compatibility issues with older models. It introduces a robust, two-tiered control mechanism: a global configuration toggle with an optional model whitelist, and a per-account opt-in setting. This change empowers operators with precise control over which Claude accounts and models utilize the 1M context, preventing errors and improving flexibility. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
| @gemini-code-assist /review |
![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 a well-designed, two-layer control mechanism for enabling Claude's 1M context feature, moving away from a hardcoded approach. The changes are comprehensive, spanning configuration, core execution logic, and management APIs, which provides great flexibility. My review includes a few suggestions to enhance robustness in boolean parsing and to optimize the performance of model whitelist checks, ensuring the new feature is both powerful and efficient.
| if v := strings.TrimSpace(authAttribute(auth, "enable_1m_context")); strings.EqualFold(v, "true") { | ||
| entry["enable_1m_context"] = true | ||
| } else if auth.Metadata != nil { | ||
| switch raw := auth.Metadata["enable_1m_context"].(type) { | ||
| case bool: | ||
| if raw { | ||
| entry["enable_1m_context"] = true | ||
| } | ||
| case string: | ||
| if strings.EqualFold(strings.TrimSpace(raw), "true") { | ||
| entry["enable_1m_context"] = true | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The logic for parsing the enable_1m_context boolean value is spread across a few checks and relies on strings.EqualFold for string values. This could be consolidated and made more robust by using strconv.ParseBool, which handles a wider range of boolean string representations (e.g., "true", "t", "1"). This refactoring improves readability and maintainability.
var is1MEnabled bool if v := strings.TrimSpace(authAttribute(auth, "enable_1m_context")); v != "" { is1MEnabled, _ = strconv.ParseBool(v) } else if auth.Metadata != nil { if val, ok := auth.Metadata["enable_1m_context"]; ok { switch v := val.(type) { case bool: is1MEnabled = v case string: is1MEnabled, _ = strconv.ParseBool(strings.TrimSpace(v)) } } } if is1MEnabled { entry["enable_1m_context"] = true }| for _, m := range cfg.Claude1MContext.Models { | ||
| if strings.EqualFold(strings.TrimSpace(m), model) { | ||
| return true | ||
| } | ||
| } |
There was a problem hiding this comment.
This loop over the model whitelist runs on every request. For better performance, consider converting cfg.Claude1MContext.Models into a map[string]struct{} when the configuration is loaded in internal/config/config.go. This would make the model check an O(1) map lookup instead of an O(n) slice iteration, which is more efficient, especially if the whitelist grows.
| if raw1M, ok := metadata["enable_1m_context"]; ok { | ||
| switch v := raw1M.(type) { | ||
| case bool: | ||
| if v { | ||
| a.Attributes["enable_1m_context"] = "true" | ||
| } | ||
| case string: | ||
| if strings.EqualFold(strings.TrimSpace(v), "true") { | ||
| a.Attributes["enable_1m_context"] = "true" | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The logic to parse the enable_1m_context boolean from metadata can be made more robust. Currently, it only checks for the string "true" (case-insensitively). Using strconv.ParseBool would align with Go's standard boolean parsing and handle other valid boolean string representations like "1", "t", etc., improving robustness.
| if raw1M, ok := metadata["enable_1m_context"]; ok { | |
| switch v := raw1M.(type) { | |
| case bool: | |
| if v { | |
| a.Attributes["enable_1m_context"] = "true" | |
| } | |
| case string: | |
| if strings.EqualFold(strings.TrimSpace(v), "true") { | |
| a.Attributes["enable_1m_context"] = "true" | |
| } | |
| } | |
| } | |
| if raw1M, ok := metadata["enable_1m_context"]; ok { | |
| var isTrue bool | |
| switch v := raw1M.(type) { | |
| case bool: | |
| isTrue = v | |
| case string: | |
| isTrue, _ = strconv.ParseBool(strings.TrimSpace(v)) | |
| } | |
| if isTrue { | |
| a.Attributes["enable_1m_context"] = "true" | |
| } | |
| } |
![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: f95c13468d
ℹ️ 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".
| if auth == nil || auth.Attributes == nil { | ||
| return false | ||
| } | ||
| if !strings.EqualFold(strings.TrimSpace(auth.Attributes["enable_1m_context"]), "true") { |
There was a problem hiding this comment.
should1MContext only reads auth.Attributes["enable_1m_context"], but auths created via UploadAuthFile are registered through registerAuthFromFile with metadata populated and no synthesized attributes. In that flow, buildAuthFileEntry now reports enable_1m_context: true from metadata, yet request execution never enables the context-1m-2025-08-07 beta header because this check returns false. This makes the new toggle silently ineffective for uploaded Claude auth files unless another path later writes the attribute.
Useful? React with 👍 / 👎.
| return resp, err | ||
| } | ||
| applyClaudeHeaders(httpReq, auth, apiKey, false, extraBetas, e.cfg) | ||
| applyClaudeHeaders(httpReq, auth, apiKey, false, extraBetas, e.cfg, baseModel) |
There was a problem hiding this comment.
The 1M gating now depends on model name, but this call passes baseModel captured before payload overrides. Since applyPayloadConfigWithRoot runs earlier and can rewrite model, should1MContext may evaluate whitelist rules against a different model than the one actually sent upstream, causing incorrect addition/removal of the 1M beta header when payload rules remap models.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Code Review
This pull request introduces a configurable toggle for Claude's 1M context beta feature, with changes spanning configuration, API handlers, and core executor logic. The implementation is well-structured and covers the necessary areas. I've included a couple of suggestions to enhance maintainability and performance by refactoring some duplicated code and optimizing the model whitelist check.
| if v := strings.TrimSpace(authAttribute(auth, "enable_1m_context")); strings.EqualFold(v, "true") { | ||
| entry["enable_1m_context"] = true | ||
| } else if auth.Metadata != nil { | ||
| switch raw := auth.Metadata["enable_1m_context"].(type) { | ||
| case bool: | ||
| if raw { | ||
| entry["enable_1m_context"] = true | ||
| } | ||
| case string: | ||
| if strings.EqualFold(strings.TrimSpace(raw), "true") { | ||
| entry["enable_1m_context"] = true | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
This block of code for checking enable_1m_context is a bit repetitive and contains logic that is also duplicated in internal/watcher/synthesizer/file.go.
To improve code clarity and maintainability, I suggest refactoring this block to be more concise. Additionally, you could extract the logic for parsing the enable_1m_context value from metadata into a shared helper function to avoid duplication across files.
For example, a helper function could look like this:
// In a shared package, e.g., internal/util func Is1MContextEnabledInMetadata(metadata map[string]any) bool { if metadata == nil { return false } raw1M, ok := metadata["enable_1m_context"] if !ok { return false } switch v := raw1M.(type) { case bool: return v case string: return strings.EqualFold(strings.TrimSpace(v), "true") } return false }| if v := strings.TrimSpace(authAttribute(auth, "enable_1m_context")); strings.EqualFold(v, "true") { | |
| entry["enable_1m_context"] = true | |
| } else if auth.Metadata != nil { | |
| switch raw := auth.Metadata["enable_1m_context"].(type) { | |
| case bool: | |
| if raw { | |
| entry["enable_1m_context"] = true | |
| } | |
| case string: | |
| if strings.EqualFold(strings.TrimSpace(raw), "true") { | |
| entry["enable_1m_context"] = true | |
| } | |
| } | |
| } | |
| enable1M := false | |
| if v := strings.TrimSpace(authAttribute(auth, "enable_1m_context")); strings.EqualFold(v, "true") { | |
| enable1M = true | |
| } else if auth.Metadata != nil { | |
| switch raw := auth.Metadata["enable_1m_context"].(type) { | |
| case bool: | |
| enable1M = raw | |
| case string: | |
| enable1M = strings.EqualFold(strings.TrimSpace(raw), "true") | |
| } | |
| } | |
| if enable1M { | |
| entry["enable_1m_context"] = true | |
| } |
| func should1MContext(auth *cliproxyauth.Auth, cfg *config.Config, model string) bool { | ||
| if cfg == nil || !cfg.Claude1MContext.Enabled { | ||
| return false | ||
| } | ||
| if auth == nil || auth.Attributes == nil { | ||
| return false | ||
| } | ||
| if !strings.EqualFold(strings.TrimSpace(auth.Attributes["enable_1m_context"]), "true") { | ||
| return false | ||
| } | ||
| // If models whitelist is empty, allow all models. | ||
| if len(cfg.Claude1MContext.Models) == 0 { | ||
| return true | ||
| } | ||
| // Check if the request model is in the whitelist. | ||
| for _, m := range cfg.Claude1MContext.Models { | ||
| if strings.EqualFold(strings.TrimSpace(m), model) { | ||
| return true | ||
| } | ||
| } | ||
| return false | ||
| } |
There was a problem hiding this comment.
The current implementation of should1MContext iterates through the Models slice on every request to check the whitelist. For better performance, especially if the model list grows, this check can be optimized by converting the slice to a map for O(1) lookups.
You can achieve this by adding a ModelSet map[string]struct{} to the Claude1MContext struct in internal/config/config.go and populating it once when the configuration is loaded. This avoids repeated string operations and iterations in the request hot path.
Here's how you could modify internal/config/config.go:
// In internal/config/config.go // 1. Add ModelSet to the struct type Claude1MContext struct { Enabled bool `yaml:"enabled" json:"enabled"` Models []string `yaml:"models" json:"models"` ModelSet map[string]struct{} `yaml:"-" json:"-"` } // 2. Populate the set in LoadConfig() after unmarshalling if len(cfg.Claude1MContext.Models) > 0 { cfg.Claude1MContext.ModelSet = make(map[string]struct{}, len(cfg.Claude1MContext.Models)) for _, m := range cfg.Claude1MContext.Models { cfg.Claude1MContext.ModelSet[strings.ToLower(strings.TrimSpace(m))] = struct{}{} } } else { cfg.Claude1MContext.ModelSet = nil }func should1MContext(auth *cliproxyauth.Auth, cfg *config.Config, model string) bool { if cfg == nil || !cfg.Claude1MContext.Enabled { return false } if auth == nil || auth.Attributes == nil { return false } if !strings.EqualFold(strings.TrimSpace(auth.Attributes["enable_1m_context"]), "true") { return false } // If models whitelist is empty (ModelSet is nil), allow all models. if cfg.Claude1MContext.ModelSet == nil { return true } // Check if the request model is in the whitelist using the pre-computed set. _, ok := cfg.Claude1MContext.ModelSet[strings.ToLower(model)] return ok }
| hi we need this plzzz |
luispater left a comment
luispater left a comment There was a problem hiding this comment.
Summary:
This PR introduces a useful two-layer 1M-context control for Claude (global config + per-account opt-in + model whitelist), and keeps header override compatibility. The overall direction is good, but there is one behavior gap that can make the feature appear enabled while not actually applying at runtime.
Key findings:
-
Blocking:
enable_1m_contextis not consistently wired for UploadAuthFile-created auths.registerAuthFromFilestores full JSON inMetadatabut does not projectenable_1m_contextintoAttributes.should1MContextuses onlyauth.Attributes["enable_1m_context"]to decide whether to addcontext-1m-2025-08-07.buildAuthFileEntryalready falls back toMetadata, so management output can reportenable_1m_context: truewhile runtime still does not enable 1M.- Please align read/write paths so runtime and management API reflect the same source of truth (e.g., populate attribute during registration, or let runtime also read metadata fallback).
-
Non-blocking: Missing tests for new behavior matrix.
- Recommended coverage:
- global enabled + account enabled + whitelisted model => header injected
- account disabled / global disabled / non-whitelisted model => header not injected
X-CPA-CLAUDE-1Moverride => header injected regardless of account toggle- UploadAuthFile path specifically (metadata-only input) to prevent this regression class
- Recommended coverage:
Test plan:
I reviewed PR metadata, changed files, full patch, and CI checks via gh (ignoring translator-path-guard per request). I did not execute local runtime tests in this review pass.
| Thanks for the thorough review @luispater! Blocking issue — fixed in
Non-blocking — test coverage: Good call on the test matrix. Will add coverage for the behavior combinations (global/account/model whitelist/header override/UploadAuthFile path) in a follow-up commit. |
Minidoracat force-pushed the feat/claude-1m-context-toggle branch from c7dfc3d to b1eb655 Compare There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b1eb65509e
ℹ️ 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".
| if auth.Attributes != nil { | ||
| enabled, _ = strconv.ParseBool(strings.TrimSpace(auth.Attributes["enable_1m_context"])) | ||
| } | ||
| if !enabled && auth.Metadata != nil { |
There was a problem hiding this comment.
should1MContext currently falls back to auth.Metadata["enable_1m_context"] whenever the parsed attribute value is false (if !enabled && auth.Metadata != nil). This makes a conflicting state (Attributes["enable_1m_context"] = "false" with metadata true) evaluate to enabled and incorrectly append context-1m-2025-08-07, even though the account-level toggle is explicitly off. The metadata fallback should be conditioned on attribute absence (or parse failure), not on the boolean result itself, so a deliberate false attribute remains authoritative.
Useful? React with 👍 / 👎.
Minidoracat force-pushed the feat/claude-1m-context-toggle branch from 5ff3181 to d7fbdcb Compare Remove hardcoded context-1m-2025-08-07 from baseBetas and replace with a two-layer control mechanism: 1. Global config (claude-1m-context.enabled + models whitelist) 2. Per-account toggle (enable_1m_context in auth JSON / management API) The X-CPA-CLAUDE-1M header override is preserved for backward compat. Changes: - config: add Claude1MContext struct (enabled + models whitelist) - executor: applyClaudeHeaders now conditionally includes 1M beta - synthesizer: parse enable_1m_context from auth JSON into Attributes - management API: PatchAuthFileFields accepts enable_1m_context - management API: buildAuthFileEntry exposes enable_1m_context - gitignore: add server binary and coverage.out
- Use strconv.ParseBool for more robust boolean parsing (auth_files, synthesizer) - Use map lookup for O(1) model whitelist check (executor)
registerAuthFromFile stores JSON fields in Metadata but does not project enable_1m_context into Attributes. Without this fallback, the 1M toggle was silently ineffective for uploaded auth files even though buildAuthFileEntry correctly reported it as enabled. Addresses review from luispater (CHANGES_REQUESTED).
12 test cases covering: - global enabled/disabled - account enabled/disabled (Attributes path) - Metadata fallback for UploadAuthFile path (bool + string) - model whitelist match/miss/empty - nil auth/config guards - Attributes precedence over Metadata - case-insensitive model matching Addresses non-blocking review feedback from luispater.
An explicit Attributes["enable_1m_context"] = "false" is now authoritative and will not be overridden by Metadata. The Metadata fallback only triggers when the key does not exist in Attributes (i.e. UploadAuthFile path where synthesizer never ran). Addresses Codex P2 review.
Minidoracat force-pushed the feat/claude-1m-context-toggle branch from d7fbdcb to 2bfa05d Compare 3 participants
概述 / Summary
新增 Claude 1M context beta（
context-1m-2025-08-07）的可控開關，支援全局設定 + 每帳號獨立切換，讓使用者可以精確控制哪些帳號和哪些模型啟用 1M context。Adds configurable control for Claude's 1M context beta (
context-1m-2025-08-07) with a two-layer mechanism: global config toggle with model whitelist + per-account opt-in. This gives operators precise control over which accounts and models use 1M context, avoiding errors on models that don't support the feature.改動詳情 / Changes
1.
internal/config/config.go— 新增設定結構 / New config structClaude1MContextstruct（Enabled bool+Models []string）claude-1m-contextYAML 欄位Claude1MContextstruct with global toggle and model whitelist2.
internal/runtime/executor/claude_executor.go— 核心邏輯 / Core logicapplyClaudeHeaders新增model參數，3 個呼叫點同步更新should1MContext()判斷函數：config enabled + auth attribute + model whitelistAnthropic-Betaheader 中加入context-1m-2025-08-07X-CPA-CLAUDE-1Mheader 覆蓋機制保留（向後相容）3.
internal/watcher/synthesizer/file.go— 合成階段 / Synthesisenable_1m_context欄位讀取（支援 bool/string 格式）auth.Attributes["enable_1m_context"]，模式與priority/note一致4.
internal/api/handlers/management/auth_files.go— 管理 API / Management APIbuildAuthFileEntry回傳enable_1m_context欄位（Attributes 優先，Metadata 回退）PatchAuthFileFields新增Enable1MContext *bool請求欄位5.
config.example.yaml/.gitignoreclaude-1m-context設定範例.gitignore加入server和coverage.out改動文件 / Changed files
internal/config/config.goClaude1MContextstruct 和 Config 欄位internal/runtime/executor/claude_executor.gointernal/watcher/synthesizer/file.goenable_1m_context到 Attributesinternal/api/handlers/management/auth_files.goenable_1m_contextconfig.example.yaml.gitignore使用方式 / Usage
config.yaml：
Auth JSON 檔案：
{ "type": "claude", "enable_1m_context": true }Management API：
啟用條件（AND）：
claude-1m-context.enabled = true（全局開關）enable_1m_context = true（帳號開關）claude-1m-context.models白名單中（空 = 全部允許）或：客戶端發送
X-CPA-CLAUDE-1Mheader（直接覆蓋，向後相容）測試計劃 / Test plan
go build ./cmd/server/編譯通過go test ./internal/runtime/executor/...通過go test ./internal/watcher/...通過go test ./internal/api/handlers/management/...通過go test ./internal/config/...通過go test ./sdk/...通過go test ./test/...整合測試通過claude-1m-context.enabled: true+ auth JSONenable_1m_context: true，白名單模型請求帶 1M beta headerX-CPA-CLAUDE-1Mheader 覆蓋機制正常運作enable_1m_context前端配套（待做）/ Frontend follow-up
管理面板 UI（Cli-Proxy-API-Management-Center）需要在 Claude 卡片操作列新增 1M toggle，調用本 PR 新增的 API。