diff --git a/src/handlers/chat.js b/src/handlers/chat.js index d3ea3b4..5d3f614 100644 --- a/src/handlers/chat.js +++ b/src/handlers/chat.js @@ -1807,6 +1807,33 @@ function acquireConnectFailover(triedKeys, signal, callerKey, selector = null) { return getApiKey(triedKeys, null, callerKey, selector); } +// Prompt-cache affinity for the DEVIN_CONNECT path. +// +// setStickyBinding is otherwise only reached from the two Cascade success paths, +// where it exists so a resumed cascade_id keeps its account. The connect path +// never wrote a binding, so on a DEVIN_CONNECT deployment STICKY_SESSION_ENABLED=1 +// was a no-op: getApiKey's lookup ran on every turn and always missed, and the +// candidate sort (which ends on `lastUsed` ascending) then handed each turn of a +// conversation to a DIFFERENT account on purpose. +// +// Upstream prompt caches are per-account. Replaying an identical payload on a +// second account is a fresh cache WRITE, not a read — measured on a paid Teams +// account, three accounts in a row each reported metadata #7 tag 4 (write) for +// the same body, while a repeat on the FIRST account reported tag 5 (read). +// Because a write costs roughly an order of magnitude more than a read, rotating +// mid-conversation re-pays for the whole accumulated context every turn. +// +// The connect path always acquires with modelKey=null (acquireConnectAccount and +// acquireConnectFailover both pass null), so the binding MUST be written with +// null too: bindingKey() is `callerKey\0(modelKey || '*')` and a mismatch here +// would silently never resolve on lookup. +export function bindConnectSticky(callerKey, acct) { + if (!callerKey || !acct?.id || !isStickyEnabled()) return; + try { + setStickyBinding(callerKey, null, acct.id, currentApiKeyForId(acct.id, acct.apiKey)); + } catch { /* affinity is best-effort — never fail a served request over it */ } +} + // How many times a single DEVIN_CONNECT request may hop to a fresh pooled // account after a dead-token failure. 0 disables failover (same-account // re-login still applies). Default 2 so a request survives a couple of stale @@ -2866,6 +2893,13 @@ async function _handleChatCompletionsInner(body, context = {}) { if (acct) triedKeys.push(acct.apiKey); const r = await attemptStream(acct); if (r.kind === 'ok') { + // Pin this conversation to the account that just served it, so the + // next turn reads the prompt cache back instead of re-writing it. + // Complementary to the pair-chain commit below: that keeps the + // upstream session_id stable, this keeps the ACCOUNT stable — the + // prompt cache lives on the account, so without both the next turn + // still lands elsewhere and re-writes the whole context. + bindConnectSticky(callerKey, acct); // Session continuity: commit the completed request→response pair // from the streamed result so the next turn resolves to this same // session_id via pair-chain overlap. Gate-checked + best-effort @@ -3001,6 +3035,8 @@ async function _handleChatCompletionsInner(body, context = {}) { if (acct) triedKeys.push(acct.apiKey); const r = await attempt(acct); if (r.kind === 'ok') { + // Same cache-affinity pin as the streaming path above. + bindConnectSticky(callerKey, acct); // Feed the dashboard token-usage breakdown from the DEVIN_CONNECT path // too. Without this the non-stream connect responses never reached // recordTokenUsage (only the cascade paths did), so the "Token 用量分布" diff --git a/test/connect-sticky-affinity.test.js b/test/connect-sticky-affinity.test.js new file mode 100644 index 0000000..8c1e5bb --- /dev/null +++ b/test/connect-sticky-affinity.test.js @@ -0,0 +1,77 @@ +// DEVIN_CONNECT prompt-cache affinity. +// +// setStickyBinding was only reached from the two Cascade success paths, so on a +// connect-only deployment STICKY_SESSION_ENABLED=1 bound nothing: getApiKey's +// lookup missed on every turn and the candidate sort (which ends on `lastUsed` +// ascending) then moved each turn of a conversation to a different account. +// Upstream prompt caches are per-account, so that re-wrote the whole accumulated +// context every turn instead of reading it back. +// +// The invariant this suite pins: the connect path acquires with modelKey=null +// (acquireConnectAccount / acquireConnectFailover both pass null), so the +// binding must be WRITTEN with null as well — bindingKey() is +// `callerKey\0(modelKey || '*')`, and a mismatch is silent (it degrades to the +// exact no-op this fix removes). +// +// ⚠️ sticky-session.js reads STICKY_SESSION_ENABLED into a module-load const, so +// the env has to be set before the first import. node:test gives each file its +// own process, so setting it at the top of this file is enough. + +process.env.STICKY_SESSION_ENABLED = '1'; + +import { describe, it, beforeEach } from 'node:test'; +import assert from 'node:assert/strict'; + +const sticky = await import('../src/account/sticky-session.js'); +const { bindConnectSticky } = await import('../src/handlers/chat.js'); + +const CALLER = 'api:deadbeefdeadbeefdeadbeefdeadbeef:user:abc123'; + +describe('connect sticky affinity — key shape must match the connect-path lookup', () => { + beforeEach(() => sticky.resetAllBindings()); + + it('binds under the null-modelKey slot the connect path looks up', () => { + bindConnectSticky(CALLER, { id: 'acct-1', apiKey: 'sk-live-1' }); + // acquireConnectAccount / acquireConnectFailover both call getApiKey with + // modelKey=null, so this is the only lookup that matters for the connect path. + const bound = sticky.getStickyBinding(CALLER, null); + assert.ok(bound, 'connect-path lookup (modelKey=null) must resolve'); + assert.equal(bound.accountId, 'acct-1'); + assert.equal(bound.apiKey, 'sk-live-1'); + }); + + it('a model-scoped binding would NOT satisfy the connect lookup (regression guard)', () => { + // Writing with a concrete modelKey is the mistake this fix has to avoid: + // bindingKey() would be `caller\0gpt-...` while the lookup asks for + // `caller\0*`, so every turn silently misses and rotation resumes. + sticky.setStickyBinding(CALLER, 'gpt-5-6-sol-max', 'acct-2', 'sk-live-2'); + assert.equal(sticky.getStickyBinding(CALLER, null), null, + 'model-scoped write must not be visible to the null lookup'); + }); + + it('rebinds to the account that most recently served the caller', () => { + bindConnectSticky(CALLER, { id: 'acct-1', apiKey: 'sk-live-1' }); + bindConnectSticky(CALLER, { id: 'acct-9', apiKey: 'sk-live-9' }); + assert.equal(sticky.getStickyBinding(CALLER, null).accountId, 'acct-9', + 'after a failover hop the new account owns the cache and must be pinned'); + }); +}); + +describe('connect sticky affinity — inert on every incomplete input', () => { + beforeEach(() => sticky.resetAllBindings()); + + it('no callerKey, no account, or no account id writes nothing', () => { + bindConnectSticky('', { id: 'acct-1', apiKey: 'sk-1' }); + bindConnectSticky(CALLER, null); + bindConnectSticky(CALLER, undefined); + bindConnectSticky(CALLER, { apiKey: 'sk-1' }); // env-token fallback: no pooled id + // getStickyStats().creates is cumulative for the module's lifetime, so assert + // on the binding table itself rather than the counter. + assert.equal(sticky.getStickyBinding(CALLER, null), null, 'nothing may be bound'); + }); + + it('never throws — affinity is best-effort and must not fail a served request', () => { + assert.doesNotThrow(() => bindConnectSticky(CALLER, { id: 'acct-1' })); + assert.doesNotThrow(() => bindConnectSticky(CALLER, { id: 'acct-1', apiKey: null })); + }); +});