Skip to content

Commit b656af3

Browse files
sudo-teesudo-tee
committed
fix(ui/question_window): avoid showing or restoring already-resolved questions
1 parent 6efcb2e commit b656af3

2 files changed

Lines changed: 220 additions & 4 deletions

File tree

‎lua/opencode/ui/question_window.lua‎

Lines changed: 104 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
local state = require('opencode.state')
22
local icons = require('opencode.ui.icons')
33
local Dialog = require('opencode.ui.dialog')
4+
local Promise = require('opencode.promise')
45

56
local config = require('opencode.config')
67

@@ -51,6 +52,92 @@ function M.belongs_to_active_session(question_request)
5152
return false
5253
end
5354

55+
---@param question_request OpencodeQuestionRequest|nil
56+
---@return boolean
57+
local function has_tool_identifiers(question_request)
58+
local tool = question_request and question_request.tool
59+
return tool ~= nil and ((tool.callID and tool.callID ~= '') or (tool.messageID and tool.messageID ~= ''))
60+
end
61+
62+
---@param part OpencodeMessagePart|nil
63+
---@param question_request OpencodeQuestionRequest|nil
64+
---@return boolean
65+
local function question_part_matches_request(part, question_request)
66+
if not part or part.tool ~= 'question' or not question_request then
67+
return false
68+
end
69+
70+
local tool = question_request.tool
71+
if not tool then
72+
return false
73+
end
74+
75+
if tool.callID and tool.callID ~= '' and part.callID ~= tool.callID then
76+
return false
77+
end
78+
79+
if tool.messageID and tool.messageID ~= '' and part.messageID ~= tool.messageID then
80+
return false
81+
end
82+
83+
return true
84+
end
85+
86+
---@param parts OpencodeMessagePart[]|nil
87+
---@param question_request OpencodeQuestionRequest|nil
88+
---@return OpencodeMessagePart|nil
89+
local function find_matching_question_part(parts, question_request)
90+
for _, part in ipairs(parts or {}) do
91+
if question_part_matches_request(part, question_request) then
92+
return part
93+
end
94+
end
95+
end
96+
97+
---@param question_request OpencodeQuestionRequest|nil
98+
---@return OpencodeMessagePart|nil
99+
local function get_question_part(question_request)
100+
if not has_tool_identifiers(question_request) then
101+
return nil
102+
end
103+
104+
local tool = question_request.tool
105+
local tool_message_id = tool and tool.messageID
106+
107+
if tool_message_id and state.messages then
108+
for _, message in ipairs(state.messages) do
109+
if message.info and message.info.id == tool_message_id then
110+
local part = find_matching_question_part(message.parts, question_request)
111+
if part then
112+
return part
113+
end
114+
end
115+
end
116+
end
117+
118+
if question_request and question_request.sessionID and question_request.sessionID ~= '' then
119+
local render_state = require('opencode.ui.renderer.ctx').render_state
120+
return find_matching_question_part(render_state:get_child_session_parts(question_request.sessionID), question_request)
121+
end
122+
end
123+
124+
---@param question_request OpencodeQuestionRequest|nil
125+
---@return boolean
126+
local function is_resolved_question_request(question_request)
127+
local part = get_question_part(question_request)
128+
if not part or not part.state then
129+
return false
130+
end
131+
132+
local metadata = part.state.metadata
133+
if metadata and metadata.answers and #metadata.answers > 0 then
134+
return true
135+
end
136+
137+
local status = part.state.status
138+
return status ~= nil and status ~= '' and status ~= 'pending' and status ~= 'running'
139+
end
140+
54141
---Request the renderer to show the current question display.
55142
local function render_question()
56143
require('opencode.ui.renderer.events').render_question_display()
@@ -67,6 +154,10 @@ function M.show_question(question_request)
67154
return
68155
end
69156

157+
if is_resolved_question_request(question_request) then
158+
return
159+
end
160+
70161
M._current_question = question_request
71162
M._current_question_index = 1
72163
M._collected_answers = {}
@@ -82,21 +173,30 @@ end
82173
---@param session_id string|nil
83174
function M.restore_pending_question(session_id)
84175
if not state.api_client or not session_id or session_id == '' then
85-
return
176+
return Promise.new():resolve(nil)
86177
end
87178

88179
if M.has_question() and M.belongs_to_active_session(M._current_question) then
89-
return
180+
if not is_resolved_question_request(M._current_question) then
181+
return Promise.new():resolve(nil)
182+
end
183+
184+
M.clear_question()
90185
end
91186

92-
state.api_client:list_questions()
187+
return state.api_client:list_questions()
93188
:and_then(function(requests)
94189
if not requests or type(requests) ~= 'table' then
95190
return
96191
end
97192

98193
for _, request in ipairs(requests) do
99-
if request and request.questions and #request.questions > 0 and M.belongs_to_active_session(request) then
194+
if request
195+
and request.questions
196+
and #request.questions > 0
197+
and M.belongs_to_active_session(request)
198+
and not is_resolved_question_request(request)
199+
then
100200
if M.matches_active_question(request) then
101201
return
102202
end

‎tests/unit/question_window_spec.lua‎

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
local question_window = require('opencode.ui.question_window')
22
local Output = require('opencode.ui.output')
3+
local Promise = require('opencode.promise')
4+
local state = require('opencode.state')
5+
local stub = require('luassert.stub')
36

47
describe('question_window', function()
58
after_each(function()
@@ -8,6 +11,9 @@ describe('question_window', function()
811
question_window._collected_answers = {}
912
question_window._answering = false
1013
question_window._dialog = nil
14+
state.renderer.set_messages({})
15+
state.session.set_active(nil)
16+
state.jobs.set_api_client(nil)
1117
end)
1218

1319
it('adds the Other option when missing', function()
@@ -35,4 +41,114 @@ describe('question_window', function()
3541
assert.are.equal('On save', captured_opts.options[1].label)
3642
assert.are.equal('Other', captured_opts.options[2].label)
3743
end)
44+
45+
it('does not show a question that is already completed', function()
46+
state.renderer.set_messages({
47+
{
48+
info = {
49+
id = 'msg_question',
50+
sessionID = 'sess1',
51+
},
52+
parts = {
53+
{
54+
id = 'part_question',
55+
type = 'tool',
56+
tool = 'question',
57+
callID = 'call_question',
58+
messageID = 'msg_question',
59+
sessionID = 'sess1',
60+
state = {
61+
status = 'completed',
62+
metadata = {
63+
answers = {
64+
{ 'Red' },
65+
},
66+
},
67+
},
68+
},
69+
},
70+
},
71+
})
72+
73+
question_window.show_question({
74+
id = 'question_1',
75+
sessionID = 'sess1',
76+
tool = {
77+
messageID = 'msg_question',
78+
callID = 'call_question',
79+
},
80+
questions = {
81+
{
82+
question = 'Pick one',
83+
options = {
84+
{ label = 'One', description = 'first' },
85+
},
86+
},
87+
},
88+
})
89+
90+
assert.is_nil(question_window._current_question)
91+
end)
92+
93+
it('clears a stale completed question instead of restoring it again', function()
94+
local request = {
95+
id = 'question_1',
96+
sessionID = 'sess1',
97+
tool = {
98+
messageID = 'msg_question',
99+
callID = 'call_question',
100+
},
101+
questions = {
102+
{
103+
question = 'Pick one',
104+
options = {
105+
{ label = 'One', description = 'first' },
106+
},
107+
},
108+
},
109+
}
110+
111+
state.session.set_active({ id = 'sess1' })
112+
state.renderer.set_messages({
113+
{
114+
info = {
115+
id = 'msg_question',
116+
sessionID = 'sess1',
117+
},
118+
parts = {
119+
{
120+
id = 'part_question',
121+
type = 'tool',
122+
tool = 'question',
123+
callID = 'call_question',
124+
messageID = 'msg_question',
125+
sessionID = 'sess1',
126+
state = {
127+
status = 'completed',
128+
metadata = {
129+
answers = {
130+
{ 'Red' },
131+
},
132+
},
133+
},
134+
},
135+
},
136+
},
137+
})
138+
question_window._current_question = request
139+
state.jobs.set_api_client({
140+
list_questions = function()
141+
return Promise.new():resolve({ request })
142+
end,
143+
})
144+
145+
local show_stub = stub(question_window, 'show_question')
146+
147+
question_window.restore_pending_question('sess1'):wait()
148+
149+
assert.is_nil(question_window._current_question)
150+
assert.stub(show_stub).was_not_called()
151+
152+
show_stub:revert()
153+
end)
38154
end)

0 commit comments

Comments
 (0)