Skip to content

Commit 5e47626

Browse files
committed
fix(commands): address copilot review findings
- include allowed subcommands in agent/session/diff invalid argument errors - clamp help table column width in narrow windows to avoid format failures - fail run_tests.sh on non-zero nvim/plenary exits in addition to output parsing - add handlers tests for actionable subcommand errors and narrow-window help rendering
1 parent 33bb81d commit 5e47626

6 files changed

Lines changed: 93 additions & 9 deletions

File tree

lua/opencode/commands/handlers/agent.lua

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ M.command_defs = {
8484
execute = function(args)
8585
local action = agent_subcommand_calls[args[1]]
8686
if not action then
87-
invalid_arguments('Invalid agent subcommand')
87+
invalid_arguments('Invalid agent subcommand. Use: ' .. table.concat(agent_subcommands, ', '))
8888
end
8989
return action()
9090
end,

lua/opencode/commands/handlers/diff.lua

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ M.command_defs = {
155155
local subcommand = args[1] or 'open'
156156
local action = diff_subcommand_actions[subcommand]
157157
if not action then
158-
invalid_arguments('Invalid diff subcommand')
158+
invalid_arguments('Invalid diff subcommand. Use: ' .. table.concat(diff_subcommands, ', '))
159159
end
160160
return action()
161161
end,

lua/opencode/commands/handlers/session.lua

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -434,7 +434,7 @@ M.command_defs = {
434434
local subcommand = args[1]
435435
local action = session_subcommand_actions[subcommand]
436436
if not action then
437-
invalid_arguments('Invalid session subcommand')
437+
invalid_arguments('Invalid session subcommand. Use: ' .. table.concat(session_subcommands, ', '))
438438
end
439439
return action(args)
440440
end,

lua/opencode/commands/handlers/surface.lua

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ function M.actions.help()
6868
return
6969
end
7070

71-
local max_desc_length = math.min(90, nvim.nvim_win_get_width(state.windows.output_win) - 35)
71+
local max_desc_length = math.max(10, math.min(90, nvim.nvim_win_get_width(state.windows.output_win) - 35))
7272

7373
local command_defs = get_command_definitions()
7474
local sorted_commands = vim.tbl_keys(command_defs)

run_tests.sh

Lines changed: 30 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -86,12 +86,17 @@ has_failures() {
8686
minimal_output=""
8787
unit_output=""
8888
replay_output=""
89+
minimal_status=0
90+
unit_status=0
91+
replay_status=0
92+
specific_status=0
8993

9094
if [ "$TEST_TYPE" = "all" ] || [ "$TEST_TYPE" = "minimal" ]; then
9195
minimal_output=$(nvim --headless -u tests/minimal/init.lua -c "lua require('plenary.test_harness').test_directory('./tests/minimal', {minimal_init = './tests/minimal/init.lua', sequential = true$FILTER_OPTION})" 2>&1)
96+
minimal_status=$?
9297
clean_output "$minimal_output"
9398

94-
if has_failures "$minimal_output"; then
99+
if [ "$minimal_status" -ne 0 ] || has_failures "$minimal_output"; then
95100
echo -e "${RED}✗ Minimal tests failed${NC}"
96101
else
97102
echo -e "${GREEN}✓ Minimal tests passed${NC}"
@@ -101,9 +106,10 @@ fi
101106

102107
if [ "$TEST_TYPE" = "all" ] || [ "$TEST_TYPE" = "unit" ]; then
103108
unit_output=$(nvim --headless -u tests/minimal/init.lua -c "lua require('plenary.test_harness').test_directory('./tests/unit', {minimal_init = './tests/minimal/init.lua'$FILTER_OPTION})" 2>&1)
109+
unit_status=$?
104110
clean_output "$unit_output"
105111

106-
if has_failures "$unit_output"; then
112+
if [ "$unit_status" -ne 0 ] || has_failures "$unit_output"; then
107113
echo -e "${RED}✗ Unit tests failed${NC}"
108114
else
109115
echo -e "${GREEN}✓ Unit tests passed${NC}"
@@ -113,9 +119,10 @@ fi
113119

114120
if [ "$TEST_TYPE" = "all" ] || [ "$TEST_TYPE" = "replay" ]; then
115121
replay_output=$(nvim --headless -u tests/minimal/init.lua -c "lua require('plenary.test_harness').test_directory('./tests/replay', {minimal_init = './tests/minimal/init.lua'$FILTER_OPTION})" 2>&1)
122+
replay_status=$?
116123
clean_output "$replay_output"
117124

118-
if has_failures "$replay_output"; then
125+
if [ "$replay_status" -ne 0 ] || has_failures "$replay_output"; then
119126
echo -e "${RED}✗ Replay tests failed${NC}"
120127
else
121128
echo -e "${GREEN}✓ Replay tests passed${NC}"
@@ -127,9 +134,10 @@ fi
127134
if [ "$TEST_TYPE" != "all" ] && [ "$TEST_TYPE" != "minimal" ] && [ "$TEST_TYPE" != "unit" ] && [ "$TEST_TYPE" != "replay" ]; then
128135
if [ -f "$TEST_TYPE" ]; then
129136
specific_output=$(nvim --headless -u tests/minimal/init.lua -c "lua require('plenary.test_harness').test_directory('./$TEST_TYPE', {minimal_init = './tests/minimal/init.lua'$FILTER_OPTION})" 2>&1)
137+
specific_status=$?
130138
clean_output "$specific_output"
131139

132-
if has_failures "$specific_output"; then
140+
if [ "$specific_status" -ne 0 ] || has_failures "$specific_output"; then
133141
echo -e "${RED}✗ Specific test failed${NC}"
134142
else
135143
echo -e "${GREEN}✓ Specific test passed${NC}"
@@ -148,9 +156,26 @@ all_output="$minimal_output
148156
$unit_output
149157
$replay_output"
150158

151-
if has_failures "$all_output"; then
159+
if has_failures "$all_output" \
160+
|| [ "$minimal_status" -ne 0 ] \
161+
|| [ "$unit_status" -ne 0 ] \
162+
|| [ "$replay_status" -ne 0 ] \
163+
|| [ "$specific_status" -ne 0 ]; then
152164
echo -e "\n${RED}======== TEST FAILURES SUMMARY ========${NC}"
153165

166+
if [ "$minimal_status" -ne 0 ]; then
167+
echo -e "${RED}Runner exit:${NC} minimal tests exited with code $minimal_status"
168+
fi
169+
if [ "$unit_status" -ne 0 ]; then
170+
echo -e "${RED}Runner exit:${NC} unit tests exited with code $unit_status"
171+
fi
172+
if [ "$replay_status" -ne 0 ]; then
173+
echo -e "${RED}Runner exit:${NC} replay tests exited with code $replay_status"
174+
fi
175+
if [ "$specific_status" -ne 0 ]; then
176+
echo -e "${RED}Runner exit:${NC} specific tests exited with code $specific_status"
177+
fi
178+
154179
# Extract and format failures
155180
failures_file=$(mktemp)
156181
plain_output=$(strip_ansi "$all_output")

tests/unit/commands_handlers_spec.lua

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
local assert = require('luassert')
2+
local stub = require('luassert.stub')
23

34
describe('opencode.commands.handlers', function()
45
local tracked_modules = {
@@ -110,6 +111,64 @@ describe('opencode.commands.handlers', function()
110111
}, err)
111112
end)
112113

114+
it('returns actionable invalid subcommand errors for agent/session/diff handlers', function()
115+
local agent = require('opencode.commands.handlers.agent')
116+
local session = require('opencode.commands.handlers.session')
117+
local diff = require('opencode.commands.handlers.diff')
118+
119+
local ok_agent, err_agent = pcall(agent.command_defs.agent.execute, { 'unknown' })
120+
local ok_session, err_session = pcall(session.command_defs.session.execute, { 'unknown' })
121+
local ok_diff, err_diff = pcall(diff.command_defs.diff.execute, { 'unknown' })
122+
123+
assert.is_false(ok_agent)
124+
assert.same({
125+
code = 'invalid_arguments',
126+
message = 'Invalid agent subcommand. Use: ' .. table.concat(agent.command_defs.agent.completions, ', '),
127+
}, err_agent)
128+
129+
assert.is_false(ok_session)
130+
assert.same({
131+
code = 'invalid_arguments',
132+
message = 'Invalid session subcommand. Use: ' .. table.concat(session.command_defs.session.completions, ', '),
133+
}, err_session)
134+
135+
assert.is_false(ok_diff)
136+
assert.same({
137+
code = 'invalid_arguments',
138+
message = 'Invalid diff subcommand. Use: ' .. table.concat(diff.command_defs.diff.completions, ', '),
139+
}, err_diff)
140+
end)
141+
142+
it('keeps help rendering stable in narrow output windows', function()
143+
local surface = require('opencode.commands.handlers.surface')
144+
local window = require('opencode.commands.handlers.window')
145+
local state = require('opencode.state')
146+
local ui = require('opencode.ui.ui')
147+
148+
local open_input_stub = stub(window.actions, 'open_input')
149+
local is_visible_stub = stub(state.ui, 'is_visible').returns(true)
150+
local set_route_stub = stub(state.ui, 'set_display_route')
151+
local render_lines_stub = stub(ui, 'render_lines')
152+
local width_stub = stub(vim.api, 'nvim_win_get_width').returns(20)
153+
154+
local original_windows = state.store.get('windows')
155+
local test_windows = vim.deepcopy(original_windows or {})
156+
test_windows.output_win = 1
157+
state.store.set_raw('windows', test_windows)
158+
159+
local ok = pcall(surface.actions.help)
160+
161+
state.store.set_raw('windows', original_windows)
162+
open_input_stub:revert()
163+
is_visible_stub:revert()
164+
set_route_stub:revert()
165+
render_lines_stub:revert()
166+
width_stub:revert()
167+
168+
assert.is_true(ok)
169+
assert.stub(render_lines_stub).was_called()
170+
end)
171+
113172
it('keeps command semantic routing in diff revert handler (session target -> nil snapshot)', function()
114173
local called = {}
115174
local diff = require('opencode.commands.handlers.diff')

0 commit comments

Comments
 (0)