Skip to content

fix(resilience): widen combo cooldown-wait gate to all strategies - #8573

Closed
hartmark wants to merge 1 commit into
diegosouzapw:release/v3.8.49from
hartmark:fix/combo-cooldown-wait-gate
Closed

hartmark wants to merge 1 commit into
diegosouzapw:release/v3.8.49from
hartmark:fix/combo-cooldown-wait-gate

Conversation

@hartmark

Copy link
Copy Markdown
Contributor

Summary

Fixes #8541.

isComboCooldownWaitEligible() in comboConfig.ts was restricted to quota-share and auto strategies, but the cooldown-wait decision body in combo.ts, its inline comments, and the priority test all describe universal behavior per #7301. The restriction arrived in #8213 when extracting the predicate — incidental to the timeout-floor coupling, not a deliberate scope reduction.

Changes

  • open-sse/services/comboConfig.ts: Remove strategy check from isComboCooldownWaitEligible — now returns comboCooldownWait.enabled for all strategies (the _strategy param is kept for API compatibility)
  • tests/unit/combo-config.test.ts: Update test to assert all strategies are eligible when the feature is enabled; update resolveComboTargetTimeoutMsForCombo test to expect raised floor for all strategies
  • tests/unit/serial/combo-quota-share-cooldown-wait-timing.test.ts: Status assertion changes from 429→403 in the quota_exhausted test because the lockout path now runs for priority strategy (the security property — no wait+redispatch on quota_exhausted — still holds)

Verification

  • combo-config.test.ts: 44/44 pass
  • combo-quota-share-cooldown-wait-timing.test.ts: 4/4 pass (including the previously failing priority test)
  • ESLint: clean
  • lint-staged: passed on commit

isComboCooldownWaitEligible() was restricted to quota-share/auto only,
but the combo.ts comments, the cooldown-wait decision body, and the
priority test all describe universal behavior per diegosouzapw#7301. The restriction
arrived in diegosouzapw#8213 when extracting the predicate — incidental to the
timeout-floor coupling, not a deliberate scope reduction.

- Remove strategy check from isComboCooldownWaitEligible (now returns
  comboCooldownWait.enabled for all strategies)
- Update combo-config unit test to assert all strategies are eligible
- Update combo-quota-share-cooldown-wait-timing test: status assertion
  changes from 429→403 because the lockout path now runs for priority

Closes diegosouzapw#8541
@hartmark
hartmark requested a review from diegosouzapw as a code owner July 25, 2026 13:15
@hartmark

Copy link
Copy Markdown
Contributor Author

Closing in favor of #8559 — the reporter's PR is more thorough. It fixes both the gate AND the entry condition in combo.ts (decouples the cooldown-wait decision from whichever target last overwrote status), plus updates UI strings, docs, and type comments. My PR only widens the gate but leaves the status === 429 check that still gets bypassed when a later target returns 403.

@hartmark hartmark closed this Jul 25, 2026
@hartmark
hartmark deleted the fix/combo-cooldown-wait-gate branch July 25, 2026 14:09
@hartmark

Copy link
Copy Markdown
Contributor Author

Another PR already exists
#8559

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.

fix(resilience): combo cooldown-wait gate still quota-share/auto only, contradicting its own "ALL strategies" contract (#7301)

1 participant