feat(init): add shell completion installation step to mgw:init#168
Conversation
Adds install_completions step to mgw:init that detects the user's shell (bash/zsh/fish), prompts to install completions from the completions/ directory to the appropriate shell config dir, and skips gracefully when non-interactive or when completions are unavailable. Closes #124 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
snipcodeit
left a comment
There was a problem hiding this comment.
Review: PR #168 — shell completion installation step in mgw:init
The UX design is good — opt-in prompt with sensible defaults, idempotent re-run, graceful skip in CI. Two issues need addressing.
🔴 require.resolve("mgw/package.json") fails in development/local installs
The step uses:
MGW_PKG_DIR=$(node -e "require.resolve('mgw/package.json')" 2>/dev/null | xargs dirname)require.resolve("mgw/package.json") works when mgw is installed as a named global package. But during local development (node bin/mgw.cjs or node dist/bin/mgw.cjs from the repo), the package name mgw may not be resolvable this way. The resolved path might point to a node_modules cache, not the local completions/ directory.
A more robust approach uses __dirname relative to the running script:
# In the context of bin/mgw.cjs (resolved to dist/bin/mgw.cjs at runtime):
MGW_PKG_DIR=$(node -e "console.log(require('path').resolve(__dirname, '..', '..'))")
# Or for the installed case: navigate from dist/bin/ up to package rootSince the executor runs this as pseudocode inside a Claude session, the actual resolution should use the same getCommandsDir() path resolution pattern already used elsewhere in bin/mgw.cjs.
🔴 Merge conflict with PR #169
Both this PR and PR #169 modify commands/init.md — specifically both add new <step> blocks and update the <report> section. They will conflict when merged sequentially. The two steps need to be reconciled:
- This PR adds:
install_completionsstep + completion status line in report - PR #169 adds:
run_config_wizardstep + config.json status line in report
Both PRs should coordinate on merge order. Recommend merging one, then rebasing the other on top and resolving the conflict. Add a note in both PR descriptions referencing this dependency.
🟡 zsh ~/.zsh/completions/ is not in default $fpath
The target path ~/.zsh/completions/ is not in the default zsh $fpath. Users must also add:
fpath=(~/.zsh/completions $fpath)
autoload -Uz compinit
compinitto their ~/.zshrc. The step currently mentions this only as a comment ("or add to ~/.zshrc if not already present") but does not make it clear that without this step, completions will not load at all.
Recommend showing this as a required follow-up action (not a comment) in the activation instructions printed after install.
✅ Good points
- Default-yes prompt
[Y/n]is the right UX for completion install - Non-interactive fallback prints manual install hint — correct for CI
- Idempotent overwrite behavior is correct
- Per-shell target directory mapping is accurate for bash and fish
- Graceful skip when completions dir not found
…h requirement
- Replace require.resolve('mgw/package.json') with __dirname-based path resolution
that works in both installed and development contexts
- Clarify zsh fpath addition is required (not optional) for completions to work
- Add merge note comment for conflict resolution with PR #125
Summary
install_completionsstep tomgw:initthat detects the user's current shell (bash/zsh/fish) and offers to copy the appropriate completion script from the bundledcompletions/directory[Y/n]) and skips gracefully in non-interactive/CI contexts, printing a manual install hint insteadShell completionsstatus line showinginstalled (bash|zsh|fish),skipped, ornot availableCloses #124
Milestone Context
Changes
commands/init.md
descriptionfrontmatter to reflect completion install capabilityinstall_completionsstep (betweenensure_labelsandreport) covering: package dir resolution, shell detection, per-shell target path mapping, interactive prompt with default-yes, non-interactive fallback hint, and activation instructionsreportstep to includeShell completionsstatus linesuccess_criteriaentries for the completion install behaviourTest Plan
mgw:initin a new repo with mgw installed globally — verify prompt appears for detected shelln— verify report showsShell completions: skippedy(or Enter) — verify file copied to correct target dir and activation hint printedmgw:init < /dev/null) — verify prompt is skipped and manual install hint is shownnot available