fix(tui): tab completion cursor, trailing space, and /quit alias (#183)#185
Conversation
Three related bugs in the React TUI slash-command flow (fixes HKUDS#183): 1. Tab completion left a trailing space after the command, so hitting Enter right after Tab would submit "/exit " (with arg) instead of running /exit. Drop the space — user can add one themselves if they want to type an argument. 2. After a programmatic setInput() from completion/history, the MultilineTextInput cursor stayed at its previous offset instead of moving to the end of the newly-inserted text. Subsequent keystrokes landed in the middle of the completed command ("/eXxit" when typing X after Tab-completing "/exit"). Track the last internally-authored value via a ref; when the incoming value differs, treat it as an external change and snap the cursor to the end. 3. /quit is the other obvious word for "exit" and users were surprised when it wasn't registered. Add an `aliases` field to SlashCommand and make /quit an alias of /exit. help/list output deduplicates aliases so they resolve via lookup without cluttering the command listing. Tests: - tests/test_commands/test_registry.py: new tests for the alias path and for help/list dedup. Both pass locally. - MultilineTextInput doesn't have a standalone unit test for the cursor behavior (uses node:test + ink render harness, can't run without node toolchain on this machine), but the fix is a local, obvious change in a single component.
tjb-tech
left a comment
There was a problem hiding this comment.
Reviewed and tested locally. This implementation fixes #183 without relying on remounting the input component: it distinguishes internally-authored input changes from external setInput updates, so Tab completion and history/programmatic updates move the cursor to the end while normal editing preserves cursor position.\n\nVerified locally on top of current main:\n- PYTHONPATH=src pytest -q tests/test_commands/test_registry.py\n- PYTHONPATH=src pytest -q tests/test_commands/test_cli.py tests/test_commands/test_registry.py\n- ruff check src/openharness/commands/registry.py tests/test_commands/test_registry.py\n- cd frontend/terminal && npm install --no-audit --no-fund && ./node_modules/.bin/tsc --noEmit\n- cd frontend/terminal && ./node_modules/.bin/tsx --test src/components/PromptInput.test.tsx
- feat(commands): add aliases field to SlashCommand; /quit becomes an
alias of /exit via aliases=("quit",). _canonical_names tracks
registration order so help_text/list_commands omit duplicates.
- feat(utils): restore helpers.py (get_data_path, split_message,
safe_filename) used by channel adapters (cherry-picked from 75c0481).
- test: add regression tests for alias dedup and helpers.
Ported from main commits:
0bba07f fix(tui): complete slash commands without cursor drift (HKUDS#185)
75c0481 fix(channels): restore shared helper utilities
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Three related bugs in the React TUI slash-command flow (fixes HKUDS#183): 1. Tab completion left a trailing space after the command, so hitting Enter right after Tab would submit "/exit " (with arg) instead of running /exit. Drop the space — user can add one themselves if they want to type an argument. 2. After a programmatic setInput() from completion/history, the MultilineTextInput cursor stayed at its previous offset instead of moving to the end of the newly-inserted text. Subsequent keystrokes landed in the middle of the completed command ("/eXxit" when typing X after Tab-completing "/exit"). Track the last internally-authored value via a ref; when the incoming value differs, treat it as an external change and snap the cursor to the end. 3. /quit is the other obvious word for "exit" and users were surprised when it wasn't registered. Add an `aliases` field to SlashCommand and make /quit an alias of /exit. help/list output deduplicates aliases so they resolve via lookup without cluttering the command listing. Tests: - tests/test_commands/test_registry.py: new tests for the alias path and for help/list dedup. Both pass locally. - MultilineTextInput doesn't have a standalone unit test for the cursor behavior (uses node:test + ink render harness, can't run without node toolchain on this machine), but the fix is a local, obvious change in a single component. Co-authored-by: Yufeng He <40085740+universeplayer@users.noreply.github.com>
Fixes #183.
Problems
Three bugs in the React TUI slash-command flow, all reported in the same issue.
1. Trailing space on Tab-completion
App.tsxcompleted/exitto/exit(note the space), so hitting Enter right after Tab would submit/exit<space>with an empty arg instead of running/exitdirectly.2. Cursor stuck after programmatic setInput
MultilineTextInputkept itscursorOffsetacrossvalueprop changes and only clamped the cursor down viaMath.min(prev, value.length). After Tab-completion (which grows the value), the cursor stayed at the old offset instead of moving to the end of the completed text. Subsequent keystrokes landed in the middle — e.g. typingXafter Tab-completing/exitproduced/eXxit. Same class of bug for history recall (↑), which also changes value externally.3.
/quitnot registered/quitis the other obvious word for "exit" and users were surprised when it didn't work.Fixes
frontend/terminal/src/App.tsxDrop the trailing space from the completion path. Users who want to pass an argument can type the space themselves.
frontend/terminal/src/components/PromptInput.tsxTrack the last internally-authored value via
lastInternalValueRef. On eachuseEffect([value]), if the incoming value matches the ref, the change came from our ownonChangeand the cursor was already positioned correctly by the handler — do nothing. Otherwise the change is external (tab completion, history recall, esc-clear) and we snap the cursor to the end. Wrapped all internalonChange(nextValue)calls in acommitValuehelper that updates the ref before delegating.src/openharness/commands/registry.pyAdded
aliases: tuple[str, ...] = ()toSlashCommand.CommandRegistry.registernow installs each alias under its own key pointing at the same command object.help_text/list_commandsdedupe via a separate_canonical_nameslist so aliases resolve vialookupwithout cluttering the listing.Then:
SlashCommand(\"exit\", ..., aliases=(\"quit\",)).Testing
New Python unit tests in
tests/test_commands/test_registry.py:test_quit_is_alias_for_exit— `/exit` and `/quit` resolve to the sameSlashCommandobject.test_help_and_list_do_not_duplicate_aliases—list_commands()has exactly oneexitentry; `/quit` does not appear in `help_text()` output.Both pass locally (
python3.12 -m pytest tests/test_commands/test_registry.py::test_quit_is_alias_for_exit tests/test_commands/test_registry.py::test_help_and_list_do_not_duplicate_aliases). Rest oftest_registry.pyis unaffected by this change (one pre-existing failure intest_auth_feedback_and_project_context_commandswas already failing onmainbecause the test reads a real API key from the host environment; unrelated to this PR).I don't have a node toolchain on this machine so I couldn't run the ink-based
PromptInput.test.tsxharness, but the cursor fix is local to one component and the change is straightforward.Before / After
Before:
/, picker opens, Tab →/exit(with space), Enter → runs/exitwith empty arg/, Tab → cursor stays at offset 1, typingXproduces/Xxit(or similar garbled output)/quit, Enter →Unknown command: quitAfter:
/exit, Enter runs it cleanly/quitresolves to the same handler as/exit