fix(core): bound CSI ?997 re-query with a cooldown damper (stacked on #1213)#1228
Open
DEAN-Cherry wants to merge 2 commits into
Open
fix(core): bound CSI ?997 re-query with a cooldown damper (stacked on #1213)#1228DEAN-Cherry wants to merge 2 commits into
DEAN-Cherry wants to merge 2 commits into
Conversation
…eout A terminal that answers OSC 10/11 slower than the 250ms query window — common under WezTerm, where a color-scheme change can take ~1s to settle in the OSC 11 reply — previously had its late answer silently dropped by the themeQueryPending guard, leaving the runtime theme mode stuck on the old value. Accept late replies instead: fg/bg are reset to null at the start of each query, so stale values from a prior query cannot leak in, and applyThemeMode dedupes unchanged modes. With the guard gone, themeQueryPending became write-only, so the field is removed entirely. This also means unsolicited OSC 10/11 replies can mutate the theme mode at any time; post-suspend inertness relies on the stdin data listener being detached.
…op damper The anomalyco#976 fix silently drops any ?997 notification that arrives while an OSC 10/11 query is in flight. A terminal whose colors are still settling when it notifies (settle-lag) therefore gets stuck on the pre-transition mode until some later event. Replace the drop with a bounded follow-up query: - A ?997 during an in-flight query sets a single-slot requeryPending flag instead of being discarded. - The flag is consumed on BOTH completion paths: the 250ms timeout and early completion (full reply before the timeout), so fast terminals answering with pre-settle colors still get the follow-up. - Follow-up queries are throttled by a 5s cooldown: at most one follow-up per cooldown no matter how many notifications arrive. This breaks the query→provoked-?997→re-query cycle from anomalyco#975, where the OSC round-trip itself provokes the next notification. - An idle ?997 arriving within 1s of the last query activity is treated as a follow-up candidate under the same cooldown (a fast-answering terminal completes the query before its provoked ?997 lands), closing the fast-reply bypass around the in-flight guard. - Genuine notifications outside the provoked window always start a fresh query immediately. Includes a regression test modeling the anomalyco#975 shape (each query provokes the next ?997): queries stay bounded at <=2 over 10s, matching main's behavior, plus a fast-reply variant of the same loop.
DEAN-Cherry
requested review from
Hona,
kommander,
msmps and
simonklee
as code owners
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Note
Stacked on #1213. The first commit here is #1213's Bug-A fix (honor late OSC replies); until #1213 merges, this diff includes it. Please review only the second commit.
Follows up on the review feedback in #1213: the original bounded re-query reintroduced the #975 feedback loop and missed its own target case. This PR replaces the #976 silent in-flight drop with a follow-up query behind a real loop damper, per the suggested path forward.
Problem
The #976 fix silently drops any
?997notification arriving while an OSC 10/11 query is in flight. A terminal whose colors are still settling when it notifies gets stuck on the pre-transition mode until some later event. But naively re-querying per dropped notification re-opens #975, where the OSC round-trip itself provokes the next?997.Design
?997during an in-flight query sets a single-slotrequeryPendingflag instead of being dropped.FOLLOW_UP_COOLDOWN_MS) regardless of how many notifications arrive. In the 0.1.103: DSR 997 (color-scheme updates) creates an infinite OSC 10/11 query loop under tmux on macOS #975 shape (each query provokes the next?997), queries settle at 2 instead of looping at one per 250ms.?997arriving within 1s (PROVOKED_WINDOW_MS) of the last query activity is treated as a follow-up candidate under the same cooldown. This closes the fast-reply bypass: a terminal that answers within the 250ms window completes the query before its provoked?997lands, so that notification would otherwise count as "fresh" and sustain the loop around the in-flight guard.Tests
All modeled with
ManualClockinrenderer.input.test.ts:queryThemeColorsprovokes a?99750ms later, terminal never answers → queries stay ≤ 2 over 10s of virtual time.?997lands at 50ms while idle → still ≤ 2.?997Support Node/Deno (NAPI instead of bun:ffi) #2 in-flight + fast pre-settle reply → follow-up runs immediately via early completion, settled colors applied.?997produces exactly one follow-up; further notifications within cooldown produce none.All 102 tests in
renderer.input.test.tspass; tests 2–4 fail on the Bug-A-only base (#1213), confirming they exercise this change.