Skip to content

feat: add Zsh shell completion support#79

Open
jelloeater-agent wants to merge 1 commit into
aannoo:mainfrom
jelloeater-agent:main
Open

feat: add Zsh shell completion support#79
jelloeater-agent wants to merge 1 commit into
aannoo:mainfrom
jelloeater-agent:main

Conversation

@jelloeater-agent

Copy link
Copy Markdown

Add Zsh shell completion for all hcom commands via a new hcom completion zsh command.

Changes

New: hcom completion zsh command

  • Generates a complete _hcom Zsh completion function covering:
    • All 18 CLI commands (send, list, events, stop, start, listen, status, config, hooks, archive, reset, transcript, bundle, kill, term, relay, run, update, completion)
    • All 11 released tools + aliases (claude, gemini, codex, opencode, kilo/kilocode, pi/pi-agent, omp/omp-agent, antigravity/agy, cursor/cursor-agent, kimi, copilot)
    • All hook command names (70+)
    • Resume/fork commands (r/resume, f/fork)
    • Global flags (--name, --go, --help, -h, --version, -v, --new-terminal)
    • Numeric count prefix (hcom 3 claude ...)
    • Per-tool args flags (e.g. --claude-args, --gemini-args)
    • All per-command flags with descriptions

--install flag

  • Writes the completion script to a writable directory in $fpath or standard locations (~/.zsh/completion, /usr/local/share/zsh/site-functions/, ~/.oh-my-zsh/completions/)

Generation from integration specs

  • The completion script is built programmatically from integration_spec::ALL, ensuring it stays in sync as tools are added or removed
  • All 1993 tests pass

Usage

# Source directly
eval "$(hcom completion zsh)"

# Install permanently
hcom completion zsh --install

* New 'hcom completion zsh' command generates a _hcom Zsh completion
  function covering all commands, tools, hooks, and flags
* Supports '--install' flag to write completions to a detected
  directory
* Completion script is generated programmatically from integration_spec,
  staying in sync as tools are added or removed
* All 1993 existing tests pass

Usage:
  eval "$(hcom completion zsh)"
  hcom completion zsh --install
@aannoo

aannoo commented Jul 21, 2026

Copy link
Copy Markdown
Owner

thanks for the pr. here is ai review:

PR #79feat: add Zsh shell completion support

Verdict: Request changes. Do not merge.

The feature idea is reasonable (a command-heavy CLI benefits from completion), but this
implementation does not work beyond a static top-level menu, has a shell-injection hazard in
its own documented install command, and maintains a ~600-line hand-copy of the CLI that is
already stale on arrival. Patching the small bugs would still leave the design problems.
Findings below are verified against the diff and current main.


Blockers

1. All contextual completion is unreachable — the feature effectively doesn't work

completion.rs reads the subcommand as local cmd="$words[1]". In a Zsh completion
function $words[1] is always the command itself (hcom); the subcommand is $words[2].

Consequence: cmd is always "hcom", so no case arm ever matches. Every input
(hcom send --<TAB>, hcom claude --<TAB>, hcom 3 claude --<TAB>) falls through to the
same static top-level _alternative menu. The numeric-prefix reindex
([[ "$cmd" = <-> ]] && cmd="$words[2]") also never fires, since "hcom" isn't numeric.

The unused global_opts array signals a global-flag-skipping state machine was intended but
never written. Robust logic must skip global flags and their values, handle the [N] <tool>
prefix, and track nested subcommands — not just switch 12.

2. The documented eval "$(hcom completion zsh)" can execute unread message text

completion is routed as an ordinary native command through dispatch_native_command
(router.rs), which after the handler calls maybe_deliver_pending_messages(...) and
appends unread hcom messages to stdout (cli_context.rs:181, with has_json=false).

So under a resolved Codex/adhoc identity, hcom completion zsh can emit the completion
script followed by verbatim message bodies. Feeding that to eval (or redirecting it into
_hcom) means a multiline message can drop arbitrary shell syntax onto a fresh line and get
evaluated / persisted into the autoloaded completion file.

This is not fixable by escaping or --json. Completion output must be pure. Dispatch it
before the DB/identity/status/delivery machinery — the same way relay daemon is special-
cased earlier in dispatch_native_command — or introduce a side-effect-free command class.

3. Direct sourcing never registers the completion

The script is #compdef hcom … _hcom "$@". That shape works only when installed as an
autoloadable file that compinit picks up from $fpath. When eval'd or source'd
directly (both documented in help.rs and the script header), _hcom is defined and called
once outside any completion context but is never bound to hcom, so Tab does nothing. The
direct-source path needs an explicit compdef _hcom hcom after loading compinit — or drop
the claim. (And per Blocker 2, the eval form should be removed regardless.)


Correctness / accuracy defects

  • --<tool>-args flags are invented. generate_tool_arms turns spec.launch.args_env
    into flags like --claude-args. But args_env names an environment variable
    (HCOM_CLAUDE_ARGS, integration_spec.rs:121), not a launch flag. The launch parser
    forwards unknown tokens to the underlying tool, so a completed --claude-args gets passed
    to Claude as a bogus native option.

  • --device (user-facing) is missing from the tool arms. The launch parser accepts
    --device (launch.rs:614), documented as "Launch on a remote relay device"
    (README.md:283, help.rs:710) — it's not offered. (Note: the parser also accepts
    --batch-id, --run-here, --no-run-here, but those are intentionally internal/power-
    user — help.rs:933 explicitly keeps --run-here/--no-run-here out of help — so
    completion is right to omit them. --system is just an alias for --hcom-system-prompt,
    which is already offered. So the only genuine gap here is --device.)

  • Terminal values are hardcoded to 6 (default kitty wezterm tmux zellij iterm). The
    project ships many more presets (ghostty, alacritty, cmux, waveterm, herdr, warp, …) plus
    user-defined ones — all omitted.

  • resume/fork case arms are dead and misleading. Arms exist for (r|resume) /
    (f|fork), but (a) they're unreachable per Blocker 1, and (b) is_launch_tool only accepts r
    or f (router.rs:48-50), so the long forms hcom resume/hcom fork don't route at all.

  • Resume/fork don't complete their required target. hcom r/hcom f require a
    name-or-session positional; completion offers only flags, missing the single most useful
    thing (active/stopped agent names, session IDs).

  • Nested command grammars are flattened or wrong. events (sub/unsub/launch),
    transcript (search/timeline), bundle (list/show/cat/chain/create/…), hooks
    (add/remove + tool), term (inject/debug), relay (many subcommands), run (dynamic
    scripts) all lose their subcommands. Some flags are attached to the wrong level
    (e.g. transcript --live and bundle --title belong to transcript search / bundle create). Suggesting invalid commands is worse than suggesting nothing.

  • 64 hook command names are surfaced as top-level suggestions. These are integration
    plumbing that expect tool-specific stdin/argv and can mutate lifecycle/status state; they
    shouldn't be public completions. Notably the router's internal pty/relay-worker modes
    are correctly omitted — "routable" ≠ "publicly discoverable." There's no public/internal
    taxonomy to lean on.


Installer (--install) is unsafe and non-portable

zsh_completion_dir reads std::env::var("fpath") split on whitespace. Zsh's lowercase
fpath is a shell array not normally exported; the exported scalar is uppercase FPATH,
colon-joined — so this reads the wrong variable with the wrong delimiter. Other issues:
writability is probed by writing/deleting .hcom_completion_test (clobbers a same-named user
file if present, and is racy); a candidate dir is chosen merely because it or its parent
exists, without confirming writability; _hcom is overwritten unconditionally (incl.
package-manager copies); the write isn't atomic; there's no fpath activation, uninstall, or
Windows handling despite Windows being supported. Recommend dropping --install from v1 and
leaving install to packaging or an explicit --dir.


Maintenance model: hand-duplicated CLI that's already stale

The PR frames the script as "generated from integration_spec::ALL," but only tool names,
aliases, and hook names are actually derived. Every command, description, flag, positional,
and nested subcommand is hand-copied. There are now parallel registries in router.rs,
help.rs, each command's Clap parser, the manual subcommand dispatchers, and
completion.rs::CLI_COMMANDS — and the new list is already out of sync: CLI_COMMANDS
omits completion itself (so hcom <TAB> won't suggest the command this very PR adds), while
the PR description claims "all 18 commands" then lists 19. integration_spec.rs is documented
as a tool-integration plane, not a CLI schema; this PR gives it a job it wasn't built for.

The added tests don't catch any of this: they are substring checks (starts_with "#compdef", contains "claude", contains _arguments), and zsh_script_valid_syntax never
runs zsh -n. zsh_script_contains_all_commands iterates the same stale CLI_COMMANDS
constant, so it can only confirm the generator contains its own list — it can't detect drift.
All the blockers above pass the suite green.


Minor

  • Help alignment regression (help.rs:938-942): update and completion are emitted
    with 3 leading spaces vs 2 for every other command in the top-level list — visibly
    misaligned. Reinforces the lack of rendered-output tests.

Recommendation

Request changes; this implementation should be replaced, not incrementally patched. Even
after fixing $words[2], registration, the omitted command, and alignment, the core problems
remain: a hand-maintained second copy of the CLI that will silently drift, integration
metadata treated as a command schema, internal hooks exposed as user commands, and a
completion path wired through DB/identity/message-delivery.

A mergeable version would:

  1. Generate grammar from the existing Clap parsers (e.g. clap_complete) over a curated
    public command spec, rather than a handwritten case tree.
  2. Bypass DB/identity/status/delivery entirely and guarantee script-only stdout.
  3. Exclude hook and internal worker commands.
  4. Complete the high-value dynamic targets (agent names, tags, config keys, workflow scripts,
    terminal presets) via explicitly read-only queries.
  5. Drop --install or implement a safe, portable, FPATH-correct install with uninstall.
  6. Carry real Zsh behavioral tests (zsh -n, autoload smoke, per-word assertions) and a
    parity test against the canonical command definitions.

Scope note

Whether hcom — an agent-coordination CLI where agents/scripts are the primary callers — even
wants a Zsh-human-Tab feature is a fair question. A small, safe, generated completion aid for
contributors and recovery is defensible; ~600 lines of duplicated, partly-incorrect machinery
for a static top-level menu is not. That's a product call for the maintainer, but it doesn't
change the engineering verdict: not this patch.

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.

2 participants