fix(cli): fix zsh completions and eliminate Node.js/npm startup bottleneck

[lavaman131]

Summary

Fixes broken zsh shell completions and removes the ~48s Node.js/npm installation bottleneck from the startup auto-sync flow by migrating all global package management from npm to bun.

Key Changes

Bug Fixes

• zsh completions: Replace _atomic "$@" with compdef _atomic atomic in the generated completion script for correct zsh completion registration
• completions command: Skip auto-sync during atomic completions invocations so tab-completion does not trigger the setup flow

Performance

• Startup time: Remove the two-phase auto-sync (Phase 1: install Node.js/npm via fnm ~48s, Phase 2: npm-dependent installs) and collapse to a single parallel phase — bun is already available as the runtime
• All @playwright/cli, @llamaindex/liteparse, and skills installs now run concurrently on startup with no blocking Node.js download

Refactor

• src/lib/spawn.ts: Delete ensureNpmInstalled, upgradeNpm, and installNodeViaFnm (~210 lines removed); upgradeGlobalPackage now uses bun install -g --trust
• src/services/system/skills.ts: Use bunx (falling back to npx) for running the skills CLI tool
• Devcontainer features (claude, copilot, opencode v1.0.10 → v1.0.11): Drop ghcr.io/devcontainers/features/node:1 dependency; replace NVM/npm global install script with bun install -g --trust
• .devcontainer/devcontainer.json: Remove Node.js devcontainer feature

[claude]

Code Review

Solid hotfix — the gains here are real: cutting the ~48 s fnm/Node bootstrap, removing ~225 lines of platform-specific shell-out code, fixing a broken zsh completion, and skipping autoSyncIfStale() on completions (which shells invoke constantly) all reduce the perceived startup time meaningfully. The cleanup is well-motivated and the rationale comments are good.

A few things worth tightening before merge:

🐛 Bugs

1. Windows-broken endsWith(\"bunx\") check — src/services/system/skills.ts:30

const isBunx = runner.endsWith(\"bunx\");

Bun.which() returns the full resolved path. On Windows that's ...\\bunx.exe, so endsWith(\"bunx\") is false and the code will pass --yes skills ... to bunx, which doesn't accept --yes. Suggest tracking which lookup succeeded directly:

const bunxPath = Bun.which(\"bunx\");
const npxPath = bunxPath ? null : Bun.which(\"npx\");
const runner = bunxPath ?? npxPath;
if (!runner) return { ok: false, details: \"neither bunx nor npx found on PATH\" };
const cmd = bunxPath
  ? [runner, \"skills\", ...args]
  : [runner, \"--yes\", \"skills\", ...args];

2. argv[0] === \"completions\" is positionally fragile — src/cli.ts:331

This works for the canonical atomic completions zsh invocation, but misses cases like atomic --no-banner completions zsh (where argv[0] is --no-banner). Given how often shells invoke this for completion redraws, missing it once means a multi-second delay shows up at an unexpected moment. Consider argv.includes(\"completions\") — over-broad in theory, but safe in practice since users don't pass completions as a value to other flags.

⚠️ Concerns

3. Parallel bun install -g calls may race — src/services/system/auto-sync.ts:102-110

The old phasing kept npm-install steps in their own phase. The new code runs upgradePlaywrightCli and upgradeLiteparse concurrently — both write to the same ~/.bun/install/global/ directory and the same package.json. Bun's global install isn't documented as concurrency-safe; if this races on lockfile/manifest writes you'll get intermittent failures that are hard to reproduce. Worth either:

• Sequencing the two bun install -g calls (a single bun install -g --trust @playwright/cli@latest @llamaindex/liteparse@latest matches what install.sh does and is one process), or
• Adding a comment explaining you've verified bun handles this.

The install.sh files already batch them into one command — auto-sync should arguably do the same:

{ label: \"playwright + liteparse\", fn: () => upgradeGlobalPackages([\"@playwright/cli\", \"@llamaindex/liteparse\"]) }

4. Lost post-install verification — devcontainer-features/src/{claude,copilot,opencode}/install.sh

The old script verified command -v playwright and command -v lit after install. The new version drops those checks and only logs a generic warning. If the install silently puts binaries somewhere not on the user's PATH (a real risk with bun's global bin and the way /etc/profile.d/atomic-path.sh only adds ~/.bun/bin), users won't know until they try to use the tool. Cheap to keep:

su - \"\${REMOTE_USER}\" -c 'command -v playwright >/dev/null' && echo \"✓ playwright available\"
su - \"\${REMOTE_USER}\" -c 'command -v lit >/dev/null' && echo \"✓ liteparse (lit) available\"

5. --trust for @llamaindex/liteparse — minor

--trust is needed for @playwright/cli (browser download in postinstall) but @llamaindex/liteparse doesn't appear to have postinstall scripts. Granting trust to it is harmless but slightly broader than necessary. Not blocking.

✅ Things that look good

• The shared install.sh removal of the fnm/NVM dance and the explicit (non-fatal) semantics on failure are clean.
• zshCompletionScript change from _atomic \"$@\" to compdef _atomic atomic is the correct fix — sourcing it via eval previously executed the function with no args instead of registering it.
• Removing installNodeViaFnm, ensureNpmInstalled, upgradeNpm, and the npm install -g bun fallback in ensureBunInstalled is the right call now that nothing in the runtime path needs npm.
• Devcontainer feature versions correctly bumped 1.0.10 → 1.0.11 across all three packages.

🧪 Test coverage

No tests added/changed. src/lib/spawn.ts and src/services/system/auto-sync.ts have no existing tests, so this isn't a regression — but the bunx-vs-npx detection and the parallel-install race window above would both benefit from coverage. Even a unit test for runNpxSkills that asserts the right args are passed for each runner basename (especially the .exe case) would catch issue #1.

📝 Style nit

src/services/system/skills.ts still names the function runNpxSkills and types NpxSkillsResult even though it now prefers bunx. Renaming to runSkillsCli / SkillsCliResult would match the new semantics.

---

Overall direction is great — this is a meaningful UX win. The Windows endsWith bug is the only thing I'd consider blocking; everything else is polish.