Skip to content

feat(management): expose routing.session-affinity and its TTL like routing/strategy - #5447

Open
deathemperor wants to merge 3 commits into
router-for-me:devfrom
deathemperor:feat/routing-session-affinity
Open

feat(management): expose routing.session-affinity and its TTL like routing/strategy#5447
deathemperor wants to merge 3 commits into
router-for-me:devfrom
deathemperor:feat/routing-session-affinity

Conversation

@deathemperor

Copy link
Copy Markdown

What

GET / PUT / PATCH /v0/management/routing/session-affinity, the companion of routing/strategy.

GET  → {"enabled": false, "ttl": "1h"}
PUT  {"enabled": true, "ttl": "30m"}     → {"status": "ok"}
PUT  {"enabled": true}                   → keeps the TTL
PUT  {"ttl": ""}                         → clears the TTL back to the runtime default
  • ttl is validated with time.ParseDuration (must be positive); a bad body or TTL is a 400 and leaves the config untouched.
  • Persists through the same persist path as routing/strategy: comment-preserving save, then the async hot-reload that rebuilds the selector (sdk/cliproxy/service_config.go already reads Routing.SessionAffinity and the TTL on reload, so no runtime change is needed).
  • GET reports the effective TTL: the configured string, or 1h when unset, matching the runtime default.

Why

routing/strategy can be switched at runtime, but routing.session-affinity (+ session-affinity-ttl) is config-file only. A management client that flips the strategy to round-robin cannot also turn affinity on, so every request lands on a different credential and every prompt-cache read misses until the user edits config.yaml by hand. Infinitus (a macOS menu-bar account manager that drives CLIProxyAPI purely over the Management API and never touches the config file) currently has to tell the user to go edit YAML for this one knob; with this route the toggle sits next to the strategy picker.

Implementation

  • internal/api/handlers/management/config_basic.go: GetRoutingSessionAffinity, PutRoutingSessionAffinity, defaultSessionAffinityTTL next to the strategy handlers.
  • internal/api/server_management.go: three route lines under /routing/strategy.
  • internal/api/handlers/management/config_basic_session_affinity_test.go: round trip through a temp config file (values in memory and on disk), partial updates, empty TTL, rejected bodies leave the config unchanged.

Verified with gofmt -l, go vet ./internal/api/..., go build -o test-output ./cmd/server && rm test-output, go test ./internal/api/..., and go test -race -count=2 on the new tests.

🤖 Generated with Claude Code

…uting/strategy

`routing/strategy` can be read and switched at runtime, but the setting
that makes the round-robin modes usable with Claude Code's prompt cache,
`routing.session-affinity` (+ `session-affinity-ttl`), was config-file
only. A management client that flips the strategy to round-robin could
not also turn affinity on, so every request landed on a different
credential and every prompt-cache read missed until the user edited
config.yaml by hand.

This adds GET/PUT/PATCH /v0/management/routing/session-affinity.
GET returns {"enabled": bool, "ttl": "1h"} (the TTL falls back to the
runtime default when unset). PUT/PATCH accepts either or both of
{"enabled": bool, "ttl": string}; the TTL is validated with
time.ParseDuration (empty clears it back to the default) and the change
persists with the same comment-preserving save and hot-reload as
routing/strategy.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 798136de60

ℹ️ 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".

Comment on lines +363 to +364
parsed, errParse := time.ParseDuration(ttl)
if errParse != nil || parsed <= 0 {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Enforce the runtime's one-second TTL floor

When a management client sends a positive sub-second value such as {"ttl":"500ms"}, this validation accepts and persists it, but normalizedRoutingRuntimeState in sdk/cliproxy/service_config.go:49-54 clamps the selector TTL to one second while this endpoint's GET returns the raw 500ms. The successful update and readback therefore misrepresent the effective routing behavior; reject values below one second or normalize them consistently with the runtime.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fixed in ead55b5: the PUT now rejects any TTL below one second (minSessionAffinityTTL = time.Second, mirroring the clamp in normalizedRoutingRuntimeState), so the persisted value and the readback never diverge from what the selector actually uses. {"ttl":"500ms"} is covered in the bad-input test.

…nd floor

normalizedRoutingRuntimeState clamps the selector TTL to 1s, so a
persisted 500ms would read back as something the router never uses.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ead55b5742

ℹ️ 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".

Comment on lines +376 to +379
h.cfg.Routing.SessionAffinityTTL = ttl
}
if body.Enabled != nil {
h.cfg.Routing.SessionAffinity = *body.Enabled

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Persist disabled and cleared affinity values

After these keys have been set to nonzero values, sending {"enabled":false} or {"ttl":""} only clears the in-memory fields. Both RoutingConfig fields use yaml:",omitempty", while SaveConfigPreserveComments merges generated keys without pruning omitted routing keys, so the YAML retains session-affinity: true and/or the old TTL. A restart or any reload from disk therefore restores settings that this endpoint returned 200 for clearing; explicitly remove or overwrite the corresponding YAML scalars when applying these values.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fixed in 882e60b: SaveConfigPreserveComments now prunes the routing mapping to the keys the struct renders (same pruneMappingToGeneratedKeys used for the oauth maps and plugin configs), so {"enabled":false} / {"ttl":""} remove session-affinity / session-affinity-ttl from the YAML instead of only the in-memory fields; strategy survives. Covered by TestRoutingSessionAffinityClearedValuesLeaveTheFile (enable → clear → re-read the file).

routing.session-affinity and session-affinity-ttl are omitempty, and
SaveConfigPreserveComments merged generated keys without removing the
ones the struct no longer renders — so disabling affinity or clearing
the TTL through the management API left the old values in the YAML and
a reload restored them.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 882e60ba2a

ℹ️ 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".

Comment on lines +59 to +61
// routing.* keys are omitempty: a cleared session-affinity / TTL must
// leave the file too, or the next reload restores it.
pruneMappingToGeneratedKeys(original.Content[0], generated.Content[0], "routing")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Limit pruning to the affinity keys

When a config's routing mapping contains a YAML merge key or an extension key not represented by RoutingConfig, any management API save now passes the whole mapping to pruneMissingMapKeys, which permanently deletes those entries even when the request is unrelated to routing. Previously the comment-preserving merge left unmodeled nested entries intact; remove only session-affinity and session-affinity-ttl when those values are explicitly cleared rather than pruning the entire mapping.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant