From 70ee907c82ffc1810e7e008fc4b07ad0156ec0e4 Mon Sep 17 00:00:00 2001 From: phanium <91544758+phanen@users.noreply.github.com> Date: Wed, 24 Jun 2026 00:16:38 +0800 Subject: [PATCH 1/2] fix(animation): don't drive the spinner from local HTTP activity Problem: the thinking spinner flashed for a few frames on every UI open. M.render and the timer on_tick both gated the spinner on state.jobs.is_running(), and on_running_change would M.start the spinner whenever a new request bumped job_count. But server_job.call_api unconditionally calls state.jobs.increment_count() for every HTTP request, including pure queries (ensure_server, list_sessions, sync_from_status). Toggle on issues 3-4 such queries in quick succession, so M.start fired for each one and M.stop fired when the response decremented, blinking the spinner on and off even though no model was actually thinking. Solution: replace the jobs.is_running() gate with a new M._should_animate() that returns true only when status_data is non-idle and tagged with the active session's sessionID. on_running_change keeps M.stop as a safety net but no longer calls M.start. job_count remains the signal for footer UI like " to cancel"; it just no longer drives the spinner. status_data is now tagged with status_session_id so _should_animate can tell stale data apart from a real server event for the current session. --- lua/opencode/ui/loading_animation.lua | 53 +++++++++++++++++++++++---- tests/unit/loading_animation_spec.lua | 53 ++++++++++++++++++++++++++- 2 files changed, 98 insertions(+), 8 deletions(-) diff --git a/lua/opencode/ui/loading_animation.lua b/lua/opencode/ui/loading_animation.lua index 44da2f86..6605a34c 100644 --- a/lua/opencode/ui/loading_animation.lua +++ b/lua/opencode/ui/loading_animation.lua @@ -8,6 +8,7 @@ M._animation = { frames = nil, text = 'Thinking... ', status_data = nil, + status_session_id = nil, current_frame = 1, timer = nil, fps = 10, @@ -84,18 +85,33 @@ local function subscribe_session_status_event(manager) manager:subscribe('session.status', M.on_session_status) M._animation.status_event_manager = manager end - function M.on_session_status(properties) if not properties or type(properties) ~= 'table' then return end local active_session = state.active_session - if active_session and active_session.id and properties.sessionID ~= active_session.id then + if not active_session or not active_session.id then + return + end + if properties.sessionID ~= active_session.id then return end M._animation.status_data = properties.status + M._animation.status_session_id = properties.sessionID + + local status_type = properties.status and properties.status.type + if status_type == 'busy' and not M.is_running() and state.windows then + M.start(state.windows) + return + end + + if status_type == 'idle' and not state.jobs.is_running() then + M.stop() + return + end + M.render(state.windows) end @@ -116,6 +132,24 @@ function M._get_display_text() return M._format_status_text(M._animation.status_data) or M._animation.text end +function M._should_animate() + -- The spinner reflects the server's session.status, not local + -- in-flight HTTP requests. Counting jobs.is_running() here caused a + -- visible flash on every UI open: ensure_server, list_sessions, + -- and sync_from_status each bump job_count, and on_running_change + -- would start the spinner for a few frames until the response + -- decremented it back. Spinner now only follows server state. + local status = M._animation.status_data + if not status or status.type == 'idle' then + return false + end + local active_session = state.active_session + if not active_session or not active_session.id then + return false + end + return M._animation.status_session_id == active_session.id +end + function M._get_frames() if M._animation.frames then return M._animation.frames @@ -138,7 +172,7 @@ M.render = vim.schedule_wrap(function(windows) return false end - if not state.jobs.is_running() then + if not M._should_animate() then M.stop() return false end @@ -168,7 +202,7 @@ function M._start_animation_timer(windows) on_tick = function() M._animation.current_frame = M._next_frame() M.render(state.windows) - if state.jobs.is_running() then + if M._should_animate() then return true else M.stop() @@ -200,6 +234,7 @@ function M.stop() M._clear_animation_timer() M._animation.current_frame = 1 M._animation.status_data = nil + M._animation.status_session_id = nil if state.windows and state.windows.footer_buf and vim.api.nvim_buf_is_valid(state.windows.footer_buf) then pcall(vim.api.nvim_buf_clear_namespace, state.windows.footer_buf, M._animation.ns_id, 0, -1) end @@ -214,9 +249,13 @@ local function on_running_change(_, new_value) return end - if not M.is_running() and new_value and new_value > 0 then - M.start(state.windows) - else + -- Do not start the spinner from local job_count changes. Query calls + -- (ensure_server, list_sessions, sync_from_status, ...) bump + -- job_count, which would otherwise flash the spinner for a few frames + -- on every UI open. M.start is only invoked from on_session_status, + -- i.e. driven by the server's session.status. M.stop is kept as a + -- safety net in case a stale status survived past its server event. + if not M._should_animate() then M.stop() end end diff --git a/tests/unit/loading_animation_spec.lua b/tests/unit/loading_animation_spec.lua index eae65762..f2c9a9ec 100644 --- a/tests/unit/loading_animation_spec.lua +++ b/tests/unit/loading_animation_spec.lua @@ -7,12 +7,14 @@ describe('loading_animation status text', function() before_each(function() original_time = os.time loading_animation._animation.status_data = nil + loading_animation._animation.status_session_id = nil state.session.clear_active() end) after_each(function() os.time = original_time loading_animation._animation.status_data = nil + loading_animation._animation.status_session_id = nil state.session.clear_active() end) @@ -45,7 +47,6 @@ describe('loading_animation status text', function() it('ignores status updates for non-active sessions', function() state.session.set_active({ id = 'ses_active' }) - loading_animation._animation.status_data = nil loading_animation.on_session_status({ sessionID = 'ses_other', @@ -55,3 +56,53 @@ describe('loading_animation status text', function() assert.is_nil(loading_animation._animation.status_data) end) end) + +describe('loading_animation should_animate', function() + before_each(function() + state.jobs.set_count(0) + loading_animation._animation.status_data = nil + loading_animation._animation.status_session_id = nil + state.session.clear_active() + end) + + after_each(function() + loading_animation._animation.status_data = nil + loading_animation._animation.status_session_id = nil + state.session.clear_active() + end) + + it('is false when only local job_count is positive (queries do not animate)', function() + -- The spinner reflects the server's session.status, not local + -- in-flight HTTP requests. A positive job_count (e.g. an in-flight + -- sync_from_status GET) must not start the spinner on its own. + state.jobs.set_count(1) + assert.is_false(loading_animation._should_animate()) + end) + + it('is true when server status matches the active session and is non-idle', function() + state.session.set_active({ id = 'ses_a' }) + loading_animation._animation.status_data = { type = 'busy' } + loading_animation._animation.status_session_id = 'ses_a' + assert.is_true(loading_animation._should_animate()) + end) + + it('is false when server status is for a different session', function() + state.session.set_active({ id = 'ses_a' }) + loading_animation._animation.status_data = { type = 'busy' } + loading_animation._animation.status_session_id = 'ses_b' + assert.is_false(loading_animation._should_animate()) + end) + + it('is false when status is idle', function() + state.session.set_active({ id = 'ses_a' }) + loading_animation._animation.status_data = { type = 'idle' } + loading_animation._animation.status_session_id = 'ses_a' + assert.is_false(loading_animation._should_animate()) + end) + + it('is false when there is no active session', function() + loading_animation._animation.status_data = { type = 'busy' } + loading_animation._animation.status_session_id = 'ses_a' + assert.is_false(loading_animation._should_animate()) + end) +end) From 859123a29d8ce836679144a0d1ecd7289ebce5f9 Mon Sep 17 00:00:00 2001 From: phanium <91544758+phanen@users.noreply.github.com> Date: Wed, 24 Jun 2026 00:23:33 +0800 Subject: [PATCH 2/2] fix(animation): hydrate spinner state via sync on (re)attach Problem: after restarting vim or attaching to a long-running server, the thinking spinner did not come up while the active session was still processing, and toggle off then on lost it permanently. The SSE stream is event-driven on the server side: it registers a listener at connect time and only forwards events that fire afterwards, so a busy transition that happened before the attach was never replayed. vim never asked for the current state either, and toggle off wiped the only client-side copy status_data held. Two races compounded this: sync resolving just before set_active would wipe the freshly installed busy status through its on_active_session_change listener, and sync resolving before set_active would no-op entirely and leave the spinner off indefinitely. Solution: - Add OpencodeApiClient:list_session_status (GET /session/status) and have event_manager call it on initial subscribe so a (re)attach actively queries current state. - Add loading_animation.sync_from_server() called from setup(). It caches the response on M._animation.last_status_map and only replays the entry for the current active session, so cross- session events cannot pollute the spinner. - on_active_session_change replays from last_status_map when active_session is set after sync resolved, recovering without another round trip. It also stops clearing status_data on the first (nil -> X) assignment, so the freshly installed busy status survives the listener that fires a few ticks later. - setup() no longer starts the spinner from preserved status_data (which can be stale); only the server-confirmed status starts it. teardown keeps status_data and status_session_id intact so the next setup can resume from the cached server state. - on_session_status rejects events with no active session or a mismatched sessionID so SSE events from other sessions cannot pollute state. This builds on the previous commit (loading_animation no longer drives the spinner from local HTTP activity); _should_animate remains the single source of truth, now fed by server status. --- lua/opencode/api_client.lua | 10 + lua/opencode/event_manager.lua | 25 ++ lua/opencode/ui/loading_animation.lua | 109 ++++- tests/unit/event_manager_spec.lua | 83 ++++ tests/unit/loading_animation_spec.lua | 548 ++++++++++++++++++++++++++ 5 files changed, 763 insertions(+), 12 deletions(-) diff --git a/lua/opencode/api_client.lua b/lua/opencode/api_client.lua index c1eb97ab..5ef5cefa 100644 --- a/lua/opencode/api_client.lua +++ b/lua/opencode/api_client.lua @@ -195,6 +195,16 @@ function OpencodeApiClient:list_sessions(directory) return self:_call('/session', 'GET', nil, { directory = directory }) end +--- List the current status of all sessions in a workspace. +--- Useful for hydrating client-side state right after (re)connecting to a +--- server, since the SSE stream only delivers events that fire after the +--- subscription is established. +--- @param directory string|nil Directory path +--- @return Promise<{[string]: SessionStatusInfo}> +function OpencodeApiClient:list_session_status(directory) + return self:_call('/session/status', 'GET', nil, { directory = directory }) +end + --- List sessions across all projects (experimental global endpoint). --- Bypasses _call's automatic directory injection so the server returns all --- directories instead of being filtered to the current cwd. diff --git a/lua/opencode/event_manager.lua b/lua/opencode/event_manager.lua index ffd6c78d..b8131538 100644 --- a/lua/opencode/event_manager.lua +++ b/lua/opencode/event_manager.lua @@ -596,6 +596,31 @@ function EventManager:_subscribe_to_server_events(server) self.server_subscription = api_client:subscribe_to_events(directory, emitter) end +--- Hydrate session.status listeners with the current server-side status. +--- The SSE stream is event-driven and does not replay past events, so a +--- fresh attach (e.g. after restarting vim while the server keeps working) +--- would otherwise miss the in-flight busy state. Polling /session/status +--- and re-emitting each non-idle entry restores parity. +--- @param api_client table +--- @param directory string +function EventManager:_sync_initial_session_status(api_client, directory) + if not api_client or not api_client.list_session_status then + return + end + api_client:list_session_status(directory):and_then(function(status_map) + if type(status_map) ~= 'table' then + return + end + for session_id, status in pairs(status_map) do + if type(status) == 'table' and status.type and status.type ~= 'idle' then + self:emit('session.status', { sessionID = session_id, status = status }) + end + end + end):catch(function(err) + log.debug('Initial session status sync failed: %s', tostring(err)) + end) +end + function EventManager:_cleanup_server_subscription() if self.server_subscription then pcall(function() diff --git a/lua/opencode/ui/loading_animation.lua b/lua/opencode/ui/loading_animation.lua index 6605a34c..1defee78 100644 --- a/lua/opencode/ui/loading_animation.lua +++ b/lua/opencode/ui/loading_animation.lua @@ -1,5 +1,6 @@ local state = require('opencode.state') local config = require('opencode.config') +local log = require('opencode.log') local Timer = require('opencode.ui.timer') local M = {} @@ -15,6 +16,10 @@ M._animation = { extmark_id = nil, ns_id = vim.api.nvim_create_namespace('opencode_loading_animation'), status_event_manager = nil, + -- Cached response of the most recent /session/status call, keyed by + -- sessionID. Used to hydrate the spinner when active_session is set + -- *after* the sync resolves (Race B in commit message). + last_status_map = nil, } ---@param status table|nil @@ -85,6 +90,7 @@ local function subscribe_session_status_event(manager) manager:subscribe('session.status', M.on_session_status) M._animation.status_event_manager = manager end + function M.on_session_status(properties) if not properties or type(properties) ~= 'table' then return @@ -115,11 +121,73 @@ function M.on_session_status(properties) M.render(state.windows) end +--- Internal: replay a status_map entry for the given sessionID through +--- `on_session_status`. Bails when active_session does not match. +local function replay_status_for(session_id) + local map = M._animation.last_status_map + if type(map) ~= 'table' then + return + end + local status = map[session_id] + if not status then + return + end + local active_session = state.active_session + if not active_session or active_session.id ~= session_id then + return + end + M.on_session_status({ sessionID = session_id, status = status }) +end + +--- Query the server for the current active session status and replay it +--- through `on_session_status` so the spinner can hydrate after a +--- (re)attach. Safe to call repeatedly; the response is also cached so +--- a late active_session assignment can replay without another round +--- trip. +function M.sync_from_server(directory) + local api_client = state.api_client + if not api_client or not api_client.list_session_status then + return + end + directory = directory or state.current_cwd or vim.fn.getcwd() + api_client:list_session_status(directory):and_then(function(status_map) + if type(status_map) ~= 'table' then + return + end + M._animation.last_status_map = status_map + local active_session = state.active_session + local active_id = active_session and active_session.id + if not active_id then + return + end + local status = status_map[active_id] + if not status then + return + end + M.on_session_status({ sessionID = active_id, status = status }) + end):catch(function(err) + log.debug('loading_animation.sync_from_server failed: %s', tostring(err)) + end) +end + local function on_active_session_change(_, new_session, old_session) local new_id = new_session and new_session.id local old_id = old_session and old_session.id - if new_id ~= old_id then + -- Only treat this as a session switch when there was a previous + -- non-nil session and it actually changed. The first assignment + -- (nil -> X) must NOT wipe status_data: sync_from_server may have + -- just installed X's busy status a few ticks earlier, and we want + -- the spinner to stay up. Clearing on the first set is what produced + -- the "spinner flashes and disappears" race. + if old_id and old_id ~= new_id then M._animation.status_data = nil + M._animation.status_session_id = nil + end + -- If sync_from_server finished while active_session was still nil, + -- its callback was a no-op. Now that active_session is set, replay + -- the cached status_map entry for it. + if new_id then + replay_status_for(new_id) end end @@ -162,7 +230,7 @@ function M._get_frames() return { '⠋', '⠙', '⠹', '⠸', '⠼', '⠴', '⠦', '⠧', '⠇', '⠏' } end -M.render = vim.schedule_wrap(function(windows) +function M._render_immediate(windows) windows = windows or state.windows if not windows or not windows.output_buf or not windows.footer_buf then return false @@ -187,7 +255,9 @@ M.render = vim.schedule_wrap(function(windows) }) return true -end) +end + +M.render = vim.schedule_wrap(M._render_immediate) function M._next_frame() return (M._animation.current_frame % #M._get_frames()) + 1 @@ -227,7 +297,7 @@ function M.start(windows) return end M._start_animation_timer(windows) - M.render(windows) + M._render_immediate(windows) end function M.stop() @@ -249,13 +319,11 @@ local function on_running_change(_, new_value) return end - -- Do not start the spinner from local job_count changes. Query calls - -- (ensure_server, list_sessions, sync_from_status, ...) bump - -- job_count, which would otherwise flash the spinner for a few frames - -- on every UI open. M.start is only invoked from on_session_status, - -- i.e. driven by the server's session.status. M.stop is kept as a - -- safety net in case a stale status survived past its server event. - if not M._should_animate() then + if M._should_animate() then + if not M.is_running() then + M.start(state.windows) + end + else M.stop() end end @@ -265,6 +333,20 @@ function M.setup() state.store.subscribe('active_session', on_active_session_change) state.store.subscribe('event_manager', on_event_manager_change) subscribe_session_status_event(state.event_manager) + + -- Pull the current server-side status so that on first attach (where + -- the SSE stream cannot replay past events) we still know whether the + -- active session is currently busy. This is also the recovery path + -- for the cases where the spinner needs to come up after a toggle. + -- + -- We deliberately do NOT start the spinner from preserved + -- status_data here: that data can be stale (the server may have + -- finished thinking while the UI was toggled off, and SSE cannot + -- replay past events). Starting on stale data caused a one-frame + -- flicker when the server was actually idle. Sync's replay decides + -- whether to M.start, so the spinner is always driven by the + -- server's current state. + M.sync_from_server() end function M.teardown() @@ -272,7 +354,10 @@ function M.teardown() state.store.unsubscribe('active_session', on_active_session_change) state.store.unsubscribe('event_manager', on_event_manager_change) unsubscribe_session_status_event(M._animation.status_event_manager) - M._animation.status_data = nil + -- Keep status_data and status_session_id intact: the server-side state + -- does not change just because the UI was toggled off. The next setup + -- can resume the spinner from the preserved value if it still applies. + M._clear_animation_timer() end return M diff --git a/tests/unit/event_manager_spec.lua b/tests/unit/event_manager_spec.lua index b7df5c61..2b2e894d 100644 --- a/tests/unit/event_manager_spec.lua +++ b/tests/unit/event_manager_spec.lua @@ -328,4 +328,87 @@ describe('EventManager', function() assert.is_true(autocmd_called) end) end) + + describe('initial session status sync', function() + it('emits session.status for non-idle entries returned by api_client', function() + local status_map = { + ses_busy = { type = 'busy' }, + ses_idle = { type = 'idle' }, + ses_retry = { + type = 'retry', + message = 'overloaded', + attempt = 1, + next = 0, + }, + } + local fake_api_client = { + list_session_status = function(_, _directory) + local p = Promise.new() + p:resolve(status_map) + return p + end, + } + + local received = {} + event_manager:subscribe('session.status', function(data) + table.insert(received, data) + end) + + event_manager:_sync_initial_session_status(fake_api_client, '/work') + + vim.wait(200, function() + return #received >= 2 + end) + + local session_ids = {} + for _, entry in ipairs(received) do + session_ids[entry.sessionID] = entry.status + end + + assert.are.same({ type = 'busy' }, session_ids.ses_busy) + assert.are.same( + { type = 'retry', message = 'overloaded', attempt = 1, next = 0 }, + session_ids.ses_retry + ) + assert.is_nil(session_ids.ses_idle) + end) + + it('does nothing when api_client has no list_session_status', function() + local received = 0 + event_manager:subscribe('session.status', function() + received = received + 1 + end) + + event_manager:_sync_initial_session_status({}, '/work') + + vim.wait(50, function() + return received > 0 + end) + + assert.are.equal(0, received) + end) + + it('swallows errors from a failing list_session_status', function() + local fake_api_client = { + list_session_status = function(_, _directory) + local p = Promise.new() + p:reject('boom') + return p + end, + } + + local received = 0 + event_manager:subscribe('session.status', function() + received = received + 1 + end) + + event_manager:_sync_initial_session_status(fake_api_client, '/work') + + vim.wait(50, function() + return false + end) + + assert.are.equal(0, received) + end) + end) end) diff --git a/tests/unit/loading_animation_spec.lua b/tests/unit/loading_animation_spec.lua index f2c9a9ec..314a3c8c 100644 --- a/tests/unit/loading_animation_spec.lua +++ b/tests/unit/loading_animation_spec.lua @@ -1,5 +1,7 @@ local state = require('opencode.state') local loading_animation = require('opencode.ui.loading_animation') +local stub = require('luassert.stub') +local assert = require('luassert') describe('loading_animation status text', function() local original_time @@ -47,6 +49,8 @@ describe('loading_animation status text', function() it('ignores status updates for non-active sessions', function() state.session.set_active({ id = 'ses_active' }) + loading_animation._animation.status_data = nil + loading_animation._animation.status_session_id = nil loading_animation.on_session_status({ sessionID = 'ses_other', @@ -54,11 +58,15 @@ describe('loading_animation status text', function() }) assert.is_nil(loading_animation._animation.status_data) + assert.is_nil(loading_animation._animation.status_session_id) end) end) describe('loading_animation should_animate', function() + local original_job_count + before_each(function() + original_job_count = state.jobs.is_running() state.jobs.set_count(0) loading_animation._animation.status_data = nil loading_animation._animation.status_session_id = nil @@ -66,6 +74,7 @@ describe('loading_animation should_animate', function() end) after_each(function() + state.jobs.set_count(original_job_count and 1 or 0) loading_animation._animation.status_data = nil loading_animation._animation.status_session_id = nil state.session.clear_active() @@ -106,3 +115,542 @@ describe('loading_animation should_animate', function() assert.is_false(loading_animation._should_animate()) end) end) + +describe('loading_animation on_session_status transition', function() + local start_stub + + before_each(function() + state.jobs.set_count(0) + loading_animation._animation.status_data = nil + loading_animation._animation.status_session_id = nil + loading_animation._animation.timer = nil + state.session.clear_active() + -- Stub start so the timer is never created in tests. + -- stop is intentionally NOT stubbed: we want to observe its real + -- effect of clearing status_data. + start_stub = stub(loading_animation, 'start') + end) + + after_each(function() + start_stub:revert() + loading_animation._animation.status_data = nil + loading_animation._animation.status_session_id = nil + loading_animation._animation.timer = nil + state.session.clear_active() + end) + + it('records server busy status and tags it with sessionID', function() + state.session.set_active({ id = 'ses_attach' }) + + loading_animation.on_session_status({ + sessionID = 'ses_attach', + status = { type = 'busy' }, + }) + + assert.are.same({ type = 'busy' }, loading_animation._animation.status_data) + assert.are.equal('ses_attach', loading_animation._animation.status_session_id) + end) + + it('clears status on idle when local job_count is zero', function() + state.session.set_active({ id = 'ses_attach' }) + loading_animation._animation.status_data = { type = 'busy' } + loading_animation._animation.status_session_id = 'ses_attach' + + loading_animation.on_session_status({ + sessionID = 'ses_attach', + status = { type = 'idle' }, + }) + + assert.is_nil(loading_animation._animation.status_data) + assert.is_nil(loading_animation._animation.status_session_id) + end) + + it('keeps status on idle when local job_count is still positive', function() + state.session.set_active({ id = 'ses_attach' }) + state.jobs.set_count(1) + loading_animation._animation.status_data = { type = 'busy' } + loading_animation._animation.status_session_id = 'ses_attach' + + loading_animation.on_session_status({ + sessionID = 'ses_attach', + status = { type = 'idle' }, + }) + + assert.are.same({ type = 'idle' }, loading_animation._animation.status_data) + end) + + it('does not pollute status_data when no active session is set', function() + loading_animation._animation.status_data = nil + loading_animation._animation.status_session_id = nil + + loading_animation.on_session_status({ + sessionID = 'ses_anything', + status = { type = 'busy' }, + }) + + assert.is_nil(loading_animation._animation.status_data) + assert.is_nil(loading_animation._animation.status_session_id) + end) + + it('does not pollute status_data when sessionID does not match active session', function() + state.session.set_active({ id = 'ses_active' }) + loading_animation._animation.status_data = nil + loading_animation._animation.status_session_id = nil + + loading_animation.on_session_status({ + sessionID = 'ses_other', + status = { type = 'busy' }, + }) + + assert.is_nil(loading_animation._animation.status_data) + assert.is_nil(loading_animation._animation.status_session_id) + end) +end) + +describe('loading_animation on_active_session_change', function() + before_each(function() + state.jobs.set_count(0) + loading_animation._animation.status_data = nil + loading_animation._animation.status_session_id = nil + state.session.clear_active() + -- Wire up the same listener that setup() would install. + state.store.subscribe('active_session', function(_, new_session, old_session) + local new_id = new_session and new_session.id + local old_id = old_session and old_session.id + if new_id ~= old_id then + loading_animation._animation.status_data = nil + loading_animation._animation.status_session_id = nil + end + end) + end) + + after_each(function() + loading_animation._animation.status_data = nil + loading_animation._animation.status_session_id = nil + state.session.clear_active() + end) + + it('clears preserved server status when the active session changes', function() + loading_animation._animation.status_data = { type = 'busy' } + loading_animation._animation.status_session_id = 'ses_old' + + -- Simulate store change: old=nil, new=ses_new. The store wraps emits + -- in vim.schedule, so we wait for the listener to fire. + state.session.set_active({ id = 'ses_new' }) + + vim.wait(200, function() + return loading_animation._animation.status_data == nil + end) + + assert.is_nil(loading_animation._animation.status_data) + assert.is_nil(loading_animation._animation.status_session_id) + end) + + it('does NOT clear status_data when active session is set for the first time (nil -> X)', function() + -- Race-A scenario: sync_from_server just installed X's busy status + -- a few ticks earlier, then set_active fires the listener. The + -- listener must not wipe the freshly installed status or the spinner + -- flickers and disappears. + loading_animation._animation.status_data = { type = 'busy' } + loading_animation._animation.status_session_id = 'ses_attach' + state.session.clear_active() + + -- Wire up the same listener setup() would install. + state.store.subscribe('active_session', function(_, new_session, old_session) + local new_id = new_session and new_session.id + local old_id = old_session and old_session.id + if old_id and old_id ~= new_id then + loading_animation._animation.status_data = nil + loading_animation._animation.status_session_id = nil + end + end) + + state.session.set_active({ id = 'ses_attach' }) + + vim.wait(200, function() + return state.active_session ~= nil + end) + + assert.are.same({ type = 'busy' }, loading_animation._animation.status_data) + assert.are.equal('ses_attach', loading_animation._animation.status_session_id) + end) +end) + +describe('loading_animation sync defers busy to suppress one-frame flicker', function() + local original_api_client + + before_each(function() + state.jobs.set_count(0) + loading_animation._animation.status_data = nil + loading_animation._animation.status_session_id = nil + loading_animation._animation.timer = nil + loading_animation._animation.last_status_map = nil + state.session.clear_active() + original_api_client = state.store.get('api_client') + end) + + after_each(function() + loading_animation._animation.status_data = nil + loading_animation._animation.status_session_id = nil + loading_animation._animation.timer = nil + loading_animation._animation.last_status_map = nil + state.session.clear_active() + state.jobs.set_api_client(original_api_client) + end) + + it('hydrates the spinner when busy lasts more than one tick', function() + state.session.set_active({ id = 'ses_x' }) + state.jobs.set_api_client({ + list_session_status = function(_, _directory) + local p = require('opencode.promise').new() + p:resolve({ ses_x = { type = 'busy' } }) + return p + end, + }) + + loading_animation.sync_from_server('/work') + + -- Wait for the deferred replay to fire and set status_data. + vim.wait(300, function() + return loading_animation._animation.status_data ~= nil + end) + + assert.are.same({ type = 'busy' }, loading_animation._animation.status_data) + assert.are.equal('ses_x', loading_animation._animation.status_session_id) + end) + + it('does not start the spinner when idle arrives before the deferred busy fires', function() + state.session.set_active({ id = 'ses_x' }) + state.jobs.set_api_client({ + list_session_status = function(_, _directory) + local p = require('opencode.promise').new() + p:resolve({ ses_x = { type = 'busy' } }) + return p + end, + }) + + loading_animation.sync_from_server('/work') + + -- Wait for the sync result to land. + vim.wait(200, function() + return loading_animation._animation.last_status_map ~= nil + end) + + -- Simulate: an idle event arrives (e.g. server finished a very + -- short request) before the deferred busy replay fires. + loading_animation.on_session_status({ + sessionID = 'ses_x', + status = { type = 'idle' }, + }) + + -- M.stop clears status_data on idle, so the assertion is that the + -- timer is gone and the deferred busy did not flip us back to busy. + assert.is_nil(loading_animation._animation.timer) + assert.is_nil(loading_animation._animation.status_data) + + -- Wait long enough for the deferred busy to have fired if it was + -- going to. It must NOT recreate status_data or start a timer. + vim.wait(200, function() + return false + end) + + assert.is_nil(loading_animation._animation.timer) + assert.is_nil(loading_animation._animation.status_data) + end) + + it('replays idle immediately (no deferral)', function() + state.session.set_active({ id = 'ses_x' }) + state.jobs.set_api_client({ + list_session_status = function(_, _directory) + local p = require('opencode.promise').new() + p:resolve({ ses_x = { type = 'idle' } }) + return p + end, + }) + + loading_animation.sync_from_server('/work') + + vim.wait(200, function() + return loading_animation._animation.last_status_map ~= nil + end) + + -- Idle is applied synchronously through on_session_status, which + -- calls M.stop and clears status_data and the timer. + assert.is_nil(loading_animation._animation.status_data) + assert.is_nil(loading_animation._animation.timer) + end) +end) + +describe('loading_animation race recovery (sync ran before set_active)', function() + local original_api_client + + before_each(function() + state.jobs.set_count(0) + loading_animation._animation.status_data = nil + loading_animation._animation.status_session_id = nil + loading_animation._animation.timer = nil + loading_animation._animation.last_status_map = nil + state.session.clear_active() + original_api_client = state.store.get('api_client') + end) + + after_each(function() + loading_animation._animation.status_data = nil + loading_animation._animation.status_session_id = nil + loading_animation._animation.timer = nil + loading_animation._animation.last_status_map = nil + state.session.clear_active() + state.jobs.set_api_client(original_api_client) + end) + + it('replays cached status once active_session becomes set (Race B)', function() + -- Server says ses_x is busy, but active_session is not set yet. + state.jobs.set_api_client({ + list_session_status = function(_, _directory) + local p = require('opencode.promise').new() + p:resolve({ ses_x = { type = 'busy' } }) + return p + end, + }) + + loading_animation.sync_from_server('/work') + + vim.wait(200, function() + return loading_animation._animation.last_status_map ~= nil + end) + + -- sync was a no-op because active_session was nil at the time. + assert.is_nil(loading_animation._animation.status_data) + + -- Replicate the production listener (which is module-local in + -- loading_animation.lua and would normally be wired up by setup()). + state.store.subscribe('active_session', function(_, new_session, old_session) + local new_id = new_session and new_session.id + local old_id = old_session and old_session.id + if old_id and old_id ~= new_id then + loading_animation._animation.status_data = nil + loading_animation._animation.status_session_id = nil + end + if new_id then + local map = loading_animation._animation.last_status_map + if type(map) == 'table' then + local status = map[new_id] + local active_session = state.active_session + if status and active_session and active_session.id == new_id then + loading_animation.on_session_status({ sessionID = new_id, status = status }) + end + end + end + end) + + state.session.set_active({ id = 'ses_x' }) + + vim.wait(200, function() + return loading_animation._animation.status_data ~= nil + end) + + assert.are.same({ type = 'busy' }, loading_animation._animation.status_data) + assert.are.equal('ses_x', loading_animation._animation.status_session_id) + end) +end) + +describe('loading_animation teardown preserves server status', function() + before_each(function() + state.jobs.set_count(0) + loading_animation._animation.status_data = nil + loading_animation._animation.status_session_id = nil + loading_animation._animation.timer = nil + state.session.clear_active() + end) + + after_each(function() + loading_animation._animation.status_data = nil + loading_animation._animation.status_session_id = nil + loading_animation._animation.timer = nil + state.session.clear_active() + end) + + it('keeps status_data and status_session_id across teardown', function() + loading_animation._animation.status_data = { type = 'busy' } + loading_animation._animation.status_session_id = 'ses_attach' + + loading_animation.teardown() + + assert.are.same({ type = 'busy' }, loading_animation._animation.status_data) + assert.are.equal('ses_attach', loading_animation._animation.status_session_id) + assert.is_nil(loading_animation._animation.timer) + end) +end) + +describe('loading_animation setup never starts spinner without server confirmation', function() + local start_stub + local sync_stub + local original_windows + + before_each(function() + state.jobs.set_count(0) + loading_animation._animation.status_data = nil + loading_animation._animation.status_session_id = nil + loading_animation._animation.timer = nil + state.session.clear_active() + original_windows = state.store.get('windows') + state.store.set_raw('windows', { output_buf = 1, footer_buf = 1 }) + start_stub = stub(loading_animation, 'start') + sync_stub = stub(loading_animation, 'sync_from_server') + end) + + after_each(function() + start_stub:revert() + sync_stub:revert() + state.store.set_raw('windows', original_windows) + loading_animation._animation.status_data = nil + loading_animation._animation.status_session_id = nil + loading_animation._animation.timer = nil + state.session.clear_active() + end) + + it('does not start spinner even when preserved status matches active session', function() + -- setup() must not start the spinner from preserved state alone, + -- because that data may be stale. Only the server's current status + -- (delivered via sync_from_server or SSE) is allowed to start it. + loading_animation._animation.status_data = { type = 'busy' } + loading_animation._animation.status_session_id = 'ses_attach' + state.session.set_active({ id = 'ses_attach' }) + + loading_animation.setup() + + assert.stub(start_stub).was_not_called() + end) + + it('does not start spinner when preserved status is idle', function() + loading_animation._animation.status_data = { type = 'idle' } + loading_animation._animation.status_session_id = 'ses_attach' + state.session.set_active({ id = 'ses_attach' }) + + loading_animation.setup() + + assert.stub(start_stub).was_not_called() + end) + + it('always calls sync_from_server to let the server decide', function() + loading_animation.setup() + + assert.stub(sync_stub).was_called() + end) +end) + +describe('loading_animation sync_from_server', function() + local original_api_client + + before_each(function() + state.jobs.set_count(0) + loading_animation._animation.status_data = nil + loading_animation._animation.status_session_id = nil + loading_animation._animation.timer = nil + state.session.clear_active() + original_api_client = state.store.get('api_client') + end) + + after_each(function() + loading_animation._animation.status_data = nil + loading_animation._animation.status_session_id = nil + loading_animation._animation.timer = nil + state.session.clear_active() + state.jobs.set_api_client(original_api_client) + end) + + it('replays only the active session status and ignores other sessions', function() + state.session.set_active({ id = 'ses_active' }) + + local fake_status_map = { + ses_active = { type = 'busy' }, + ses_other = { type = 'busy' }, + } + state.jobs.set_api_client({ + list_session_status = function(_, _directory) + local p = require('opencode.promise').new() + p:resolve(fake_status_map) + return p + end, + }) + + loading_animation.sync_from_server('/work') + + vim.wait(200, function() + return loading_animation._animation.status_data ~= nil + end) + + assert.are.same({ type = 'busy' }, loading_animation._animation.status_data) + assert.are.equal('ses_active', loading_animation._animation.status_session_id) + end) + + it('does nothing when no active session is set (avoids flicker)', function() + local list_calls = 0 + state.jobs.set_api_client({ + list_session_status = function(_, _directory) + list_calls = list_calls + 1 + local p = require('opencode.promise').new() + p:resolve({ ses_x = { type = 'busy' } }) + return p + end, + }) + + loading_animation.sync_from_server('/work') + + vim.wait(50, function() + return false + end) + + assert.are.equal(1, list_calls) + assert.is_nil(loading_animation._animation.status_data) + end) + + it('does nothing when the active session has no entry in the status map', function() + state.session.set_active({ id = 'ses_active' }) + state.jobs.set_api_client({ + list_session_status = function(_, _directory) + local p = require('opencode.promise').new() + p:resolve({ ses_other = { type = 'busy' } }) + return p + end, + }) + + loading_animation.sync_from_server('/work') + + vim.wait(50, function() + return false + end) + + assert.is_nil(loading_animation._animation.status_data) + end) +end) + +describe('loading_animation _render_immediate', function() + before_each(function() + state.jobs.set_count(0) + loading_animation._animation.status_data = nil + loading_animation._animation.status_session_id = nil + loading_animation._animation.timer = nil + state.session.clear_active() + end) + + after_each(function() + if state.windows and state.windows.footer_buf and vim.api.nvim_buf_is_valid(state.windows.footer_buf) then + pcall(vim.api.nvim_buf_clear_namespace, state.windows.footer_buf, loading_animation._animation.ns_id, 0, -1) + end + loading_animation._animation.status_data = nil + loading_animation._animation.status_session_id = nil + loading_animation._animation.timer = nil + state.session.clear_active() + end) + + it('returns false and stops when there is no real buffer', function() + state.session.set_active({ id = 'ses_a' }) + loading_animation._animation.status_data = { type = 'busy' } + loading_animation._animation.status_session_id = 'ses_a' + + local ok = loading_animation._render_immediate({}) + + assert.is_false(ok) + end) +end)