Skip to content

fix(tui): tab completion cursor, trailing space, and /quit alias (#183)#185

Merged
tjb-tech merged 1 commit intoHKUDS:mainfrom
he-yufeng:fix/tui-tab-completion-183
Apr 23, 2026
Merged

fix(tui): tab completion cursor, trailing space, and /quit alias (#183)#185
tjb-tech merged 1 commit intoHKUDS:mainfrom
he-yufeng:fix/tui-tab-completion-183

Conversation

@he-yufeng
Copy link
Copy Markdown
Contributor

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.tsx completed /exit to /exit (note the space), so hitting Enter right after Tab would submit /exit<space> with an empty arg instead of running /exit directly.

2. Cursor stuck after programmatic setInput

MultilineTextInput kept its cursorOffset across value prop changes and only clamped the cursor down via Math.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. typing X after Tab-completing /exit produced /eXxit. Same class of bug for history recall (), which also changes value externally.

3. /quit not registered

/quit is the other obvious word for "exit" and users were surprised when it didn't work.

Fixes

frontend/terminal/src/App.tsx

Drop the trailing space from the completion path. Users who want to pass an argument can type the space themselves.

frontend/terminal/src/components/PromptInput.tsx

Track the last internally-authored value via lastInternalValueRef. On each useEffect([value]), if the incoming value matches the ref, the change came from our own onChange and 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 internal onChange(nextValue) calls in a commitValue helper that updates the ref before delegating.

src/openharness/commands/registry.py

Added aliases: tuple[str, ...] = () to SlashCommand. CommandRegistry.register now installs each alias under its own key pointing at the same command object. help_text / list_commands dedupe via a separate _canonical_names list so aliases resolve via lookup without 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 same SlashCommand object.
  • test_help_and_list_do_not_duplicate_aliaseslist_commands() has exactly one exit entry; `/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 of test_registry.py is unaffected by this change (one pre-existing failure in test_auth_feedback_and_project_context_commands was already failing on main because 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.tsx harness, but the cursor fix is local to one component and the change is straightforward.

Before / After

Before:

  • Type /, picker opens, Tab → /exit (with space), Enter → runs /exit with empty arg
  • Type /, Tab → cursor stays at offset 1, typing X produces /Xxit (or similar garbled output)
  • Type /quit, Enter → Unknown command: quit

After:

  • Tab completes to /exit, Enter runs it cleanly
  • Tab moves cursor to end, subsequent typing appends normally
  • /quit resolves to the same handler as /exit

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.
Copy link
Copy Markdown
Collaborator

@tjb-tech tjb-tech left a comment

Choose a reason for hiding this comment

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

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

@tjb-tech tjb-tech merged commit 0bba07f into HKUDS:main Apr 23, 2026
yl-jiang added a commit to yl-jiang/OpenHarness that referenced this pull request Apr 23, 2026
- 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>
arik08 pushed a commit to arik08/MyHarness that referenced this pull request Apr 26, 2026
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>
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.

bug(tui): Tab completion for slash commands leaves cursor at wrong position and appends trailing space

2 participants