Skip to content

feat: add shell completions and improve switch UX#111

Open
behnambagheri wants to merge 6 commits into
Loongphy:mainfrom
behnambagheri:feat/fish-completion-cx-switch-ux
Open

feat: add shell completions and improve switch UX#111
behnambagheri wants to merge 6 commits into
Loongphy:mainfrom
behnambagheri:feat/fish-completion-cx-switch-ux

Conversation

@behnambagheri

@behnambagheri behnambagheri commented May 20, 2026

Copy link
Copy Markdown

Summary

  • add shell completions for Bash, Zsh, and Fish via codex-auth completion <shell>
  • improve switch completion suggestions across shells and keep the switch TUI input handling fixes
  • remove the cx alias from the package and docs

Details

  • add completion bash, completion zsh, and completion fish
  • keep Fish switch suggestions dynamic and sourced from stored accounts
  • add Bash switch suggestions for both display numbers and emails
  • add Zsh switch suggestions and tune the presentation for command completion and switch completion
  • remove the cx npm binary alias and related docs/examples
  • keep the earlier switch UX fixes for Esc exit handling and arrow-key navigation
  • add screenshots requested for review directly in this PR

Validation

  • HOME=/tmp/codex-auth-pr-update ZIG_GLOBAL_CACHE_DIR=/tmp/codex-auth-pr-update/zig-global-cache ZIG_LOCAL_CACHE_DIR=/tmp/codex-auth-pr-update/zig-local-cache zig build run -- list
  • manual completion verification in Fish, Bash, and Zsh

Screenshots

Fish command completion

Fish command completion

Fish switch suggestions

Fish switch suggestions

Bash switch suggestions

Bash switch suggestions

Zsh command completion

Zsh command completion

Zsh switch suggestions

Zsh switch suggestions

@Loongphy

Copy link
Copy Markdown
Owner

Thanks, this looks great overall!

A few requests before we move forward:

  1. Could we also support Bash and Zsh completions in addition to Fish? It would be nice to keep completion support consistent across the common shells.
  2. Please remove the cx alias from this PR. It feels a bit too opinionated for the project itself. If you prefer using cx, you can still define that alias locally in Fish.
  3. Could you add a few screenshots to the PR description to demonstrate the implementation? For example, screenshots showing command completion and the switch account suggestions would be very helpful for review.

@behnambagheri behnambagheri changed the title feat: add Fish completions and improve switch UX feat: add shell completions and improve switch UX May 20, 2026
@behnambagheri

Copy link
Copy Markdown
Author

Thanks for the review.

I addressed the requested follow-up changes in this PR:

  • added Bash and Zsh completion support alongside Fish via codex-auth completion <shell>
  • removed the cx alias from the package and docs
  • added screenshots to the PR description for Fish, Bash, and Zsh completion behavior, including switch suggestions

I also rechecked the completion flows after the shell-specific updates and pushed the follow-up changes to this same PR.

@greptile-apps

greptile-apps Bot commented May 22, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds codex-auth completion <bash|zsh|fish> to generate shell completion scripts, improves switch TUI UX (Esc exits, arrow keys always clear typed number input), and removes the cx binary alias from the npm package.

  • Shell completions: New completion.zig emits fully-featured Bash, Zsh, and Fish scripts; completion query switch outputs tab-separated display_number\ email rows that the scripts consume dynamically. A parseDisplayNumber change in query.zig lets the NUM:EMAIL Bash completion format resolve directly by display number.
  • TUI UX fixes: Removes the Kitty keyboard enhancement protocol, adds lone-Esc-to-quit detection, and moves number_len = 0 outside navigation conditionals so typed digits are cleared even when the cursor is already at a boundary.
  • Alias removal: cx binary removed from package.json and docs; resolveLocalBinary dev-fallback added to bin/codex-auth.js.

Confidence Score: 5/5

Safe to merge; all changed Zig paths are tested and the memory management in writeUniqueCandidate is correct.

The memory ownership pattern in writeUniqueCandidate (dupe to errdefer to print to putNoClobber) is correct. Shell script generation is straightforward string output with good test coverage. The Bash colon word-break behavior and the unreachable Zsh switch branch are UX and maintenance concerns that do not affect correctness of the current release.

src/cli/completion.zig — Bash colon word-break handling and the unreachable Zsh switch branch in the args state deserve a second look before the next completion iteration.

Important Files Changed

Filename Overview
src/cli/completion.zig New file: emits Bash/Zsh/Fish completion scripts and handles the completion query switch data source. Two issues: Bash colon word-break handling, and an unreachable Zsh switch branch in the args state.
src/workflows/completion.zig New thin workflow shim; routes shell vs query completion correctly, but uses a forced codex_home.? unwrap that would panic if the optional is ever null.
src/cli/commands/completion.zig New command parser for completion shell and completion query target; handles all error cases and forwards correctly.
src/cli/tui.zig Removes the Kitty keyboard enhancement protocol and fixes lone Escape key to trigger quit; arrow keys now unconditionally map to move_up/move_down.
src/cli/picker_switch.zig UX improvement: number_len is now reset unconditionally on any navigation key press, even when the cursor is already at a boundary.
src/cli/live_switch.zig Mirrors picker_switch.zig UX fix: number input is cleared on all navigation, not only when selection actually moves.
src/workflows/query.zig parseDisplayNumber now strips the email label after the first colon, enabling the NUM:EMAIL Bash completion format to resolve directly to the display number.
tests/cli_behavior_test.zig Good coverage of new completion parsing, script content, and help rendering; also adds a NUM:EMAIL display-number resolution test.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["codex-auth completion shell"] --> B["writeCompletion()"]
    B --> C1["writeBashCompletion()"]
    B --> C2["writeZshCompletion()"]
    B --> C3["writeFishCompletion()"]
    C1 & C2 & C3 --> D["Installed script calls: codex-auth completion query switch"]
    D --> E["handleCompletion(.query)"]
    E --> F["writeSwitchQueryCompletion()"]
    F --> G["loadRegistry + buildDisplayRows"]
    G --> H["writeUniqueCandidate() outputs NUM TAB EMAIL per line"]
    H --> I1["Bash: awk to NUM:EMAIL + cut to EMAIL"]
    H --> I2["Zsh: compadd NUM with email as description"]
    H --> I3["Fish: NUM shown, email as hint"]
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A["codex-auth completion shell"] --> B["writeCompletion()"]
    B --> C1["writeBashCompletion()"]
    B --> C2["writeZshCompletion()"]
    B --> C3["writeFishCompletion()"]
    C1 & C2 & C3 --> D["Installed script calls: codex-auth completion query switch"]
    D --> E["handleCompletion(.query)"]
    E --> F["writeSwitchQueryCompletion()"]
    F --> G["loadRegistry + buildDisplayRows"]
    G --> H["writeUniqueCandidate() outputs NUM TAB EMAIL per line"]
    H --> I1["Bash: awk to NUM:EMAIL + cut to EMAIL"]
    H --> I2["Zsh: compadd NUM with email as description"]
    H --> I3["Fish: NUM shown, email as hint"]
Loading

Reviews (6): Last reviewed commit: "fix: avoid completion key double free" | Re-trigger Greptile

Comment thread src/cli/completion.zig
@pkg-pr-new

pkg-pr-new Bot commented May 22, 2026

Copy link
Copy Markdown

Open in StackBlitz

@loongphy/codex-auth-darwin-arm64

npx https://pkg.pr.new/@loongphy/codex-auth-darwin-arm64@2ae55f1

@loongphy/codex-auth-darwin-x64

npx https://pkg.pr.new/@loongphy/codex-auth-darwin-x64@2ae55f1

@loongphy/codex-auth-linux-arm64

npx https://pkg.pr.new/@loongphy/codex-auth-linux-arm64@2ae55f1

@loongphy/codex-auth-linux-x64

npx https://pkg.pr.new/@loongphy/codex-auth-linux-x64@2ae55f1

@loongphy/codex-auth-win32-arm64

npx https://pkg.pr.new/@loongphy/codex-auth-win32-arm64@2ae55f1

@loongphy/codex-auth-win32-x64

npx https://pkg.pr.new/@loongphy/codex-auth-win32-x64@2ae55f1

@loongphy/codex-auth

npx https://pkg.pr.new/@loongphy/codex-auth@2ae55f1

commit: 2ae55f1

@Loongphy

Copy link
Copy Markdown
Owner

@greptileai

@andrei-hasna

Copy link
Copy Markdown

Merge review on 2026-07-05: not merging yet. Blockers: (1) PR checks are not green: GitHub reports mergeStateStatus=UNSTABLE and Build & Test (windows-latest) is failed/cancelled, while other checks are passing. (2) The branch is stale against current main: origin/main is 34 commits ahead of PR head 2ae55f1, so the successful checks are for an old head. (3) Existing completion review issues still reproduce in the PR diff: generated Bash/Zsh/Fish command lists omit the current top-level app command, and Zsh switch completion reads the number+email TSV but only offers the email column. Local probes: git merge-tree produced a clean synthetic merge tree and git diff --check was clean, so I did not find a textual conflict. Local Zig validation could not be run in the worker because zig is not installed. Please update/rebase the PR, fix the completion gaps, and rerun CI (including Windows) before merge.

@andrei-hasna

Copy link
Copy Markdown

Additional merge review on 2026-07-05: still not merging.

Evidence:

  • PR head is 2ae55f1 while main is 22d87d1; git rev-list --left-right --count pr-111...origin/main shows the PR has 3 unique commits and current main has 34 newer commits.
  • GitHub reports mergeable=MERGEABLE but mergeStateStatus=UNSTABLE; gh pr checks shows Build & Test (windows-latest) failed/cancelled.
  • A synthetic merge tree was clean (e21a25c092792dbb06723b18dd4a07f5a557c75c) and git diff --check was clean, so I do not see a textual conflict.
  • The merged completion implementation is stale against current main: generated Bash/Zsh/Fish command lists omit the existing app command, Zsh switch completion only offers emails rather than display numbers, Bash invokes the dynamic switch query twice per completion, and switch completions do not account for current codex-auth - / codex-auth switch - previous-account UX.
  • Local Zig validation could not be run in this worker because zig is not installed.

Please update/rebase the branch, fix/resolve the completion review issues, and rerun CI including Windows before merge.

@andrei-hasna

Copy link
Copy Markdown

Worker merge review on 2026-07-05: not merging.

Current blockers:

  • Branch freshness: PR head is 2ae55f1, current main is 22d87d1, and git rev-list --left-right --count origin/main...origin/pr/111 shows 34 3; the green checks are from the old PR head/base lineage.
  • CI: GitHub reports mergeable=MERGEABLE but mergeStateStatus=UNSTABLE; gh pr checks shows Build & Test (windows-latest) failed/cancelled while the other checks pass.
  • Completion correctness: current main has a real top-level app command, but the PR's generated Bash/Zsh/Fish command lists omit app. Zsh switch completion also reads the number+email TSV but only offers the email column via compadd, so display-number completion is missing.
  • Local validation: git diff --check origin/main...origin/pr/111 passed and git merge-tree --write-tree origin/main origin/pr/111 produced a clean synthetic merge tree (e21a25c092792dbb06723b18dd4a07f5a557c75c), so I do not see a textual conflict. Full Zig validation could not run in this worker because zig is not installed (zig version and zig env both returned command not found).
  • Adversarial reviewer independently recommended not merging for the same blockers: stale branch, unstable/cancelled Windows check, confirmed completion omissions, and blocked local Zig validation.

Please update/rebase the branch, fix the completion gaps, and rerun CI including Windows before merge.

@andrei-hasna

Copy link
Copy Markdown

Worker merge review on 2026-07-05: still not merging PR #111.

Fresh evidence:

  • GitHub state: PR is open, mergeable at the textual layer, but mergeStateStatus is UNSTABLE. gh pr checks 111 --repo Loongphy/codex-auth still reports Build & Test (windows-latest) as failed while other checks pass.
  • Branch freshness: current origin/main is 22d87d1531420102fa2f3d51d134f29344dda27c; PR head is 2ae55f18ba7c4ce665a4345d5c8af6312c1b5d9c; git rev-list --left-right --count origin/main...origin/pr/111 reports 34 3.
  • Conflict/probe: git merge-tree --write-tree origin/main origin/pr/111 produced synthetic tree e21a25c092792dbb06723b18dd4a07f5a557c75c, and git diff --check origin/main...origin/pr/111 is clean, so I did not find a textual conflict.
  • Local validation on exported synthetic merge tree with Zig 0.16.0 under /tmp: zig build test passed after isolating HOME/cache and PATH; required zig build run -- list passed with CODEX_HOME=/tmp/codex-auth-pr111-validate/codex-home.
  • Correctness blocker remains: generated Bash/Zsh/Fish completions omit current top-level app from command lists/guards; Zsh switch completion reads number+email TSV but only offers the email column via compadd, so display-number switch completion is missing. These were confirmed from generated completion output on the synthetic merge tree.
  • Adversarial reviewer independently recommended blocking merge for stale branch, failed Windows CI, nontrivial merge, and stale completions.

Please update/rebase the branch onto current main, fix the completion command coverage and Zsh switch suggestions, and rerun CI including Windows before merge.

@behnambagheri behnambagheri force-pushed the feat/fish-completion-cx-switch-ux branch from 2ae55f1 to 1f7d55d Compare July 6, 2026 14:37
@behnambagheri

Copy link
Copy Markdown
Author

I pushed the follow-up fixes in 5ee5c1f.

This update addresses the remaining completion review items:

  • adds previous-account completion support for codex-auth - and codex-auth switch -
  • keeps app covered in Bash, Zsh, and Fish completions
  • updates Zsh switch completion to insert display numbers while showing emails as descriptions
  • improves Bash switch completion with number:email candidates and resolves those by the leading display number
  • keeps Bash from calling codex-auth completion query switch more than once per completion invocation

Local validation:

  • zig build run -- list passed
  • Bash/Zsh/Fish generated completion syntax checks passed
  • Bash completion simulation passed
  • git diff --check passed

GitHub created new CI runs for this commit, but they are currently marked action_required, so I believe a maintainer needs to approve/run them.

Comment thread src/cli/completion.zig Outdated
Comment thread src/cli/completion.zig
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.

3 participants