fix: restore Claude adaptive effort and OAuth refresh handling - #8
Conversation
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ensure Claude requests with effort-like thinking levels but no explicit thinking are forwarded as adaptive thinking with provider effort passthrough instead of being converted into budget_tokens. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Pass only the resolved proxy payload into token refresh so Claude health checks stop building invalid proxy contexts from the resolver metadata wrapper. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Try Anthropic's JSON token refresh contract first and fall back to form encoding only when the endpoint explicitly rejects the request format, so Claude refresh keeps working across endpoint contract drift. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
There was a problem hiding this comment.
Pull request overview
This PR restores Claude “adaptive thinking” behavior when requests only provide an effort signal (e.g., reasoning_effort) or a thinkingLevel signal, and hardens Claude OAuth refresh + health-check proxy handling, with added unit coverage for the affected paths.
Changes:
- Promote Claude effort-only / thinkingLevel-only requests to
thinking: { type: "adaptive" }with upstreamoutput_config.effortpassthrough. - Fix Claude OAuth health checks to pass the resolved proxy payload correctly and retry token refresh using form-encoding when JSON payloads are rejected.
- Add unit tests covering thinking-budget promotion, translation, proxy unwrap behavior, and Claude OAuth refresh fallback.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
open-sse/services/thinkingBudget.ts |
Promotes Claude effort/thinkingLevel signals into adaptive thinking and resolves effort from multiple sources. |
open-sse/services/tokenRefresh.ts |
Sends Claude OAuth refresh as JSON first with a form-encoded fallback on specific 400 errors. |
src/lib/tokenHealthCheck.ts |
Unwraps the resolved proxy config before calling token refresh, matching expected proxy payload shape. |
tests/unit/thinking-budget.test.mjs |
Adds coverage for Claude promotion behavior in PASSTHROUGH mode and updates existing expectations. |
tests/unit/translator-openai-to-claude.test.mjs |
Adds coverage ensuring translateRequest promotes Claude thinkingLevel-only requests to adaptive effort. |
tests/unit/token-refresh-service.test.mjs |
Adds coverage for Claude OAuth refresh JSON-first behavior and form-encoded fallback. |
tests/unit/token-health-check.test.mjs |
Adds coverage for unwrapping resolved proxy configs for refresh calls. |
| function resolveAnthropicEffort(body, cfg) { | ||
| function hasThinkingLevelSignal(body) { | ||
| const level = body?.thinkingLevel || body?.thinking_level; | ||
| return typeof level === "string" && level.trim().length > 0; |
There was a problem hiding this comment.
hasThinkingLevelSignal() treats any non-empty thinkingLevel string as a promotion signal. Because applyThinkingBudget() uses originalHasThinkingLevelSignal to force Claude requests into adaptive mode, a client value like thinkingLevel: "none" / "disabled" will now be promoted to thinking: {type:"adaptive"} instead of preserving the existing behavior where normalizeThinkingLevel() converts it to disabled thinking. Consider excluding disable values (and possibly unknown values) from this signal, or explicitly short-circuiting to a disabled thinking config when the level maps to none/disabled.
| return typeof level === "string" && level.trim().length > 0; | |
| if (typeof level !== "string") return false; | |
| const normalized = level.trim().toLowerCase(); | |
| return ( | |
| normalized === "low" || | |
| normalized === "medium" || | |
| normalized === "high" || | |
| normalized === "max" || | |
| normalized === "xhigh" | |
| ); |
Security: /api/sessions enrichment now uses listProviderConnectionMetadata() which selects only id/provider/name/display_name/email. Decryption of apiKey, accessToken, refreshToken, idToken via getProviderConnections() is no longer on this hot path. Fixes Copilot review #11. Routing memory: heuristicBreakHistory map now bounded by BOTH age (10min) AND hard size cap (1000). Re-set on insert moves the entry to the end of the insertion-ordered Map; size-cap eviction loop removes oldest entries when size exceeds 1000. >1000 distinct sessions inside the cooldown window can no longer grow the map without bound. Fixes Copilot review #1/#6. Routing perf: isAffinityValid heuristic-break #2 now finds the bound entry in scoredAlternatives and reuses its score instead of calling scoreAccount() again. Matches the 'score upfront once' contract of selectByEarliestResetFirst. Fixes Copilot review #2/#7. UI tooltip: RoutingBadge now flips below the badge when there isn't enough space above (TOOLTIP_HEIGHT_ESTIMATE_PX + gap + viewport padding). top anchors to r.bottom + gap on flip; transform switches from translateY(-100%) to translateY(0). Fixes Copilot review #3/#8. UI width: TOOLTIP_WIDTH_ESTIMATE_PX raised 240 -> 244 to match min-w-[220px] plus px-3 padding. Eliminates 4px overflow on narrow viewports. Fixes Copilot review #9. Comments: SSR mounted comment rewritten to describe the actual typeof window inline check and the eslint react-hooks/set-state-in-effect rule that blocks the useState+useEffect mounted pattern. Fixes Copilot review #10. Tests: SA-5/6 description corrected (re-test in same tick is strictly inside the 60s cooldown window; '30s later' wording removed). Fixes Copilot review #4. New tests/unit/api-sessions-route.test.mjs covers /api/sessions success enrichment with explicit secret-leak guard, orphan connectionId fallback, and simulated DB failure fallback. Fixes Copilot review #5. Version 3.8.2 -> 3.8.3. test:unit 2833/2833 PASS.
* feat(dashboard): limits page polish + smart session affinity (4 issues) PR #25 (v3.1 + v7) merge follow-up. User feedback identified 4 polish items observed on /dashboard/limits after Docker deploy: 1. AutoRefreshControl raw <input type="checkbox"> + raw <select> rendered like unstyled HTML controls — replaced checkbox with Toggle from the shared design system, restyled the select wrapper to match the rest of the dashboard (rounded bg-surface border, focus ring, custom chevron). 2. RoutingBadge hover tooltip was clipped by the Account Model Quotas card's overflow-hidden ancestor (PR #25 introduced the clipping). Now uses createPortal(..., document.body) with position:fixed and viewport edge clamping, matching the codebase's existing portal pattern in providers/[id]/page.tsx. 3. Smart session affinity continuation viability (v8). 5min SESSION_AFFINITY_WINDOW_MS unchanged (matches Anthropic's prompt cache TTL — librarian-confirmed via docs.anthropic.com prompt-caching). Two new heuristic break rules layered on top of existing hard exclusions: - affinity_break_low_quota: bound's min known remaining < 15% AND a usable alt exists (Oracle: 5% hard-floor × 3 cushion). - affinity_break_p1_too_urgent: alt score >= bound × 3 AND >= bound + 250 absolute delta. The absolute delta neutralizes near-zero misfire (e.g. bound=20, alt=61 trips 3× alone but 41-point gap doesn't justify a cache write). 60s per-session cooldown gates oscillation; hard exclusions bypass cooldown. selectByEarliestResetFirst now scores upfront and passes the list to isAffinityValid (avoids double scoring). 4. SessionsTab account name. /api/sessions now joins active sessions against provider connections via getAccountDisplayName (graceful degradation: connection lookup failure returns sessions with accountName:null without breaking the API). UI replaces raw connectionId.slice(0,10) with the resolved account name + provider tag, falling back to "Account #xxxxxx" when name is unavailable. New i18n key 'usage.connectionFallback' added across 32 locales (en + ko hand-translated, 30 placeholder). Tests: 10 new SA-1..SA-10 RED tests cover smart affinity (low-Q break, heavy-gap break, no-alt edge, missing-track edge, cooldown, hard- exclusion bypass, score<=0 special case, backwards-compat third arg). Test cooldown reset hook (__resetAffinityHeuristicCooldownForTesting) prevents cross-test pollution. Full unit suite: 2830/2830 PASS (was 2821; +9 net). Verify: prettier ✓ eslint (errors 0, warnings unchanged from baseline) ✓ typecheck:core ✓ typecheck:noimplicit:core ✓ test:unit 2830/2830 ✓ docs:sync ✓. Plan: .sisyphus/plans/limits-dashboard-polish.md Oracle review: APPROVED with 7 revisions, all applied (bg_79458ad3) Momus reviews: v1 REJECT (QA executability) → v2 OKAY (bg_999bd379, bg_ebbedf5b) * fix(dashboard): restore reset countdown on session/weekly mini-bars + scoreAccount terminal guard QuotaVisualization MiniBar now renders the reset countdown next to the label (`⏱ 0h 34m` / `6d 11h`), mirroring the per-model bar format. PR #25 introduced the dual mini-bars but accidentally dropped the countdown for the overall Session/Weekly windows, leaving the row visually flat. scoreAccount() now excludes connections with terminal testStatus (expired / banned / credits_exhausted) at scoring time, not only inside isAffinityValid. Without this guard the fall-through path of selectByEarliestResetFirst could re-select a terminal connection if its cached quotas still looked healthy. Mirrors the auth.ts contract via the shared isTerminalConnectionStatus helper. Fixes a SA-7 CI flake on Linux runners (selected.id === 'bound' instead of 'alt') without changing local behavior. CHANGELOG documents the OAuth-lane prompt-cache reality: clients can request ttl: '1h' but the server downgrades to 5m on the OAuth path (observed in production responses; tracked upstream as anthropics/claude-code#46829), so SESSION_AFFINITY_WINDOW_MS = 5min is the maximum we can rely on, not a default. * fix(security,routing,ui): address Copilot PR #26 review (8 issues) Security: /api/sessions enrichment now uses listProviderConnectionMetadata() which selects only id/provider/name/display_name/email. Decryption of apiKey, accessToken, refreshToken, idToken via getProviderConnections() is no longer on this hot path. Fixes Copilot review #11. Routing memory: heuristicBreakHistory map now bounded by BOTH age (10min) AND hard size cap (1000). Re-set on insert moves the entry to the end of the insertion-ordered Map; size-cap eviction loop removes oldest entries when size exceeds 1000. >1000 distinct sessions inside the cooldown window can no longer grow the map without bound. Fixes Copilot review #1/#6. Routing perf: isAffinityValid heuristic-break #2 now finds the bound entry in scoredAlternatives and reuses its score instead of calling scoreAccount() again. Matches the 'score upfront once' contract of selectByEarliestResetFirst. Fixes Copilot review #2/#7. UI tooltip: RoutingBadge now flips below the badge when there isn't enough space above (TOOLTIP_HEIGHT_ESTIMATE_PX + gap + viewport padding). top anchors to r.bottom + gap on flip; transform switches from translateY(-100%) to translateY(0). Fixes Copilot review #3/#8. UI width: TOOLTIP_WIDTH_ESTIMATE_PX raised 240 -> 244 to match min-w-[220px] plus px-3 padding. Eliminates 4px overflow on narrow viewports. Fixes Copilot review #9. Comments: SSR mounted comment rewritten to describe the actual typeof window inline check and the eslint react-hooks/set-state-in-effect rule that blocks the useState+useEffect mounted pattern. Fixes Copilot review #10. Tests: SA-5/6 description corrected (re-test in same tick is strictly inside the 60s cooldown window; '30s later' wording removed). Fixes Copilot review #4. New tests/unit/api-sessions-route.test.mjs covers /api/sessions success enrichment with explicit secret-leak guard, orphan connectionId fallback, and simulated DB failure fallback. Fixes Copilot review #5. Version 3.8.2 -> 3.8.3. test:unit 2833/2833 PASS. * fix(routing,ui,api): address Copilot PR #26 round 3 review (3 issues, Oracle-verified) /api/sessions now fetches metadata only for active session connectionIds: listProviderConnectionMetadata(ids?) accepts an optional id filter, the route collects distinct connectionIds from getActiveSessions(), short-circuits to no DB call on empty, and otherwise issues a single WHERE id IN (?, ?, ...) with bound parameters. Endpoint work is now proportional to active sessions, not total connections. Fixes Copilot review #NEW-3. RoutingBadge useLayoutEffect cleanup no longer calls setCoords(null). The tooltip is already gated by 'open && coords' so the cleanup state update was unnecessary and risked extra renders / strict-mode noise. Oracle verified no stale-frame race (useLayoutEffect runs synchronously before paint, updateCoords sets fresh coords on reopen). Fixes Copilot review #NEW-1. formatCountdown extracted to ProviderLimits/utils.tsx and reused from both index.tsx (per-model bars) and QuotaVisualization.tsx (Session/Weekly mini bars). Behavior preserved: <24h => h h m m, >=24h => d d h h, invalid => null. Eliminates the drift risk between the two countdown call sites. Fixes Copilot review #NEW-2. Tests: +2 in tests/unit/api-sessions-route.test.mjs covering 'no active sessions skips DB query' and 'distinct connectionIds collapse to single bound parameter, unrelated secrets never leak'. 2835/2835 unit tests pass. Oracle pre-commit verification (ses_1fc62b147ffeUtgRRb2uvBKS4x): APPROVED all 3 fixes with high confidence. Version 3.8.3 -> 3.8.4. * fix(routing,ui): address Oracle audit + Copilot R4 review (2 defects + 2 nits) QuotaVisualization.pickWindow no longer absorbs per-model quotas (D1). The previous Pass 2 fallback matched any name starting with 'weekly ' or 'session ', so a connection that only had 'weekly Sonnet (7d)' (no canonical 'weekly' row) populated the overall mini-bar AND rendered as its own per-model bar simultaneously. Pass 2 removed; only canonical 'session'/'weekly' (with or without parenthesised window) match. New regression suite tests/unit/quota-visualization-pickwindow.test.mjs. RoutingBadge tooltip is now visible on viewports shorter than the tooltip estimate (D2). Previous fix flipped vertically when space-above ran out but never clamped into the viewport, so very short viewports or tooltips taller than viewH-2*pad still rendered partially off-screen. The new placement (1) prefers the side with more room (mirrors providers/[id]/page.tsx overlay rule), (2) clamps on-screen top with transform-aware math (above-anchored uses translateY(-100%), so we require top >= tooltipHeight + padding), (3) caps the rendered element with maxHeight: calc(100vh - 16px) + overflow: hidden so tooltips larger than the viewport degrade to a clipped frame, never off-screen. Fixes Copilot R4-1. isAffinityValid now guards against scoredAlternatives missing the bound entry (N1). Previously fell through to boundScore=0, letting any positive alt trip the urgent-break rule. Now returns { valid: true } on missing-bound. Production selectByEarliestResetFirst always includes bound, so this is a future-caller footgun guard. New SA-11 test. /api/sessions metadata-lookup catch now logs a sanitized warning instead of swallowing the error silently (N2). Format: '[sessions] connection metadata lookup failed for N ids: <message>'. Includes only the count and the error message text — no connectionIds, no SQL, no secrets. Oracle pre-commit verification (ses_1fc4cce1bffePsARTIZ6x3AxlY) returned NEEDS_REVISION on D1 and D2; both fixes applied per the audit's concrete revision instructions, plus the two NITs. Test count: 2841/2841 PASS (was 2835; +6 net). Version 3.8.4 -> 3.8.5.
Summary
Why this surfaced now
src/lib/tokenHealthCheck.ts -> getAccessToken() -> open-sse/services/tokenRefresh.tsVerification