fix: honor channel affinity skip-retry when channel is disabled

[seefs001]

Summary by CodeRabbit

Release Notes

• Bug Fixes

  • Enhanced channel affinity handling to properly detect and reject requests routed to disabled preferred channels, returning a 403 error with localized messaging when configured.

• Tests

  • Added comprehensive test coverage for channel affinity skip-retry behavior, including edge cases and metadata-based fallback scenarios.

[coderabbitai]

Walkthrough

This change enhances channel affinity handling by adding logic to skip retries when a preferred channel is disabled. The middleware now calls ShouldSkipRetryAfterChannelAffinityFailure to determine if a 403 error should be returned, while the service function is updated to properly check context flags and fallback to metadata.

Changes

Cohort / File(s)	Summary
Channel Affinity Skip-Retry Logic middleware/distributor.go, service/channel_affinity.go	Enhanced disabled preferred channel handling in the middleware to abort with 403 when skip-retry should occur. Updated ShouldSkipRetryAfterChannelAffinityFailure to check ginKeyChannelAffinitySkipRetry context key first, then fallback to channelAffinityMeta.SkipRetry.
Channel Affinity Tests service/channel_affinity_template_test.go	Added comprehensive unit test TestShouldSkipRetryAfterChannelAffinityFailure covering nil context, explicit flag setting, metadata-based skip-retry, and default behavior cases.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 When channels preferred but disabled we find,
Skip retries swiftly, no more to unwind!
Context flags checked with metadata's care,
A hop and a bound through the affinity snare! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: when a preferred channel is disabled, the code now honors the skip-retry setting to abort with a 403 response.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

📝 Coding Plan

• Generate coding plan for human review comments

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

service/channel_affinity_template_test.go (1)

119-177: Add the missing false-overrides-meta case to this table.

Right now this proves true override and metadata fallback, but not that an explicit false beats meta.SkipRetry=true. A buggy implementation like flag || meta.SkipRetry would still pass.

🧪 Suggested table entry

 		{
 			name: "explicit skip retry flag in context",
 			ctx: func() *gin.Context {
 				ctx := buildChannelAffinityTemplateContextForTest(channelAffinityMeta{
 					RuleName:   "rule-explicit-flag",
 					SkipRetry:  false,
 					UsingGroup: "default",
 					ModelName:  "gpt-5",
 				})
 				ctx.Set(ginKeyChannelAffinitySkipRetry, true)
 				return ctx
 			},
 			want: true,
 		},
+		{
+			name: "explicit false flag overrides matched rule meta",
+			ctx: func() *gin.Context {
+				ctx := buildChannelAffinityTemplateContextForTest(channelAffinityMeta{
+					RuleName:   "rule-explicit-false",
+					SkipRetry:  true,
+					UsingGroup: "default",
+					ModelName:  "gpt-5",
+				})
+				ctx.Set(ginKeyChannelAffinitySkipRetry, false)
+				return ctx
+			},
+			want: false,
+		},
 		{
 			name: "fallback to matched rule meta",

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@service/channel_affinity_template_test.go` around lines 119 - 177, Add a test
case that verifies an explicit false flag in context overrides
meta.SkipRetry=true so a buggy OR-based check (e.g., flag || meta.SkipRetry)
would fail; specifically, in TestShouldSkipRetryAfterChannelAffinityFailure add
an entry where buildChannelAffinityTemplateContextForTest is given
channelAffinityMeta with SkipRetry: true but then
ctx.Set(ginKeyChannelAffinitySkipRetry, false) is called, and assert want: false
when calling ShouldSkipRetryAfterChannelAffinityFailure.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@service/channel_affinity_template_test.go`:
- Around line 119-177: Add a test case that verifies an explicit false flag in
context overrides meta.SkipRetry=true so a buggy OR-based check (e.g., flag ||
meta.SkipRetry) would fail; specifically, in
TestShouldSkipRetryAfterChannelAffinityFailure add an entry where
buildChannelAffinityTemplateContextForTest is given channelAffinityMeta with
SkipRetry: true but then ctx.Set(ginKeyChannelAffinitySkipRetry, false) is
called, and assert want: false when calling
ShouldSkipRetryAfterChannelAffinityFailure.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 06cdd65e-db94-435a-866c-b352a79fa3e7

📥 Commits

Reviewing files that changed from the base of the PR and between ed6ff0f and b09337e.

📒 Files selected for processing (3)

• middleware/distributor.go
• service/channel_affinity.go
• service/channel_affinity_template_test.go