Repository navigation
Conversation
The warming loop woke on its interval and sent a keep-alive request without checking whether the session was still working on a real turn. On a slow single-slot model the warm request competes with the request it exists to keep alive. Execution events already bracket a busy period and every terminal outcome clears it, so track that set and defer the warm request while the session is working. The deferral is bounded by the existing warming window: a turn that never reports an outcome cannot keep warming alive past its configured duration.
|
The following comment was made by an LLM, it may be inaccurate: |
There was a problem hiding this comment.
The fix works. I ran the #56636 reproduction (a mock slow model and a 10s warming interval) from source. On the base, a keep-alive request starts 10s into the turn while the main request is still pending. With this PR, warming is deferred for the whole turn and the first keep-alive is sent only after the turn has finished. The new tests pass here and fail on the base (2 of 3), and the packages/core typecheck is clean. I also checked that the warm request itself (ctx.session.generate) doesn't emit session.execution.* events, so it can't mark the session busy or clear a real turn's busy flag.
Public API: unchanged.
One behaviour change is worth making (inline): once a turn finishes, the next warm can fire almost immediately rather than one interval later. Minor, on the tests: settle waits on a real setTimeout so the forked subscription can consume a queued event. Please replace it with something that doesn't depend on wall-clock scheduling, so the tests stay deterministic under load.
Related: #53457
| event.type === "session.execution.interrupted", | ||
| ), | ||
| Stream.runForEach((event) => { | ||
| if (event.type === "session.execution.started") working.add(event.data.sessionID) |
There was a problem hiding this comment.
When a turn finishes, last still holds the start time of its last model request, and the deferred loop wakes on its own interval-sized timer. So the first warm after a long turn fires anywhere from right away to one interval after the turn ends. In the case from the issue (a request longer than the interval), it is usually sent within seconds of the real response, when the cache is already fresh. That wasted request also takes the single slot just as the user is likely to send the next prompt. Setting sessions.get(sessionID).last to the current time on the terminal events (and waking the loop, or letting the next deferred check pick it up) would count the warming interval from when the session went idle, as the issue's expected behaviour describes.
Issue for this PR
Closes #56636
Type of change
What does this PR do?
Session warming sends a keep-alive request on a fixed interval, measured from when the last request began. It never asks whether the session is still working, so on a slow local model the warm request lands on top of the real one and the two compete for the same single slot. The reporter's logs show exactly that: the context reload is underway, and 10 minutes in a
warming sessionrequest is sent, times out after 5 minutes, and is retried every 15 minutes — all while the original request is still pending.The fix keeps the decision inside the plugin rather than the configuration: warming is only useful for an idle session, so a warm request is now skipped while the session has a real turn in flight, and warming resumes at the next interval once that turn settles.
How the in-flight state is known: the plugin subscribes to
ctx.eventand trackssession.execution.startedin a set, removing the session onsession.execution.succeeded,failedorinterrupted. These events bracket a busy period exactly, and every terminal outcome clears them — which is why I did not use thehttp.request/http.responsehooks from my earlier note: a request that fails or times out never reacheshttp.response, so a flag built on that pair would leak and stall warming forever.The skip is a deferral, not a stop, and it is bounded by the window that already exists: the expiry check still runs first, so a turn that never reports an outcome cannot keep warming alive past its configured
duration. In the reporter's case (30 minute default duration, 10 minute interval, a request that outlives both), no warm request is sent at all, and the next real request reschedules warming as usual.Scope: this fixes the contending warm requests. The separate ~1 hour session stop described in the issue and in #55305 is not touched here.
How did you verify your code works?
New test at
packages/core/test/plugin/warming.test.ts, driving the real plugin against the plugin host stub withTestClock:session.execution.startedand no terminal event receives no warm request across three intervals, and one arrives aftersession.execution.succeeded(fails without the change: three requests are sent while the session is working);Red before / green after, measured with only
warming.tsreverted: 1 pass / 2 fail, then 3 pass / 0 fail.packages/coretypecheck clean (tsc -b tsconfig.json tsconfig.tests.json), oxlint 0/0, and the neighbouringtest/plugin/plan.test.tsstill 12/12.Honest limits: this is verified at the plugin level with a stubbed clock and session API — I did not run the issue's reproduction script against a real slow provider, so the end-to-end timing on a loaded model is unverified.
Screenshots / recordings
If this is a UI change, please include a screenshot or recording.
Not a UI change.
Checklist
Added by the opencode bot (AI-generated).
Related: #53457