Alias completion

[evokateur]

I started out with a Codex implementation then started cleaning until I had what seemed essential. Eventually I used this as a guidepost: https://www.kevinloughead.com/blog/yargs-autocompletion/

Async code in .completion(.. seems to let something go off and do things during alias name completion, like call the start-timer handler in a non-deterministic way, resulting in harmless but confusing Alias not founds in stderr

Works in Zsh and Bash

√ ~ $ hrvst start p
personal             pydantic             python-architecture

√ ~ $ hrvst log --notes "test note" 3 c
core-ai    cv-agents

√ ~ $ hrvst alias delete job-
job-apply   job-search

[kgajera]

Thanks for this @evokateur ! Will be able to review it later this week.

[kgajera]

@claude Review this PR

[claude]

Claude finished @kgajera's task in 2m 49s —— View job

---

PR Review: Alias Completion

• Gather context and read changed files
• Analyze diff and review code
• Post review feedback

---

Overall this is a clean, focused implementation. The sync-over-async switch is the right call for the reason given (avoiding handler side-effects during completion), and the three-command coverage (start, log, alias delete) is complete — stop and note don't accept aliases, so nothing is missing.

A few observations:

src/utils/config.ts

Path duplication. getAliasNamesSync constructs the config file path manually (path.join(ospath.home(), ".hrvst", "config.json")), duplicating the logic already in the private configPath() function. Since configPath() is async (it creates the directory as a side-effect), a shared sync constant would reduce duplication:

// shared constant at module level
const CONFIG_FILE_PATH = path.join(ospath.home(), ".hrvst", "config.json");

Both getAliasNamesSync and configPath could then reference it. Minor, but worth doing since there's now a second consumer of this path. Fix this →

src/cli.ts

Unused current parameter in shouldCompleteAlias. The function accepts current: string but never uses it. TypeScript's strict mode doesn't flag this (no noUnusedParameters), but it's a mild code smell. Either remove it from the function signature and drop it from the call site, or prefix it _current per TS convention to signal intent. Fix this →

Implicit assumption about argv._[0]. The checks start at argv._[1] because index 0 is the binary name (hrvst), passed through by the shell completion script's "${COMP_WORDS[@]}" expansion. This is correct for yargs-generated bash/zsh scripts but non-obvious to a future reader. A one-line comment would help:

// argv._[0] is the binary name (passed via COMP_WORDS in shell completion scripts)
if (argv._[1] === "start" && argv._.length === 3) {

Exact-length guards are fine but worth noting. The === 3 / === 4 checks intentionally fire only at the exact positional slot for the alias argument — not before the slot (when it wouldn't be an alias), not after (when it's already complete). This is correct. It does mean any future positional change to these commands would require a matching update here.

---

Nothing blocking. The async-to-sync refactor is well-motivated and the implementation is straightforward. The path duplication and unused parameter are the main things worth cleaning up before merge.