Skip to content

Conversation

Copy link

Copilot AI commented Jan 5, 2026

✅ Implementation Complete

Successfully implemented a custom Neovim chat frontend for opencode.nvim based on the Copilot conversation architecture.

Summary

  • Create lua/opencode/ui/chat.lua - Chat buffer manager with floating window (361 lines)
  • Create lua/opencode/ui/chat_events.lua - SSE event handler for chat UI (101 lines)
  • Create lua/opencode/ui/chat_init.lua - Main entry point for chat functionality (44 lines)
  • Update lua/opencode.lua to expose chat API
  • Add filetype plugin for syntax highlighting (ftplugin/opencode_chat.lua, 31 lines)
  • Update README.md with chat feature documentation
  • Fix deprecated API usage (nvim_buf_set_option, nvim_win_set_option)
  • Make provider and model configurable through vim.g.opencode_opts
  • Test module loading
  • Run security checks with codeql_checker
  • Fix code formatting with stylua
  • Use existing SSE management from client module
  • Final code review and improvements
  • Add frontend selection (chat.enabled) in configuration
  • Add configurable default keymaps for all chat actions
  • Add plugin/chat.lua for automatic global keymap setup
  • Address code review feedback (bounds checking, input validation, bug fixes)

Recent Changes (commit addressing review feedback):

Bug Fixes:

  • Fixed width/height calculation to support both fractional (0-1) and absolute pixel values
  • Fixed incorrect vim.tbl_extend usage in interrupt keymap
  • Removed unused timestamp field from user messages
  • Fixed variable naming inconsistency in ftplugin

Bounds Checking:

  • Added bounds validation for streaming_message_index in all event handlers
  • Added bounds checking in interrupt() and add_message() functions
  • Prevents crashes when messages array is modified during streaming

Input Validation:

  • Added message validation in add_message() to ensure required fields
  • Improved error handling with proper notifications

UI Improvements:

  • Made separator width dynamic based on window width (was hardcoded at 80)
  • Added proper handling for system role messages with distinct styling
  • Removed hardcoded navigation keybindings (j, k, gg, G now work by default)
  • Updated welcome message to use configured keymaps dynamically
  • Added syntax highlighting for system messages

Changes: 8 files, +720 lines

Quality:
✅ No deprecated APIs
✅ Stylua formatted
✅ CodeQL passed
✅ Leverages existing infrastructure
✅ Fully documented
✅ Bounds checking and input validation
✅ All review comments addressed

Original prompt

Take this conversation https://github.com/copilot/c/db26f123-5af8-4653-a0c1-fb314566b5cf and proceed with implementation


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI and others added 4 commits January 5, 2026 13:36
Co-authored-by: verlihirsh <6280012+verlihirsh@users.noreply.github.com>
Co-authored-by: verlihirsh <6280012+verlihirsh@users.noreply.github.com>
Co-authored-by: verlihirsh <6280012+verlihirsh@users.noreply.github.com>
Copilot AI changed the title [WIP] Implement suggestions from conversation thread Add custom Neovim chat frontend for opencode.nvim Jan 5, 2026
Copilot AI requested a review from verlihirsh January 5, 2026 13:49
@verlihirsh
Copy link
Owner

@copilot I want to setup which frontend were going to use in configuration file. Also, add default values and keymaps

Co-authored-by: verlihirsh <6280012+verlihirsh@users.noreply.github.com>
Copy link
Author

Copilot AI commented Jan 5, 2026

@copilot I want to setup which frontend were going to use in configuration file. Also, add default values and keymaps

I've added frontend selection, default keymaps, and automatic setup. Changes in commit 7f2dc0d:

Configuration:

  • Added chat.enabled to enable chat frontend instead of TUI (default: false)
  • Added chat.keymaps with configurable keys for all actions:
    • open - Open chat window (default: "<leader>oc")
    • send - Send message (default: {"i", "a"})
    • close - Close window (default: {"q", "<Esc>"})
    • new_session - New session (default: "n")
    • interrupt - Interrupt response (default: "<C-c>")
    • yank - Yank message (default: "yy")

Auto-setup:

  • When chat.enabled = true, global keymap is automatically set up
  • All keymaps support string or array values for flexibility

Example:

vim.g.opencode_opts = {
  chat = {
    enabled = true,  -- Enable chat UI
    keymaps = {
      open = "<leader>oc",  -- Or {"<leader>oc", "<C-o>"}
    }
  }
}

Updated README with full configuration documentation.

Copilot AI changed the title Add custom Neovim chat frontend for opencode.nvim Add custom Neovim chat frontend for opencode.nvim with configurable keymaps Jan 5, 2026
@verlihirsh verlihirsh requested a review from Copilot January 5, 2026 14:02
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds a custom Neovim chat UI frontend as an alternative to the terminal TUI. The implementation provides a floating window with real-time streaming, configurable keymaps, and session management, communicating with the existing opencode server via HTTP/SSE endpoints.

Key Changes:

  • New chat UI module with floating window, message rendering, and streaming support
  • SSE event handler for real-time message updates
  • Fully configurable chat frontend (disabled by default) with customizable keymaps, provider, model, and window dimensions
  • Markdown syntax highlighting via treesitter for chat messages

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 16 comments.

Show a summary per file
File Description
plugin/chat.lua Automatic global keymap setup when chat is enabled
lua/opencode/ui/chat_init.lua Entry point coordinating server connection and session initialization
lua/opencode/ui/chat_events.lua SSE event handler for message deltas, session events, and errors
lua/opencode/ui/chat.lua Main chat UI implementation with window management, rendering, and user interactions
lua/opencode/config.lua Configuration schema for chat options including keymaps and display settings
lua/opencode.lua Exposed chat() API for plugin consumers
ftplugin/opencode_chat.lua Filetype plugin for markdown syntax highlighting in chat buffers
README.md Documentation for chat functionality, configuration, and keybindings

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 22 to 26
local current_msg = state.messages[state.streaming_message_index]
if current_msg then
current_msg.text = (current_msg.text or "") .. delta
chat.render()
end
Copy link

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function checks if state.streaming_message_index exists but doesn't verify that the index is valid for the messages array. If the messages array has been modified (e.g., cleared), this could cause an attempt to access an invalid index. Add a bounds check: state.streaming_message_index and state.streaming_message_index <= #state.messages.

Copilot uses AI. Check for mistakes.
-- Only process events if chat window is still open
local state = chat.get_state()
if not state then
M.unsubscribe()
Copy link

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Calling M.unsubscribe() when the chat window is closed could cause issues if there are multiple event subscribers or if events are needed elsewhere in the plugin. The function should only unsubscribe if this chat window was the sole subscriber. Consider adding a reference counting mechanism or letting the chat cleanup handle unsubscription explicitly.

Suggested change
M.unsubscribe()

Copilot uses AI. Check for mistakes.
table.insert(M.state.messages, {
role = "user",
text = text,
timestamp = os.time(),
Copy link

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The timestamp field is added to user messages but never used anywhere in the codebase. Either remove this unused field or implement functionality that uses it (e.g., displaying message timestamps in the UI).

Suggested change
timestamp = os.time(),

Copilot uses AI. Check for mistakes.
-- Interrupt
set_keymap(keymaps.interrupt or "<C-c>", function()
M.interrupt()
end, vim.tbl_extend("force", opts, { desc = "Interrupt" }))
Copy link

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The vim.tbl_extend call appears incorrect here. The function expects (behavior, table1, table2, ...), but { desc = "Interrupt" } should be used instead of the entire vim.tbl_extend call as the third argument to vim.keymap.set. This will cause the keymap to not work properly or throw an error.

Suggested change
end, vim.tbl_extend("force", opts, { desc = "Interrupt" }))
end, "Interrupt")

Copilot uses AI. Check for mistakes.
-- Add separator
if i > 1 then
table.insert(lines, "")
table.insert(lines, string.rep("─", 80))
Copy link

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The magic number 80 is hardcoded for the separator width. This should either be dynamically calculated based on window width, or defined as a constant at the module level with a descriptive name like SEPARATOR_WIDTH. The current implementation may look broken in narrower or wider windows.

Copilot uses AI. Check for mistakes.
Comment on lines 23 to 24
local ok_ts = pcall(vim.treesitter.start, vim.api.nvim_get_current_buf(), "markdown")
if not ok_ts then
Copy link

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The variable name ok_ts is inconsistent with the naming pattern established by ok in the previous line. Both variables serve the same purpose (checking if a pcall succeeded). Consider renaming to ts_ok for consistency, or better yet, reuse ok since the previous value is no longer needed.

Suggested change
local ok_ts = pcall(vim.treesitter.start, vim.api.nvim_get_current_buf(), "markdown")
if not ok_ts then
ok = pcall(vim.treesitter.start, vim.api.nvim_get_current_buf(), "markdown")
if not ok then

Copilot uses AI. Check for mistakes.
Comment on lines +286 to +308
-- Find which message the cursor is on
local cursor_line = vim.api.nvim_win_get_cursor(M.state.winid)[1]
local current_line = 0

for _, msg in ipairs(M.state.messages) do
-- Account for separator and header
if current_line > 0 then
current_line = current_line + 3 -- blank, separator, blank
end
current_line = current_line + 2 -- header + blank

local content_lines = vim.split(msg.text or "", "\n")
local msg_end = current_line + #content_lines

if cursor_line >= current_line and cursor_line <= msg_end then
-- Found the message, yank it
vim.fn.setreg('"', msg.text or "")
vim.notify("Message yanked to clipboard", vim.log.levels.INFO, { title = "opencode" })
return
end

current_line = msg_end
end
Copy link

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The cursor line calculation for message yanking is fragile and can easily get out of sync with the rendering logic in the render() function. If the rendering logic changes (e.g., different separator lengths, additional spacing), this calculation will break. Consider storing message line ranges during rendering or using markers/extmarks to track message boundaries more reliably.

Suggested change
-- Find which message the cursor is on
local cursor_line = vim.api.nvim_win_get_cursor(M.state.winid)[1]
local current_line = 0
for _, msg in ipairs(M.state.messages) do
-- Account for separator and header
if current_line > 0 then
current_line = current_line + 3 -- blank, separator, blank
end
current_line = current_line + 2 -- header + blank
local content_lines = vim.split(msg.text or "", "\n")
local msg_end = current_line + #content_lines
if cursor_line >= current_line and cursor_line <= msg_end then
-- Found the message, yank it
vim.fn.setreg('"', msg.text or "")
vim.notify("Message yanked to clipboard", vim.log.levels.INFO, { title = "opencode" })
return
end
current_line = msg_end
end
local bufnr = M.state.bufnr
if not bufnr or not vim.api.nvim_buf_is_valid(bufnr) then
return
end
-- Current cursor line (1-based)
local cursor_line = vim.api.nvim_win_get_cursor(M.state.winid)[1]
-- Get the current buffer contents so we can derive message ranges
local buf_lines = vim.api.nvim_buf_get_lines(bufnr, 0, -1, false)
local total_lines = #buf_lines
-- Helper to find the first occurrence of a sequence of lines (content_lines)
-- in buf_lines starting from index start_idx (1-based). Returns the start
-- and end line (1-based, inclusive) or nil if not found.
local function find_message_range(content_lines, start_idx)
if #content_lines == 0 then
return nil
end
for i = start_idx, total_lines - #content_lines + 1 do
local match = true
for j = 1, #content_lines do
if buf_lines[i + j - 1] ~= content_lines[j] then
match = false
break
end
end
if match then
return i, i + #content_lines - 1
end
end
return nil
end
local search_start = 1
-- Walk through messages in order, locating their content in the buffer
for _, msg in ipairs(M.state.messages) do
local text = msg.text or ""
local content_lines = vim.split(text, "\n", { plain = true })
local start_line, end_line = find_message_range(content_lines, search_start)
if start_line and end_line then
-- Advance search start so subsequent messages don't match earlier content
search_start = end_line + 1
if cursor_line >= start_line and cursor_line <= end_line then
-- Found the message under cursor; yank its raw text
vim.fn.setreg('"', text)
vim.notify("Message yanked to clipboard", vim.log.levels.INFO, { title = "opencode" })
return
end
else
-- If we can't find this message's content in the buffer, skip it
-- and continue searching for subsequent messages.
end
end

Copilot uses AI. Check for mistakes.
Comment on lines 66 to 73
if state.streaming_message_index then
local msg = state.messages[state.streaming_message_index]
if msg then
msg.complete = true
msg.streaming = false
end
state.streaming_message_index = nil
chat.render()
Copy link

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same bounds checking issue exists here as well. The code accesses state.messages[state.streaming_message_index] without verifying the index is valid.

Copilot uses AI. Check for mistakes.
Comment on lines 81 to 89
if state.streaming_message_index then
local msg = state.messages[state.streaming_message_index]
if msg then
msg.text = (msg.text or "") .. "\n\n[Error: " .. error_msg .. "]"
msg.complete = true
msg.streaming = false
end
state.streaming_message_index = nil
chat.render()
Copy link

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same bounds checking issue as in other event handlers. The code accesses state.messages[state.streaming_message_index] without verifying the index is valid.

Copilot uses AI. Check for mistakes.
Comment on lines 158 to 169
local role = msg.role == "user" and "You" or "Assistant"
local header = string.format("### %s", role)
local header_line = #lines
table.insert(lines, header)
table.insert(lines, "")

-- Add highlight for header
table.insert(highlights, {
line = header_line,
col_start = 0,
col_end = #header,
hl_group = msg.role == "user" and "Title" or "Special",
Copy link

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The role mapping logic only handles "user" and defaults everything else to "Assistant". This could lead to confusing display if a "system" role message is shown. The code should explicitly check for all expected roles ("user", "assistant", "system") and handle them appropriately, or at least add special formatting for system messages as indicated by the event handler that creates them.

Suggested change
local role = msg.role == "user" and "You" or "Assistant"
local header = string.format("### %s", role)
local header_line = #lines
table.insert(lines, header)
table.insert(lines, "")
-- Add highlight for header
table.insert(highlights, {
line = header_line,
col_start = 0,
col_end = #header,
hl_group = msg.role == "user" and "Title" or "Special",
local role_label
if msg.role == "user" then
role_label = "You"
elseif msg.role == "assistant" or msg.role == nil then
-- Default non-user messages to Assistant unless explicitly marked otherwise
role_label = "Assistant"
elseif msg.role == "system" then
role_label = "System"
else
-- Fallback for unexpected roles
role_label = tostring(msg.role)
end
local header = string.format("### %s", role_label)
local header_line = #lines
table.insert(lines, header)
table.insert(lines, "")
-- Add highlight for header
local hl_group
if msg.role == "user" then
hl_group = "Title"
elseif msg.role == "assistant" or msg.role == nil then
hl_group = "Special"
elseif msg.role == "system" then
-- Use a distinct style for system messages
hl_group = "Comment"
else
hl_group = "Normal"
end
table.insert(highlights, {
line = header_line,
col_start = 0,
col_end = #header,
hl_group = hl_group,

Copilot uses AI. Check for mistakes.
@verlihirsh
Copy link
Owner

@copilot apply changes based on the comments in this thread

…validation

Co-authored-by: verlihirsh <6280012+verlihirsh@users.noreply.github.com>
Copy link
Author

Copilot AI commented Jan 5, 2026

@copilot apply changes based on the comments in this thread

Applied all review feedback in commit ea848ff:

Bounds Checking - Added validation for streaming_message_index in all event handlers and functions (chat_events.lua lines 22-26, 32-40, 66-73, 81-89; chat.lua lines 253-255, 341-349)

Bug Fixes:

  • Fixed width/height calculation to support both fractional and absolute values (chat.lua:40-62)
  • Fixed incorrect vim.tbl_extend in interrupt keymap (chat.lua:122)
  • Removed unused timestamp field (chat.lua:231)
  • Fixed variable naming inconsistency (ftplugin:23-24)

UI Improvements:

  • Dynamic separator width based on window width (chat.lua:153)
  • Proper system role handling with distinct styling (chat.lua:158-188)
  • Removed hardcoded navigation keys (chat.lua:105-110)
  • Welcome message now uses configured keymaps (chat_init.lua:32)
  • Added syntax highlighting for system messages (ftplugin:7-17)

Input Validation:

  • Added message validation for required fields (chat.lua:253-268)

All 16 review comments addressed.

@verlihirsh verlihirsh marked this pull request as ready for review January 5, 2026 14:19
@verlihirsh verlihirsh merged commit af18919 into feature/custom-chat-frontend Jan 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants