Skip to content

feat(payload): support persisted disable toggles #2741

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
xwil1 wants to merge 3 commits into
base: dev
Choose a base branch
from
Open

feat(payload): support persisted disable toggles #2741

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a5d991a
feat(payload): support disabling persisted rewrite rules
xwil1 Apr 12, 2026
d9d5c4e
fix(payload): ignore disabled raw params during sanitize
Xvvln Apr 12, 2026
a44447a
fix(payload): skip sanitize for disabled raw rules
Xvvln Apr 13, 2026
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
27 changes: 27 additions & 0 deletions config.example.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,33 @@ nonstream-keepalive-interval: 0
# keepalive-seconds: 15 # Default: 0 (disabled). <= 0 disables keep-alives.
# bootstrap-retries: 1 # Default: 0 (disabled). Retries before first byte is sent.

# Payload rewrite rules.
# - disabled: true skips an entire rule without deleting it.
# - disabled-params keeps parameters in YAML but prevents them from being applied.
# payload:
# default:
# - disabled: false
# models:
# - name: "gpt-*"
# protocol: "openai"
# params:
# temperature: 0.7
# top_p: 0.9
# disabled-params:
# - top_p
# override:
# - disabled: true
# models:
# - name: "gemini-*"
# params:
# candidate_count: 1
# filter:
# - disabled: false
# models:
# - name: "claude-*"
# params:
# - metadata.trace_id
#
# Signature cache validation for thinking blocks (Antigravity/Claude).
# When true (default), cached signatures are preferred and validated.
# When false, client signatures are used directly after normalization (bypass mode for testing).
Expand Down
42 changes: 42 additions & 0 deletions internal/config/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -301,19 +301,27 @@ type PayloadConfig struct {

// PayloadFilterRule describes a rule to remove specific JSON paths from matching model payloads.
type PayloadFilterRule struct {
// Disabled skips this rule entirely when true.
Disabled bool `yaml:"disabled,omitempty" json:"disabled,omitempty"`
// Models lists model entries with name pattern and protocol constraint.
Models []PayloadModelRule `yaml:"models" json:"models"`
// Params lists JSON paths (gjson/sjson syntax) to remove from the payload.
Params []string `yaml:"params" json:"params"`
// DisabledParams lists JSON paths that should remain visible in config but not be applied.
DisabledParams []string `yaml:"disabled-params,omitempty" json:"disabled-params,omitempty"`
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot Apr 12, 2026

Choose a reason for hiding this comment

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

medium

The DisabledParams field (and the keys in Params) should be sanitized during configuration loading. The current SanitizePayloadRules implementation in this file (around line 713) appears to skip these new fields and the standard Default/Override rules. Trimming these strings at load time would eliminate the need for expensive strings.TrimSpace calls and map allocations during request processing in the executor.

Copy link
Copy Markdown
Author

@xwil1 xwil1 Apr 12, 2026

Choose a reason for hiding this comment

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

Thanks. I addressed the correctness issue in d9d5c4e by teaching SanitizePayloadRules to skip validation for disabled raw params. I am intentionally deferring the broader load-time normalization and precomputation optimization for this PR to keep the fix scoped and avoid introducing unrelated behavior changes.

}

// PayloadRule describes a single rule targeting a list of models with parameter updates.
type PayloadRule struct {
// Disabled skips this rule entirely when true.
Disabled bool `yaml:"disabled,omitempty" json:"disabled,omitempty"`
// Models lists model entries with name pattern and protocol constraint.
Models []PayloadModelRule `yaml:"models" json:"models"`
// Params maps JSON paths (gjson/sjson syntax) to values written into the payload.
// For *-raw rules, values are treated as raw JSON fragments (strings are used as-is).
Params map[string]any `yaml:"params" json:"params"`
// DisabledParams lists JSON paths that should remain visible in config but not be applied.
DisabledParams []string `yaml:"disabled-params,omitempty" json:"disabled-params,omitempty"`
Comment on lines +323 to +324
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot Apr 12, 2026

Choose a reason for hiding this comment

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

P1 Badge Skip validation for disabled raw payload params

Adding disabled-params to PayloadRule here makes it possible to keep raw payload entries in config while toggling them off, but sanitizePayloadRawRules still validates every raw param and drops the whole rule on any invalid JSON (internal/config/config.go:721-747). In practice, if a disabled raw param contains an in-progress/invalid JSON fragment, config load removes the entire rule, so even enabled params in that rule stop applying; this breaks the persisted-toggle workflow for default-raw and override-raw.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Author

@xwil1 xwil1 Apr 12, 2026

Choose a reason for hiding this comment

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

Fixed in d9d5c4e. sanitizePayloadRawRules now skips JSON validation for paths listed in disabled-params, so an in-progress invalid raw fragment no longer drops the whole rule. I also added regression coverage for both cases: disabled invalid raw params keep the rule intact, while enabled invalid raw params still cause the rule to be dropped.

}

// PayloadModelRule ties a model name pattern to a specific translator protocol.
Expand Down Expand Up @@ -720,8 +728,16 @@ func sanitizePayloadRawRules(rules []PayloadRule, section string) []PayloadRule
if len(rule.Params) == 0 {
continue
}
if rule.Disabled {
out = append(out, rule)
continue
}
disabledParams := payloadDisabledParamSet(rule.DisabledParams)
invalid := false
for path, value := range rule.Params {
if payloadParamDisabled(disabledParams, path) {
Comment on lines +735 to +738
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot Apr 12, 2026

Choose a reason for hiding this comment

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

P1 Badge Skip raw JSON validation for disabled rules

sanitizePayloadRawRules now ignores disabled-params, but it still validates every raw rule even when rule.Disabled is true, so a disabled default-raw/override-raw rule with an in-progress invalid JSON fragment is dropped during config load. That breaks the persisted-toggle workflow introduced in this commit because disabling a rule is no longer enough to keep it around for later re-enable/edit, and the rule can be lost from runtime state on reload.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Author

@xwil1 xwil1 Apr 13, 2026

Choose a reason for hiding this comment

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

Fixed in a44447a. sanitizePayloadRawRules() now preserves fully disabled raw rules before validating params, so in-progress invalid JSON can stay in YAML without the rule being dropped during config sanitization. I also added focused regression coverage in internal/config/payload_sanitize_test.go for both cases: disabled raw rules are preserved, while enabled raw rules with invalid JSON are still dropped. Re-verified with go test ./internal/config -run TestSanitizePayloadRules_ and go test ./internal/runtime/executor -run TestApplyPayloadConfigWithRoot_.

continue
}
raw, ok := payloadRawString(value)
if !ok {
continue
Expand All @@ -745,6 +761,32 @@ func sanitizePayloadRawRules(rules []PayloadRule, section string) []PayloadRule
return out
}

func payloadDisabledParamSet(paths []string) map[string]struct{} {
if len(paths) == 0 {
return nil
}
out := make(map[string]struct{}, len(paths))
for _, path := range paths {
trimmed := strings.TrimSpace(path)
if trimmed == "" {
continue
}
out[trimmed] = struct{}{}
}
if len(out) == 0 {
return nil
}
return out
}

func payloadParamDisabled(disabled map[string]struct{}, path string) bool {
if len(disabled) == 0 {
return false
}
_, ok := disabled[strings.TrimSpace(path)]
return ok
}

func payloadRawString(value any) ([]byte, bool) {
switch typed := value.(type) {
case string:
Expand Down
49 changes: 49 additions & 0 deletions internal/config/payload_sanitize_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
package config

import "testing"

func TestSanitizePayloadRules_KeepsDisabledRawRuleWithInvalidJSON(t *testing.T) {
cfg := &Config{
Payload: PayloadConfig{
DefaultRaw: []PayloadRule{
{
Disabled: true,
Models: []PayloadModelRule{{Name: "gpt-*"}},
Params: map[string]any{
"metadata": `{"enabled":`,
},
},
},
},
}

cfg.SanitizePayloadRules()

if len(cfg.Payload.DefaultRaw) != 1 {
t.Fatalf("disabled raw rule should be preserved during sanitize, got %d rules", len(cfg.Payload.DefaultRaw))
}
if !cfg.Payload.DefaultRaw[0].Disabled {
t.Fatalf("disabled raw rule should remain disabled after sanitize")
}
}

func TestSanitizePayloadRules_DropsEnabledRawRuleWithInvalidJSON(t *testing.T) {
cfg := &Config{
Payload: PayloadConfig{
DefaultRaw: []PayloadRule{
{
Models: []PayloadModelRule{{Name: "gpt-*"}},
Params: map[string]any{
"metadata": `{"enabled":`,
},
},
},
},
}

cfg.SanitizePayloadRules()

if len(cfg.Payload.DefaultRaw) != 0 {
t.Fatalf("enabled raw rule with invalid JSON should be dropped, got %d rules", len(cfg.Payload.DefaultRaw))
}
}
56 changes: 51 additions & 5 deletions internal/runtime/executor/helps/payload_helpers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,14 @@ func ApplyPayloadConfigWithRoot(cfg *config.Config, model, protocol, root string
// Apply default rules: first write wins per field across all matching rules.
for i := range rules.Default {
rule := &rules.Default[i]
if !payloadModelRulesMatch(rule.Models, protocol, candidates) {
if rule.Disabled || !payloadModelRulesMatch(rule.Models, protocol, candidates) {
continue
}
disabledParams := payloadDisabledParamSet(rule.DisabledParams)
for path, value := range rule.Params {
if payloadParamDisabled(disabledParams, path) {
continue
}
fullPath := buildPayloadPath(root, path)
if fullPath == "" {
continue
Expand All @@ -64,10 +68,14 @@ func ApplyPayloadConfigWithRoot(cfg *config.Config, model, protocol, root string
// Apply default raw rules: first write wins per field across all matching rules.
for i := range rules.DefaultRaw {
rule := &rules.DefaultRaw[i]
if !payloadModelRulesMatch(rule.Models, protocol, candidates) {
if rule.Disabled || !payloadModelRulesMatch(rule.Models, protocol, candidates) {
continue
}
disabledParams := payloadDisabledParamSet(rule.DisabledParams)
for path, value := range rule.Params {
if payloadParamDisabled(disabledParams, path) {
continue
}
fullPath := buildPayloadPath(root, path)
if fullPath == "" {
continue
Expand All @@ -93,10 +101,14 @@ func ApplyPayloadConfigWithRoot(cfg *config.Config, model, protocol, root string
// Apply override rules: last write wins per field across all matching rules.
for i := range rules.Override {
rule := &rules.Override[i]
if !payloadModelRulesMatch(rule.Models, protocol, candidates) {
if rule.Disabled || !payloadModelRulesMatch(rule.Models, protocol, candidates) {
continue
}
disabledParams := payloadDisabledParamSet(rule.DisabledParams)
for path, value := range rule.Params {
if payloadParamDisabled(disabledParams, path) {
continue
}
fullPath := buildPayloadPath(root, path)
if fullPath == "" {
continue
Expand All @@ -111,10 +123,14 @@ func ApplyPayloadConfigWithRoot(cfg *config.Config, model, protocol, root string
// Apply override raw rules: last write wins per field across all matching rules.
for i := range rules.OverrideRaw {
rule := &rules.OverrideRaw[i]
if !payloadModelRulesMatch(rule.Models, protocol, candidates) {
if rule.Disabled || !payloadModelRulesMatch(rule.Models, protocol, candidates) {
continue
}
disabledParams := payloadDisabledParamSet(rule.DisabledParams)
for path, value := range rule.Params {
if payloadParamDisabled(disabledParams, path) {
continue
}
fullPath := buildPayloadPath(root, path)
if fullPath == "" {
continue
Expand All @@ -133,10 +149,14 @@ func ApplyPayloadConfigWithRoot(cfg *config.Config, model, protocol, root string
// Apply filter rules: remove matching paths from payload.
for i := range rules.Filter {
rule := &rules.Filter[i]
if !payloadModelRulesMatch(rule.Models, protocol, candidates) {
if rule.Disabled || !payloadModelRulesMatch(rule.Models, protocol, candidates) {
continue
}
disabledParams := payloadDisabledParamSet(rule.DisabledParams)
for _, path := range rule.Params {
if payloadParamDisabled(disabledParams, path) {
continue
}
fullPath := buildPayloadPath(root, path)
if fullPath == "" {
continue
Expand All @@ -151,6 +171,32 @@ func ApplyPayloadConfigWithRoot(cfg *config.Config, model, protocol, root string
return out
}

func payloadDisabledParamSet(paths []string) map[string]struct{} {
if len(paths) == 0 {
return nil
}
out := make(map[string]struct{}, len(paths))
for _, path := range paths {
trimmed := strings.TrimSpace(path)
if trimmed == "" {
continue
}
out[trimmed] = struct{}{}
}
if len(out) == 0 {
return nil
}
return out
}
Comment on lines +174 to +190
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot Apr 12, 2026

Choose a reason for hiding this comment

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

medium

The payloadDisabledParamSet function creates a new map for every matching rule that has DisabledParams. For a proxy handling many requests, these frequent allocations can impact performance. Consider pre-calculating these sets during configuration initialization or using a simple slice search if the number of disabled parameters is small (which is the typical case). If the list is small, a linear search avoids the overhead of map allocation and hashing entirely.


func payloadParamDisabled(disabled map[string]struct{}, path string) bool {
if len(disabled) == 0 {
return false
}
_, ok := disabled[strings.TrimSpace(path)]
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot Apr 13, 2026

Choose a reason for hiding this comment

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

P2 Badge Canonicalize disabled param paths before matching

ApplyPayloadConfigWithRoot normalizes rule paths via buildPayloadPath (it strips a leading . when a root is set), but payloadParamDisabled only trims whitespace. That means a rule param like .metadata.trace_id and disabled-params: ["metadata.trace_id"] are treated as different strings, so the supposedly disabled param is still applied. This breaks the new toggle behavior for rooted payload flows (for example Gemini CLI/Antigravity requests) whenever users mix equivalent dotted path forms.

Useful? React with 👍 / 👎.

return ok
}
Comment on lines +192 to +198
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot Apr 12, 2026

Choose a reason for hiding this comment

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

medium

The payloadParamDisabled function performs a strings.TrimSpace on every call. Since this is executed within a loop for every parameter of every matching rule on every request, it introduces significant redundant work. It is better to ensure that Params keys and DisabledParams entries are trimmed once during configuration loading in internal/config/config.go. This would allow this check to be a simple map lookup or a linear search through a slice without repeated string manipulations.


func payloadModelRulesMatch(rules []config.PayloadModelRule, protocol string, models []string) bool {
if len(rules) == 0 || len(models) == 0 {
return false
Expand Down
Loading
Loading