feat: configurable signature cache toggle for Antigravity/Claude thinking blocks#2412
feat: configurable signature cache toggle for Antigravity/Claude thinking blocks#2412sususu98 wants to merge 2 commits intorouter-for-me:devfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request implements a configurable signature cache for Claude thinking blocks, allowing for a bypass mode where client signatures are used directly. Changes include new configuration options, atomic state management in the cache package, and updated request/response translation logic to handle signature normalization and decoding. Review feedback recommends refactoring the configuration update logic for improved clarity and extracting duplicated signature processing code into a reusable helper function.
internal/api/server.go
Outdated
| func applySignatureCacheConfig(oldCfg, cfg *config.Config) { | ||
| oldVal := true | ||
| if oldCfg != nil && oldCfg.AntigravitySignatureCacheEnabled != nil { | ||
| oldVal = *oldCfg.AntigravitySignatureCacheEnabled | ||
| } | ||
| newVal := true | ||
| if cfg.AntigravitySignatureCacheEnabled != nil { | ||
| newVal = *cfg.AntigravitySignatureCacheEnabled | ||
| } | ||
| if oldCfg == nil || oldVal != newVal { | ||
| cache.SetSignatureCacheEnabled(newVal) | ||
| if oldCfg != nil { | ||
| log.Debugf("antigravity_signature_cache_enabled updated from %t to %t", oldVal, newVal) | ||
| } else { | ||
| log.Debugf("antigravity_signature_cache_enabled toggled to %t", newVal) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
This function's logic for handling both initial setup and hot-reloading can be made more explicit and readable. By separating the two cases and using a helper to resolve the config value, the code becomes easier to follow and maintain.
func applySignatureCacheConfig(oldCfg, cfg *config.Config) {
getEnabled := func(c *config.Config) bool {
if c != nil && c.AntigravitySignatureCacheEnabled != nil {
return *c.AntigravitySignatureCacheEnabled
}
return true // Default to true
}
newVal := getEnabled(cfg)
// Initial setup
if oldCfg == nil {
cache.SetSignatureCacheEnabled(newVal)
log.Debugf("antigravity_signature_cache_enabled toggled to %t", newVal)
return
}
// Hot-reload
oldVal := getEnabled(oldCfg)
if oldVal != newVal {
cache.SetSignatureCacheEnabled(newVal)
log.Debugf("antigravity_signature_cache_enabled updated from %t to %t", oldVal, newVal)
}
}| var sigValue string | ||
| if cache.SignatureCacheEnabled() { | ||
| sigValue = fmt.Sprintf("%s#%s", cache.GetModelGroup(modelName), thoughtSignature.String()) | ||
| } else if cache.GetModelGroup(modelName) == "claude" { | ||
| sigValue = decodeSignature(thoughtSignature.String()) | ||
| } else { | ||
| sigValue = thoughtSignature.String() | ||
| } |
There was a problem hiding this comment.
This block of logic for determining the signature value is duplicated in ConvertAntigravityResponseToClaudeNonStream (lines 464-471). To improve maintainability and reduce code duplication, consider extracting this logic into a new helper function.
For example, you could create a function like this:
func getSignatureValue(modelName, signature string) string {
if cache.SignatureCacheEnabled() {
return fmt.Sprintf("%s#%s", cache.GetModelGroup(modelName), signature)
}
if cache.GetModelGroup(modelName) == "claude" {
return decodeSignature(signature)
}
return signature
}Then you can call it from both places, for instance:
sigValue := getSignatureValue(modelName, thoughtSignature.String())
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2990e28375
ℹ️ 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".
| } else if !cache.SignatureCacheEnabled() { | ||
| // Bypass mode format: "E..." (Anthropic native, 1-layer Base64) | ||
| // Re-encode to 2-layer Base64 "R..." for the backend | ||
| clientSignature = base64.StdEncoding.EncodeToString([]byte(rawSig)) | ||
| } |
There was a problem hiding this comment.
Gate cache reads when bypass mode is enabled
This bypass branch is only reached after the earlier GetCachedSignature(...) lookup has already had a chance to fill signature, so antigravity-signature-cache-enabled=false can still reuse stale cached signatures instead of honoring the client-provided signature. In practice, toggling the new flag at runtime does not fully disable cache behavior for request translation, which contradicts the new config contract and can keep sending old signatures in multi-turn conversations.
Useful? React with 👍 / 👎.
| // Bypass mode format: "E..." (Anthropic native, 1-layer Base64) | ||
| // Re-encode to 2-layer Base64 "R..." for the backend | ||
| clientSignature = base64.StdEncoding.EncodeToString([]byte(rawSig)) | ||
| } |
There was a problem hiding this comment.
Validate raw signature before base64 re-encoding
Here rawSig is base64-encoded before HasValidSignature runs, so length-based validation is weakened: a malformed raw signature shorter than the minimum can become long enough after encoding and be forwarded as thoughtSignature. That can turn what should be a dropped unsigned thinking block into an invalid backend request, especially for malformed or user-supplied signatures in bypass mode.
Useful? React with 👍 / 👎.
|
Please don't merge this yet — I'm still running additional tests. I'll update the PR once testing is complete. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9b5ea7000c
ℹ️ 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 params.CurrentThinkingText.Len() > 0 { | ||
| cache.CacheSignature(modelName, params.CurrentThinkingText.String(), thoughtSignature.String()) | ||
| // log.Debugf("Cached signature for thinking block (textLen=%d)", params.CurrentThinkingText.Len()) |
There was a problem hiding this comment.
Gate signature cache writes in bypass mode
When antigravity-signature-cache-enabled is set to false, this path still unconditionally calls cache.CacheSignature(...), so streaming Claude responses continue to populate the signature cache even though bypass mode is supposed to avoid cache behavior. In long-running bypass deployments this can still grow cache state and, after toggling the flag back to true, stale signatures collected during bypass can be reused unexpectedly.
Useful? React with 👍 / 👎.
9b5ea70 to
6da0fbf
Compare
…dation Antigravity 的 Claude thinking signature 处理新增 cache/bypass 双模式, 并为 bypass 模式实现按 SIGNATURE-CHANNEL-SPEC.md 的签名校验。 ## 签名缓存开关 新增 antigravity-signature-cache-enabled 配置项(默认 true): - cache mode(true):使用服务端缓存的签名,行为与原有逻辑完全一致 - bypass mode(false):直接使用客户端提供的签名,经过校验和归一化 支持配置热重载,运行时可切换模式。 ## Bypass 模式签名校验 校验流程: 1. 剥离历史 cache-mode 的 'modelGroup#' 前缀(如 claude#Exxxx → Exxxx) 2. 首字符必须为 'E'(单层编码)或 'R'(双层编码),否则拒绝 3. R 开头:base64 解码 → 内层必须以 'E' 开头 → 继续单层校验 4. E 开头:base64 解码 → 首字节必须为 0x12(Claude protobuf 标识) 5. 所有合法签名归一化为 R 形式(双层 base64)发往 Antigravity 后端 非法签名处理策略: - 非严格模式(默认):translator 静默丢弃无签名的 thinking block - 严格模式(antigravity-signature-bypass-strict: true): executor 层在请求发往上游前直接返回 HTTP 400 ## 严格模式 Protobuf 树校验 按 SIGNATURE-CHANNEL-SPEC.md 解析 Claude 签名的完整 protobuf 结构: - Top-level Field 2(容器)→ Field 1(渠道块) - 渠道块提取:channel_id (Field 1)、infrastructure (Field 2)、 model_text (Field 6)、field7 (Field 7) - 计算 routing_class、infrastructure_class、schema_features - 使用 google.golang.org/protobuf/encoding/protowire 解析 ## 关键实现 - resolveThinkingSignature 拆分为 resolveCacheModeSignature / resolveBypassModeSignature - hasResolvedThinkingSignature:mode-aware 签名有效性判断 (cache: len>=50 via HasValidSignature,bypass: non-empty) - validateAntigravityRequestSignatures:executor 预检, 仅在 bypass + strict 模式下拦截非法签名返回 400 - 响应侧签名缓存逻辑与 cache mode 集成 - Cache mode 行为完全保留:无 '#' 前缀的原生签名静默丢弃
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6da0fbfbbe
ℹ️ 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 cache.SignatureCacheEnabled() { | ||
| return resolveCacheModeSignature(modelName, thinkingText, rawSignature) | ||
| } | ||
| return resolveBypassModeSignature(rawSignature) |
There was a problem hiding this comment.
Limit bypass signature normalization to Claude models
When antigravity-signature-cache-enabled is false, this branch routes all models through resolveBypassModeSignature, which only accepts Claude-style E.../R... signatures. Claude-format requests targeting non-Claude models (for example Gemini flows that rely on non-Claude signature forms like skip_thought_signature_validator) will have valid thinking signatures treated as unsigned and dropped, breaking multi-turn thinking/tool-call continuity in bypass deployments. Bypass normalization should be gated to claude model group and keep legacy validation for other groups.
Useful? React with 👍 / 👎.
| if from.String() != "claude" { | ||
| return nil |
There was a problem hiding this comment.
Guard strict bypass validator by target model group
In strict bypass mode (antigravity-signature-cache-enabled=false and antigravity-signature-bypass-strict=true), validation is triggered purely by source format (claude) and always applies Claude protobuf signature rules. That means Claude-API requests for non-Claude models can be rejected with 400 even when their signatures are valid for that model family, because the checker is not scoped by target model group. Add a model-group guard before invoking ValidateClaudeBypassSignatures.
Useful? React with 👍 / 👎.
6da0fbf to
072d4f4
Compare
Add package-level comment documenting the protobuf tree structure, base64 encoding equivalence proof, output dimensions, and spec section references. Remove unreachable legacy_vertex_group dead code.
luispater
left a comment
There was a problem hiding this comment.
Summary
This PR adds a hot-reloadable toggle for Antigravity/Claude thinking signature handling:
antigravity-signature-cache-enabled(defaulttrue): current cache/validation behavior.antigravity-signature-bypass-strict(defaultfalse): optional strict protobuf-tree validation when cache is disabled.
It also fixes a streaming edge case where thinking text co-located with a signature in the same chunk could be dropped.
Key findings
- Cache mode appears to preserve existing semantics: prefer cached signatures; fall back to client-provided only if valid.
- Bypass mode normalizes client signatures to the backend-expected 2-layer base64 (
R...) and (for Claude clients) decodes back to Anthropic-nativeE...on responses when cache is disabled. - Strict bypass precheck (
400 Bad Request) happens before any upstream call (good), and is gated behind both cache disabled + strict enabled.
Tests
Added coverage for:
- Streaming: text + signature in the same chunk emits both and caches signature for full thinking text.
- Executor: strict bypass rejects invalid signatures before any upstream request; non-strict/cache mode skip precheck.
- Request/response translators: bypass normalization and signature validation helpers.
Suggestions (non-blocking)
- Consider reducing Info-level logging for bypass strict mode when cache mode is enabled (or make it conditional on bypass being active) to avoid noisy logs.
(Translator-path-guard is failing due to expected translator changes; not considered for this recommendation.)
This is an automated Codex review result and still requires manual verification by a human reviewer.
Summary
Adds a configurable toggle (
antigravity-signature-cache-enabled) to control signature cache behavior for Claude thinking blocks in the Antigravity translator. Supports two modes:true): Cached signatures are preferred and validated — existing behavior, no change.false): Client-provided signatures are used directly after format normalization. Useful for testing and deployments where signature caching is not desired.Changes
Config & Runtime
antigravity-signature-cache-enabled(default:true, hot-reloadable)internal/cache/signature_cache.go: Thread-safeatomic.Booltoggle withSetSignatureCacheEnabled()/SignatureCacheEnabled()internal/api/server.go:applySignatureCacheConfig()for initial load and hot-reload viaUpdateClients()Request Translator (bypass mode)
E...prefix, 1-layer Base64), re-encodes to 2-layer Base64 (R...) for the Antigravity backendclaude#R...) continues to work as beforeResponse Translator (bypass mode)
decodeSignature(): DecodesR...(2-layer Base64) back toE...(Anthropic native) for client consumptionTests
TestTextAndSignatureInSameChunk: Verifies co-located text+signature chunks emit both correctlyTestSignatureOnlyChunk: Verifies signature-only chunks (normal case) still cache correctlySignature Format Reference
E...R...claude#R...Testing