diff --git a/lua/opencode/protocols/v2/observation.lua b/lua/opencode/protocols/v2/observation.lua index 28303df0..43cf5cc4 100644 --- a/lua/opencode/protocols/v2/observation.lua +++ b/lua/opencode/protocols/v2/observation.lua @@ -30,7 +30,7 @@ function M.new(connection, ref) ---@param sync table ---@return boolean refresh_after_event = function(resource, sync) - return resource ~= 'files' and sync.state == 'error' + return resource ~= 'files' and (sync.state == 'error' or sync.state == 'stale') end, request_resource = function(current, resource) ---@cast current OpencodeV2Observation diff --git a/lua/opencode/protocols/v2/observation/messages.lua b/lua/opencode/protocols/v2/observation/messages.lua index 9806e14c..b81db5d3 100644 --- a/lua/opencode/protocols/v2/observation/messages.lua +++ b/lua/opencode/protocols/v2/observation/messages.lua @@ -67,6 +67,9 @@ function M.ingest_snapshot(observation, messages, merge) end) else local state = observation:read() + -- Only a wholesale replacement moves entries, order and cursor together, + -- and only that makes the pages cut from the previous one stale. + observation._v2_snapshot_generation = observation._v2_snapshot_generation + 1 local entries_by_id, order = {}, {} for _, entry in ipairs(mapped) do entries_by_id[entry.id] = entries.replace(state.entries_by_id[entry.id], entry) @@ -247,26 +250,30 @@ end ---@param data table handlers['session.synthetic'] = function(observation, event, data) local metadata = data.metadata - -- Background completions carry no message id, so the snapshot is the only - -- source of the entry that records their outcome. - if normalize.is_background_notification(metadata) then - return invalidate_messages() - end - -- Only server-authored notices carry a description; plain synthetic parts are - -- already in the message snapshot, so projecting them again would duplicate. - if type(data.description) ~= 'string' or data.description == '' then - return false + local background = normalize.is_background_notification(metadata) + -- Pulling a snapshot for these would replace every loaded page with the + -- newest one, and the entry is already reachable from the event's id. + if not background then + -- Plain synthetic parts are already in the snapshot; only notices carry a + -- description meant for the transcript. + if type(data.description) ~= 'string' or data.description == '' then + return false + end end if type(data.text) ~= 'string' then return invalid(observation, event.type, 'is missing text') end - return put_notice(observation, event, { + local info = { id = notice_message_id(event), type = 'synthetic', text = data.text, description = data.description, time = { created = event.created }, - }) + } + if background then + info.metadata = metadata + end + return put_notice(observation, event, info) end ---@param observation OpencodeV2Observation @@ -473,7 +480,37 @@ end handlers['session.tool.success'] = finish_tool handlers['session.tool.failed'] = finish_tool -handlers['session.revert.committed'] = invalidate_messages +---A committed revert deletes every message from its boundary onwards, so a +---loaded tail can be dropped without asking for the page again. +---@param observation OpencodeV2Observation +---@param event OpencodeV2Event +---@param data table +handlers['session.revert.committed'] = function(observation, event, data) + local order = observation:read().entry_order + local boundary + if type(data.to) == 'string' then + for position, id in ipairs(order) do + if id == data.to then + boundary = position + break + end + end + end + if not boundary then + -- The boundary predates the loaded window, so any loaded message may have + -- been deleted; only a snapshot can say which. + return invalidate_messages() + end + local state = observation:read() + for position = #order, boundary, -1 do + local id = order[position] + state.entries_by_id[id] = nil + observation._v2_content_by_message[id] = nil + order[position] = nil + end + return true +end + handlers['session.compaction.ended'] = invalidate_messages ---@param observation OpencodeV2Observation @@ -489,8 +526,9 @@ function M.ingest_event(observation, event) end local result = handler(observation, event, event.data) if result == 'refetch' then + -- Let `_event_changed` start the refetch: it bumps the revision first, so + -- the request it makes is the one that lands. observation:read().sync.messages = { state = 'stale' } - observation:_start_resource('messages') return true end if not result then @@ -529,7 +567,8 @@ function M.initialize(observation) observation._v2_content_by_message = {} observation._v2_older_cursor = nil observation._v2_history_complete = false - observation._v2_older_loading = false + observation._v2_older_page = nil + observation._v2_snapshot_generation = 0 end ---@param observation OpencodeV2Observation @@ -537,21 +576,29 @@ end function M.attach_history(observation, connection) ---@return Promise function observation:load_older() - if self._v2_older_loading then - boundary.fail('load_older is already in progress') - end if self._v2_history_complete or not self._v2_older_cursor then return Promise.new():resolve(nil) end - self._v2_older_loading = true + local in_flight = self._v2_older_page + if in_flight then + if in_flight.cursor == self._v2_older_cursor then + return in_flight.promise + end + -- The cursor moved while the page was in flight; queue behind it so this + -- caller reads the cursor that a rebuild left behind, not the one it saw. + return Promise.async(function() + in_flight.promise:await() + self:load_older():await() + end)() + end + local finish = self:_begin_local_operation() local cursor = self._v2_older_cursor - local revision = self._event_revisions.messages + local generation = self._v2_snapshot_generation local ok, operation_request = pcall(connection.operations.list_messages, connection, self._session_id, cursor, 50, nil) if not ok then - self._v2_older_loading = false finish() error(operation_request, 0) end @@ -560,25 +607,38 @@ function M.attach_history(observation, connection) if not self:_is_current() then boundary.fail('older messages arrived after Observation release') end - if self._event_revisions.messages ~= revision then - self:read().sync.messages = { state = 'stale' } - self:_start_resource('messages') + if self._v2_snapshot_generation ~= generation then + -- The snapshot this page was cut from is gone; the newer one already + -- carries the server's answer. return end M.apply_page(self, page, true) self:_notify('messages') end) - return request:finally(function() - self._v2_older_loading = false + local page = { cursor = cursor, promise = nil } + ---@cast page OpencodeV2OlderPage + page.promise = request:finally(function() + -- release/initialize drop the slot; a page landing after that must not + -- blank the one that replaced it + if self._v2_older_page == page then + self._v2_older_page = nil + end finish() end) + self._v2_older_page = page + return page.promise end ---@return Promise function observation:load_complete_history() return Promise.async(function() while not self._v2_history_complete and self._v2_older_cursor do + local cursor = self._v2_older_cursor self:load_older():await() + if self._v2_older_cursor == cursor then + -- Same cursor back means another round would ask the same question. + return + end end end)() end @@ -595,6 +655,8 @@ function M.release(observation) observation._v2_content_by_message = {} observation._v2_older_cursor = nil observation._v2_history_complete = false + observation._v2_older_page = nil + observation._v2_snapshot_generation = 0 end return M diff --git a/lua/opencode/protocols/v2/types.lua b/lua/opencode/protocols/v2/types.lua index 7b05102b..d89961fe 100644 --- a/lua/opencode/protocols/v2/types.lua +++ b/lua/opencode/protocols/v2/types.lua @@ -301,6 +301,11 @@ ---@alias OpencodeV2EventHandler fun(observation: OpencodeV2Observation, event: OpencodeV2Event, data: table): false|'terminal'|nil +---An older page being fetched; callers asking for the same page share `promise`. +---@class OpencodeV2OlderPage +---@field cursor string +---@field promise Promise + ---@class OpencodeV2Observation: OpencodeObservation ---@field _connection OpencodeV2Connection ---@field _v2_admissions table @@ -312,7 +317,8 @@ ---@field _v2_content_by_message table|nil> ---@field _v2_older_cursor? string ---@field _v2_history_complete boolean ----@field _v2_older_loading boolean +---@field _v2_older_page? OpencodeV2OlderPage +---@field _v2_snapshot_generation integer Bumped when a wholesale snapshot replaces entries, order and cursor ---@field _v2_inbox_terminal table ---@field _v2_permission_terminal table ---@field _v2_question_terminal table diff --git a/lua/opencode/ui/navigation.lua b/lua/opencode/ui/navigation.lua index e3c97a39..d29e1ff7 100644 --- a/lua/opencode/ui/navigation.lua +++ b/lua/opencode/ui/navigation.lua @@ -94,7 +94,27 @@ function M.goto_prev_message() vim.api.nvim_win_set_cursor(win, { 1, 0 }) end -function M.goto_next_user_message() +---@param win integer +---@param direction 'next'|'prev' +---@param line integer Line the search starts from, 1-indexed +---@return boolean moved +local function jump_to_user_message(win, direction, line) + local target + if direction == 'next' then + target = renderer.get_next_user_message(line) + else + target = renderer.get_prev_user_message(line) + end + if not target or not target.line_start then + return false + end + mark_jump_position(win) + vim.api.nvim_win_set_cursor(win, { target.line_start + 1, 0 }) + return true +end + +---@param direction 'next'|'prev' +local function goto_user_message(direction) require('opencode.ui.ui').focus_output() local windows = state.windows or {} local win = windows.output_win @@ -104,42 +124,44 @@ function M.goto_next_user_message() return end - -- Like `gg`, under lazy render the target - -- message may not yet have a line_start, so force a full render first. - renderer.load_all_messages(nil, { scroll_to_top = false }) + local missing = direction == 'next' and 'No next user message' or 'No previous user message' + -- Growing the window renumbers every line below, so hold the position by id. + local anchor = renderer.get_prev_rendered_message(vim.api.nvim_win_get_cursor(win)[1] + 1) + local anchor_id = anchor and anchor.message.id + local _, started = renderer.load_all_messages(nil, { scroll_to_top = false }) - local current_line = vim.api.nvim_win_get_cursor(win)[1] - local next_message = renderer.get_next_user_message(current_line) - if next_message and next_message.line_start then - mark_jump_position(win) - vim.api.nvim_win_set_cursor(win, { next_message.line_start + 1, 0 }) + if jump_to_user_message(win, direction, vim.api.nvim_win_get_cursor(win)[1]) then return end - vim.notify('No next user message', vim.log.levels.INFO) -end - -function M.goto_prev_user_message() - require('opencode.ui.ui').focus_output() - local windows = state.windows or {} - local win = windows.output_win - local buf = windows.output_buf - - if not win or not buf then + if not started then + vim.notify(missing, vim.log.levels.INFO) return end - renderer.load_all_messages(nil, { scroll_to_top = false }) + renderer.after_history_load(function() + if not vim.api.nvim_win_is_valid(win) then + return + end + local rendered = anchor_id and renderer.get_rendered_message(anchor_id) + local line + if rendered and rendered.line_start then + line = rendered.line_start + 2 + else + line = vim.api.nvim_win_get_cursor(win)[1] + end + if not jump_to_user_message(win, direction, line) then + vim.notify(missing, vim.log.levels.INFO) + end + end) +end - local current_line = vim.api.nvim_win_get_cursor(win)[1] - local previous_message = renderer.get_prev_user_message(current_line) - if previous_message and previous_message.line_start then - mark_jump_position(win) - vim.api.nvim_win_set_cursor(win, { previous_message.line_start + 1, 0 }) - return - end +function M.goto_next_user_message() + goto_user_message('next') +end - vim.notify('No previous user message', vim.log.levels.INFO) +function M.goto_prev_user_message() + goto_user_message('prev') end ---@param raw string diff --git a/lua/opencode/ui/renderer.lua b/lua/opencode/ui/renderer.lua index fd0ab584..649bb268 100644 --- a/lua/opencode/ui/renderer.lua +++ b/lua/opencode/ui/renderer.lua @@ -836,6 +836,16 @@ local function grow_window_with_older_page(ctx) return true end +---Drop the pending navigation callback once the scroll target it was registered +---under is gone. +---@param ctx RendererCtx +local function drop_stale_callback(ctx) + local pending = ctx.after_history_load + if pending and pending.revision ~= ctx.history_navigation_revision then + ctx.after_history_load = nil + end +end + ---Pull the complete remaining history and render all of it. ---@return boolean Whether a history load was started ---@param ctx RendererCtx @@ -856,6 +866,7 @@ local function load_complete_history(ctx, scroll_to_top) end request :and_then(function() + drop_stale_callback(ctx) if not ctx:is_active() or ctx.observation ~= observation @@ -872,8 +883,14 @@ local function load_complete_history(ctx, scroll_to_top) pcall(output_window.restore_view_topline, win, 1) end ctx.history_top_pending = false + local after_load = ctx.after_history_load + ctx.after_history_load = nil + if after_load then + after_load.fn() + end end) :catch(function(err) + drop_stale_callback(ctx) if ctx.history_navigation_revision == navigation_revision then ctx.history_top_pending = false end @@ -1043,7 +1060,8 @@ end ---Load all remaining messages and re-render. ---Used for navigation and search; by default, gg lands at the true top. ----@return boolean Whether any messages were loaded +---@return boolean loaded Whether any messages were loaded +---@return boolean started Whether a complete-history pull was started ---@param ctx? RendererCtx ---@param opts? {scroll_to_top?: boolean} function M.load_all_messages(ctx, opts) @@ -1069,7 +1087,16 @@ function M.load_all_messages(ctx, opts) if not loading then ctx.history_top_pending = false end - return loading or expanded + return loading or expanded, loading +end + +---Run `fn` when the complete-history pull in flight lands; a later `gg` drops it +---instead of running it under a different scroll target. +---@param fn fun() +---@param ctx? RendererCtx +function M.after_history_load(fn, ctx) + ctx = ctx or contexts.current() + ctx.after_history_load = { fn = fn, revision = ctx.history_navigation_revision } end ---Render the currently observed state synchronously; this does not load history. diff --git a/lua/opencode/ui/renderer/ctx.lua b/lua/opencode/ui/renderer/ctx.lua index b51d7fe8..5f3c79ec 100644 --- a/lua/opencode/ui/renderer/ctx.lua +++ b/lua/opencode/ui/renderer/ctx.lua @@ -35,6 +35,7 @@ M.prompt_controllers = {} ---@field history_expanded boolean ---@field history_navigation_revision integer ---@field history_top_pending boolean +---@field after_history_load {fn: fun(), revision: integer}|nil ---@field get_child_parts fun(session_id: string): table[]|nil ---@field prompt_controllers {permission?: PermissionController, question?: QuestionController} ---@field formatted_parts table @@ -140,6 +141,7 @@ function ctx:reset() self.history_expanded = false self.history_navigation_revision = 0 self.history_top_pending = false + self.after_history_load = nil self.file_revision = 0 self.needs_reconcile = false self:bulk_reset() diff --git a/tests/unit/navigation_user_message_spec.lua b/tests/unit/navigation_user_message_spec.lua index cc475b84..ce7c6cc9 100644 --- a/tests/unit/navigation_user_message_spec.lua +++ b/tests/unit/navigation_user_message_spec.lua @@ -299,6 +299,78 @@ describe('navigation user message jumps', function() assert.equals(2, vim.api.nvim_win_get_cursor(output_win)[1]) end) + + it('jumps once the pulled history renders the target above the window', function() + seed({ + { id = 'a1', kind = 'assistant', line_start = 1 }, + }) + contexts.current().lazy_render_count = 1 + + local pending + local original_load = renderer.load_all_messages + local original_after = renderer.after_history_load + renderer.load_all_messages = function() + return true, true + end + renderer.after_history_load = function(fn) + pending = fn + end + local notify_stub = stub(vim, 'notify') + + vim.api.nvim_win_set_cursor(output_win, { 2, 0 }) + navigation.goto_prev_user_message() + + renderer.load_all_messages = original_load + renderer.after_history_load = original_after + notify_stub:revert() + + assert.is_function(pending) + assert.stub(notify_stub).was_called(0) + + -- the pull lands: an older user message joins above and everything moves down + local ctx = contexts.current() + local older = { id = 'u0', kind = 'user', content = {} } + table.insert(ctx.entries, 1, older) + ctx.render_state:set_message(older, 10, 10) + ctx.render_state:set_message(ctx.entries[2], 30, 30) + pending() + + assert.equals(11, vim.api.nvim_win_get_cursor(output_win)[1]) + end) + + it('jumps forward once the pulled history renders the target below the window', function() + seed({ + { id = 'u1', kind = 'user', line_start = 1 }, + { id = 'u2', kind = 'user' }, + }) + contexts.current().lazy_render_count = 1 + + local pending + local original_load = renderer.load_all_messages + local original_after = renderer.after_history_load + renderer.load_all_messages = function() + return true, true + end + renderer.after_history_load = function(fn) + pending = fn + end + local notify_stub = stub(vim, 'notify') + + vim.api.nvim_win_set_cursor(output_win, { 2, 0 }) + navigation.goto_next_user_message() + + renderer.load_all_messages = original_load + renderer.after_history_load = original_after + notify_stub:revert() + + assert.is_function(pending) + assert.stub(notify_stub).was_called(0) + + contexts.current().render_state:set_message(contexts.current().entries[2], 10, 10) + pending() + + assert.equals(11, vim.api.nvim_win_get_cursor(output_win)[1]) + end) end) it('keeps user-message jumps after a completed history request resolves asynchronously', function() diff --git a/tests/unit/protocol_v2_observation_runtime_spec.lua b/tests/unit/protocol_v2_observation_runtime_spec.lua index d19f3eb2..143447a0 100644 --- a/tests/unit/protocol_v2_observation_runtime_spec.lua +++ b/tests/unit/protocol_v2_observation_runtime_spec.lua @@ -425,6 +425,395 @@ describe('V2 protocol Observation runtime', function() stop() end) + it('gives a second caller the page already in flight instead of failing it', function() + local value = connection() + local calls = {} + local older = Promise.new() + install_operations(value, { + list_messages = function(_, _, cursor, limit) + calls[#calls + 1] = { cursor = cursor, limit = limit } + if cursor == nil then + return resolved({ data = { user('B', 'B', 4) }, cursor = { next = 'older-cursor' } }) + end + return older + end, + }) + local observed = value:observe({ id = 'ses-main' }) + local stop = observed:watch({ 'messages' }, function() end) + flush(function() + return observed:read().sync.messages.state == 'current' + end) + + local first = observed:load_older() + local second = observed:load_older() + assert.is_true(first == second, 'both callers wait on the same request') + assert.equals(2, #calls) + + older:resolve({ data = { user('A', 'A', 3) }, cursor = {} }) + first:wait() + second:wait() + assert.same({ 'A', 'B' }, observed:read().entry_order) + assert.equals(2, #calls) + stop() + end) + + it('fetches the cursor a rebuild produced only after the in-flight page lands', function() + local value = connection() + local calls = {} + local older = Promise.new() + local snapshot_cursor = 'c1' + local streams = install_operations(value, { + list_messages = function(_, _, cursor, limit) + calls[#calls + 1] = { cursor = cursor, limit = limit } + if cursor == nil then + return resolved({ data = { user('B', 'B', 4) }, cursor = { next = snapshot_cursor } }) + end + if cursor == 'c1' then + return older + end + return resolved({ data = {}, cursor = {} }) + end, + }) + local observed = value:observe({ id = 'ses-main' }) + local stop = observed:watch({ 'messages' }, function() end) + flush(function() + return observed:read().sync.messages.state == 'current' + end) + + local first = observed:load_older() + assert.equals(2, #calls) + + snapshot_cursor = 'c2' + emit(streams[1], event('ses-main', 'session.compaction.ended', {})) + flush(function() + return observed._v2_older_cursor == 'c2' + end) + assert.is_false(first:is_resolved(), 'the page in flight is still pending') + + local queued = observed:load_older() + local before = #calls + for index = 1, before do + assert.are_not.equals('c2', calls[index].cursor, 'the new cursor must wait for the page in flight') + end + + older:resolve({ data = { user('A', 'A', 3) }, cursor = { next = 'c1b' } }) + flush(function() + for index = before + 1, #calls do + if calls[index].cursor == 'c2' then + return true + end + end + return false + end) + queued:wait() + -- The rebuild replaced entries, order and cursor, so the page the caller + -- queued behind no longer describes this history. + assert.is_nil(observed:read().entries_by_id['A']) + assert.equals('c2', calls[#calls].cursor) + stop() + end) + + it('clears the in-flight page when the request fails so the next call starts over', function() + local value = connection() + local calls = {} + local older = Promise.new() + local pending = true + local streams = install_operations(value, { + list_messages = function(_, _, cursor, limit) + calls[#calls + 1] = { cursor = cursor, limit = limit } + if cursor == nil then + return resolved({ data = { user('B', 'B', 4) }, cursor = { next = 'c1' } }) + end + if cursor == 'c1' and pending then + pending = false + return older + end + return resolved({ data = { user('C', 'C', 2) }, cursor = { next = 'c1b' } }) + end, + }) + local observed = value:observe({ id = 'ses-main' }) + local stop = observed:watch({ 'messages' }, function() end) + flush(function() + return observed:read().sync.messages.state == 'current' + end) + + local first = observed:load_older() + local waited = observed:load_older() + assert.equals(2, #calls, 'the second caller shares the request in flight') + + older:reject('HTTP request cancelled') + flush(function() + return observed._v2_older_page == nil + end) + for _, promise in ipairs({ first, waited }) do + local ok, err = pcall(promise.wait, promise) + assert.is_false(ok, 'a cancelled page rejects its waiters') + assert.truthy(tostring(err):find('cancelled'), tostring(err)) + end + + local again = observed:load_older() + assert.equals(3, #calls, 'a failed page must not strand the slot') + assert.equals('c1', calls[3].cursor, 'the cursor did not move, so the page is asked for again') + again:wait() + assert.is_not_nil(observed:read().entries_by_id['C'], 'the retried page lands') + stop() + end) + + it('issues one page for twenty rapid load_older calls', function() + local value = connection() + local calls = {} + local older = Promise.new() + local streams = install_operations(value, { + list_messages = function(_, _, cursor, limit) + calls[#calls + 1] = { cursor = cursor, limit = limit } + if cursor == nil then + return resolved({ data = { user('B', 'B', 4) }, cursor = { next = 'c1' } }) + end + if cursor == 'c1' then + return older + end + return resolved({ data = {}, cursor = {} }) + end, + }) + local observed = value:observe({ id = 'ses-main' }) + local stop = observed:watch({ 'messages' }, function() end) + flush(function() + return observed:read().sync.messages.state == 'current' + end) + + local pages = {} + for _ = 1, 20 do + pages[#pages + 1] = observed:load_older() + end + assert.equals(2, #calls, 'twenty key repeats share the one page in flight') + + older:resolve({ data = { user('A', 'A', 3) }, cursor = { next = 'c1b' } }) + for _, page in ipairs(pages) do + page:wait() + end + flush(function() + return observed:read().entries_by_id['A'] ~= nil + end) + assert.equals(2, #calls, 'the repeats never started a second request') + assert.is_not_nil(observed:read().entries_by_id['A']) + stop() + end) + + it('applies an older page that arrived after live deltas and keeps the newer copy of shared entries', function() + local value = connection() + local calls = {} + local older = Promise.new() + local streams = install_operations(value, { + list_messages = function(_, _, cursor, limit) + calls[#calls + 1] = { cursor = cursor, limit = limit } + if cursor == nil then + return resolved({ data = { user('B', 'B', 4) }, cursor = { next = 'c1' } }) + end + if cursor == 'c1' then + return older + end + return resolved({ data = {}, cursor = {} }) + end, + }) + local observed = value:observe({ id = 'ses-main' }) + local stop = observed:watch({ 'messages' }, function() end) + flush(function() + return observed:read().sync.messages.state == 'current' + end) + + local page = observed:load_older() + assert.equals(2, #calls) + emit( + streams[1], + event('ses-main', 'session.step.started', { assistantMessageID = 'msg-tail', agent = 'build' }, 20) + ) + emit(streams[1], event('ses-main', 'session.text.started', { assistantMessageID = 'msg-tail', ordinal = 0 }, 21)) + emit( + streams[1], + event('ses-main', 'session.text.delta', { assistantMessageID = 'msg-tail', ordinal = 0, delta = 'live' }, 22) + ) + flush() + + local online = observed:read().entries_by_id['B'] + older:resolve({ data = { user('B', 'stale copy', 4), user('A', 'A', 3) }, cursor = { next = 'c1b' } }) + page:wait() + + assert.same({ 'A', 'B', 'msg-tail' }, observed:read().entry_order, 'the page prepends without repeating B') + assert.equals(online, observed:read().entries_by_id['B'], 'the entry received online keeps its identity') + assert.equals('B', observed:read().entries_by_id['B'].content[1].text, 'live content is not replaced by the page') + assert.equals('c1b', observed._v2_older_cursor) + assert.equals(2, #calls, 'the deltas did not cost another request') + stop() + end) + + it('drops an older page a fresh snapshot superseded and does not resurrect what it removed', function() + local value = connection() + local calls = {} + local older = Promise.new() + local streams = install_operations(value, { + list_messages = function(_, _, cursor, limit) + calls[#calls + 1] = { cursor = cursor, limit = limit } + if cursor == nil then + if #calls == 1 then + return resolved({ data = { user('B', 'B', 4) }, cursor = { next = 'c1' } }) + end + return resolved({ data = { user('A', 'A', 3) }, cursor = { next = 'c2' } }) + end + if cursor == 'c1' then + return older + end + return resolved({ data = {}, cursor = {} }) + end, + }) + local observed = value:observe({ id = 'ses-main' }) + local stop = observed:watch({ 'messages' }, function() end) + flush(function() + return observed:read().sync.messages.state == 'current' + end) + + local page = observed:load_older() + assert.equals(2, #calls) + emit(streams[1], event('ses-main', 'session.compaction.ended', {}, 20)) + flush(function() + return observed._loading.messages == nil and observed:read().sync.messages.state == 'current' and #calls >= 3 + end) + local refetched = #calls + assert.equals(3, refetched, 'one rebuild request, not a stale one followed by a second') + + older:resolve({ data = { user('B', 'B', 4), user('Z', 'Z', 1) }, cursor = {} }) + page:wait() + + assert.same({ 'A' }, observed:read().entry_order, 'the superseded page is dropped, not merged') + assert.equals('c2', observed._v2_older_cursor, 'the cursor stays with the snapshot that replaced the page') + assert.equals(refetched, #calls, 'dropping the page does not trigger another snapshot request') + stop() + end) + + it('lets a rebuild of an empty window replace text deltas appended while it flew', function() + local value = connection() + local calls = {} + local rebuild = Promise.new() + local function reply(text) + return { id = 'msg-reply', type = 'assistant', agent = 'build', content = { { type = 'text', text = text } } } + end + local streams = install_operations(value, { + list_messages = function(_, _, cursor, limit) + calls[#calls + 1] = { cursor = cursor, limit = limit } + if #calls == 1 then + return resolved({ data = {}, cursor = { next = 'c1' } }) + end + return rebuild + end, + }) + local observed = value:observe({ id = 'ses-main' }) + local stop = observed:watch({ 'messages' }, function() end) + flush(function() + return observed:read().sync.messages.state == 'current' + end) + + emit( + streams[1], + event('ses-main', 'session.step.started', { assistantMessageID = 'msg-reply', agent = 'build' }, 20) + ) + emit(streams[1], event('ses-main', 'session.text.started', { assistantMessageID = 'msg-reply', ordinal = 0 }, 21)) + emit( + streams[1], + event('ses-main', 'session.text.delta', { assistantMessageID = 'msg-reply', ordinal = 0, delta = 'b' }, 22) + ) + emit( + streams[1], + event('ses-main', 'session.text.delta', { assistantMessageID = 'msg-reply', ordinal = 0, delta = 'c' }, 23) + ) + flush() + assert.equals('bc', observed:read().entries_by_id['msg-reply'].content[1].text, 'the deltas append locally') + + emit(streams[1], event('ses-main', 'session.revert.committed', { messageID = 'msg-reply' }, 24)) + flush(function() + return #calls >= 2 + end) + rebuild:resolve({ data = { reply('b') }, cursor = { next = 'c1' } }) + flush(function() + return observed._loading.messages == nil and observed:read().sync.messages.state == 'current' + end) + + assert.equals( + 'b', + observed:read().entries_by_id['msg-reply'].content[1].text, + 'the snapshot is authoritative even though it lags the live text' + ) + + emit( + streams[1], + event('ses-main', 'session.text.ended', { assistantMessageID = 'msg-reply', ordinal = 0, text = 'bc' }, 25) + ) + flush(function() + return observed:read().entries_by_id['msg-reply'].content[1].text == 'bc' + end) + assert.equals('bc', observed:read().entries_by_id['msg-reply'].content[1].text, 'the finished text restores it') + stop() + end) + + it('stops load_complete_history when a round makes no progress', function() + local value = connection() + local calls = {} + install_operations(value, { + list_messages = function(_, _, cursor, limit) + calls[#calls + 1] = { cursor = cursor, limit = limit } + if cursor == nil then + return resolved({ data = { user('B', 'B', 4) }, cursor = { next = 'c1' } }) + end + return resolved({ data = { user('A', 'A', 3) }, cursor = { next = cursor } }) + end, + }) + local observed = value:observe({ id = 'ses-main' }) + local stop = observed:watch({ 'messages' }, function() end) + flush(function() + return observed:read().sync.messages.state == 'current' + end) + + observed:load_complete_history():wait() + assert.equals(2, #calls, 'the cursor came back unchanged, so the walk stops after one round') + assert.is_not_nil(observed:read().entries_by_id['A']) + stop() + end) + + it('discards a rebuild that a later event superseded', function() + local value = connection() + local pulls = 0 + local rebuild = Promise.new() + local streams = install_operations(value, { + list_messages = function() + pulls = pulls + 1 + if pulls == 1 then + return resolved({ data = { user('B', 'B', 4) }, cursor = { next = 'c1' } }) + end + if pulls == 2 then + return rebuild + end + return resolved({ data = { user('C', 'C', 5) }, cursor = { next = 'c3' } }) + end, + }) + local observed = value:observe({ id = 'ses-main' }) + local stop = observed:watch({ 'messages' }, function() end) + flush(function() + return observed:read().sync.messages.state == 'current' + end) + + emit(streams[1], event('ses-main', 'session.compaction.ended', {}, 20)) + flush(function() + return pulls == 2 + end) + emit(streams[1], event('ses-main', 'session.compaction.ended', {}, 21)) + rebuild:resolve({ data = { user('A', 'A', 3) }, cursor = { next = 'c2' } }) + flush(function() + return observed._loading.messages == nil and observed:read().entries_by_id['C'] ~= nil + end) + + assert.same({ 'C' }, observed:read().entry_order, 'the superseded rebuild never lands') + assert.equals('c3', observed._v2_older_cursor) + stop() + end) + it('keeps terminal inbox and form facts when a stale pending snapshot arrives', function() local value = connection() local inbox_page, form_page, permission_page = Promise.new(), Promise.new(), Promise.new() diff --git a/tests/unit/protocol_v2_observation_spec.lua b/tests/unit/protocol_v2_observation_spec.lua index da9a352e..d79edd8e 100644 --- a/tests/unit/protocol_v2_observation_spec.lua +++ b/tests/unit/protocol_v2_observation_spec.lua @@ -115,7 +115,7 @@ describe('V2 protocol Observation interpretation', function() observed._start_resource = original_refresh end) - it('refreshes messages when a background completion notification arrives', function() + it('projects a background completion from the event without pulling a snapshot', function() local observed = observation('ses-bg') local original = observed._start_resource local refreshed @@ -123,16 +123,70 @@ describe('V2 protocol Observation interpretation', function() refreshed = resource end - assert.is_true(observation_module.ingest_event( - observed, - event('ses-bg', 'session.synthetic', { - text = '\nexit 0\n', - metadata = { source = 'shell', shellID = 'sh_1', state = 'completed', exit = 0 }, - }) - )) - assert.equals('messages', refreshed) + local completion = event('ses-bg', 'session.synthetic', { + text = '\nexit 0\n', + metadata = { source = 'shell', shellID = 'sh_1', state = 'completed', exit = 0 }, + }) + assert.is_true(observation_module.ingest_event(observed, completion)) + + local state = observed:read() + assert.equals('current', state.sync.messages.state, 'projecting the completion needs no snapshot') + assert.equals(1, #state.entry_order) + local entry = state.entries_by_id[state.entry_order[1]] + assert.equals('sh_1', entry.background_result.id) + assert.equals('exit 0', entry.background_result.text) + + observed:_event_changed('messages') + assert.is_nil(refreshed, 'the dispatch has nothing to refetch') + + assert.is_true(observation_module.ingest_event(observed, completion)) + assert.equals(1, #observed:read().entry_order, 'a redelivered completion replaces its entry') + + observed._start_resource = original + end) + + it('drops the tail a committed revert deleted without refetching', function() + local observed = observation('ses-revert') + observation_module.ingest_snapshot(observed, { assistant('msg-c'), assistant('msg-b'), assistant('msg-a') }) + local original = observed._start_resource + local refreshed + observed._start_resource = function(_, resource) + refreshed = resource + end + + assert.is_true( + observation_module.ingest_event(observed, event('ses-revert', 'session.revert.committed', { to = 'msg-b' })) + ) + + local state = observed:read() + assert.same({ 'msg-a' }, state.entry_order, 'the boundary message is deleted too') + assert.is_nil(state.entries_by_id['msg-c']) + assert.equals('current', state.sync.messages.state) + assert.is_nil(refreshed, 'a known tail needs no snapshot') + + observed._start_resource = original + end) + + it('asks for the snapshot when the revert boundary was never loaded', function() + local observed = observation('ses-revert') + observation_module.ingest_snapshot(observed, { assistant('msg-a') }) + local original = observed._start_resource + local refreshed + observed._start_resource = function(_, resource) + refreshed = resource + end + + assert.is_true( + observation_module.ingest_event( + observed, + event('ses-revert', 'session.revert.committed', { to = 'msg-before-the-window' }) + ) + ) assert.equals('stale', observed:read().sync.messages.state) + observed:_event_changed('messages') + assert.equals('messages', refreshed, 'only a snapshot can say what the deletion took') + observed._start_resource = original end) diff --git a/tests/unit/renderer_lazy_spec.lua b/tests/unit/renderer_lazy_spec.lua index db679092..42a315e0 100644 --- a/tests/unit/renderer_lazy_spec.lua +++ b/tests/unit/renderer_lazy_spec.lua @@ -363,6 +363,41 @@ describe('lazy render', function() end end) + it('replaces the transcript with what a rebuilt snapshot carries', function() + local session = { id = 'ses_test', location = { directory = helpers.MOCK_CWD } } + local entries = make_session_data(50) -- 100 messages, oldest first + + ---@param list table[] + local function observed(list) + local snapshot = { entry_order = {}, entries_by_id = {}, session = session, sync = {} } + for _, entry in ipairs(list) do + snapshot.entry_order[#snapshot.entry_order + 1] = entry.id + snapshot.entries_by_id[entry.id] = entry + end + return { + read = function() + return snapshot + end, + } + end + + contexts.current().observation = observed(entries) + contexts.current().entries = entries + contexts.current().lazy_render_count = #entries + renderer.render_from_cache() + assert.are.equal(#entries, count_rendered_messages()) + + -- The snapshot carries the newest page only. + contexts.current().observation = observed(vim.list_slice(entries, 51, 100)) + renderer.render_from_cache() + + assert.are.equal(50, count_rendered_messages()) + local render_state = contexts.current().render_state + assert.is_nil(render_state:get_message('msg_u1')) + assert.is_nil(render_state:get_message('msg_a25')) + assert.is_not_nil(render_state:get_message('msg_u26')) + end) + it('load_more_messages returns false when all messages already rendered', function() -- No lazy_render_count means all messages rendered — no unrendered content local session_data = make_session_data(50) -- 100 messages total @@ -741,6 +776,109 @@ describe('older history bridge', function() assert.are.equal(1, vim.api.nvim_win_get_cursor(state.windows.output_win)[1]) end) + it('drops a navigation callback when a top jump supersedes the pull it was registered for', function() + config.ui.output.max_messages = 10 + local newer = make_session_data(20) + local entries = newer + local page_request = Promise.new() + local top_request = Promise.new() + local requests = { page_request, top_request } + local observation = { + read = function() + local by_id, order = {}, {} + for _, entry in ipairs(entries) do + by_id[entry.id] = entry + order[#order + 1] = entry.id + end + return { session = { id = 'ses_test' }, entries_by_id = by_id, entry_order = order } + end, + load_complete_history = function() + return table.remove(requests, 1) or Promise.new() + end, + } + local ctx = contexts.current() + ctx.observation = observation + ctx.lazy_render_count = 10 + renderer._render_full_session_data(newer) + while ctx.lazy_render_count < #newer do + assert.is_true(renderer.load_more_messages()) + end + local first = renderer.get_next_rendered_message(0) + vim.api.nvim_win_set_cursor(state.windows.output_win, { first.line_start + 1, 0 }) + + local reached = 0 + assert.is_true(renderer.load_all_messages(nil, { scroll_to_top = false })) + renderer.after_history_load(function() + reached = reached + 1 + end) + + require('opencode.ui.navigation').goto_first_message() + page_request:resolve(nil) + vim.wait(100, function() + return false + end) + assert.are.equal(0, reached, 'a superseded pull must not fire the callback') + + top_request:resolve(nil) + assert.is_true( + vim.wait(1000, function() + return vim.api.nvim_win_get_cursor(state.windows.output_win)[1] == 1 + end), + 'the top jump should still land at the first message' + ) + assert.are.equal(0, reached, 'the top jump must not inherit the callback') + end) + + it('keeps a callback registered while an older pull is still in flight', function() + config.ui.output.max_messages = 10 + local newer = make_session_data(20) + local entries = newer + local failing = Promise.new() + local sibling = Promise.new() + local requests = { failing, sibling } + local observation = { + read = function() + local by_id, order = {}, {} + for _, entry in ipairs(entries) do + by_id[entry.id] = entry + order[#order + 1] = entry.id + end + return { session = { id = 'ses_test' }, entries_by_id = by_id, entry_order = order } + end, + load_complete_history = function() + return table.remove(requests, 1) or Promise.new() + end, + } + local ctx = contexts.current() + ctx.observation = observation + ctx.lazy_render_count = 10 + renderer._render_full_session_data(newer) + while ctx.lazy_render_count < #newer do + assert.is_true(renderer.load_more_messages()) + end + + assert.is_true(renderer.load_all_messages(nil, { scroll_to_top = false })) + local reached = 0 + renderer.after_history_load(function() + reached = reached + 1 + end) + assert.is_true(renderer.load_all_messages(nil, { scroll_to_top = false })) + + failing:reject('boom') + vim.wait(100, function() + return false + end) + assert.are.equal(0, reached, 'a failed pull must not fire the callback') + + sibling:resolve(nil) + assert.is_true( + vim.wait(1000, function() + return reached == 1 + end), + 'the sibling pull should still fire the callback' + ) + end) + it('load_more_messages pulls an older page when the cached window is exhausted', function() local observation, older, newer, pages_left = observation_with_older_page() contexts.current().observation = observation