[mstomar125]

Problem

packages/skill was never built by any part of the build pipeline (dist/install.js, its declared bin target, never existed), and the desktop app invoked it via npx --yes @nodespaceai/skill install — an unpublished npm package, in a Bun-only repo. Every launch logged exit 127 and never installed SKILL.md/agent shims.

Fix

Build wiring (scripts/build-skill.ts, new build:skill script): runs packages/skill's own tsc build, then stages dist/, shims/, SKILL.md, and package.json into packages/desktop-app/src-tauri/resources/skill/, declared as a bundled Tauri resources entry in tauri.conf.json. Wired into:

• root build
• packages/desktop-app's dev:tauri and tauri:build (mirrors how sidecar binaries are staged before those same commands)
• scripts/test-gate.ts (the pre-push gate) — nodespace-app's build.rs (tauri_build::build()) hard-fails immediately if any declared resources/externalBin entry doesn't physically exist, so cargo test -p nodespace-app needs this staged first, same requirement the sidecar binaries already have.

Invocation (skill_setup.rs): resolve_installer_path resolves the installer via Tauri's BaseDirectory::Resource (falling back to the source-checkout path for cargo test/cargo run outside the Tauri resource pipeline), then run_skill_installer invokes it directly — Command::new("bun").arg(installer_path).arg("install") — never npx/npm.

Decision: bundled installer, not a published npm package. This is a local-first desktop app; startup should not depend on npm registry availability, and there's no @nodespaceai/skill version-skew to manage against the app version if it's never resolved externally. Bun is already the sanctioned runtime for this repo, so invoking the bundled dist/install.js directly via bun (not bunx, since it's a local file, not a registry package) fits the existing Bun-only policy rather than working around it.

Failure surfacing: setup.json now tracks skill_install_failed. A genuine failure logs at WARN and emits a skill:install-failed event to the frontend (surfaced via the existing statusBar.error() pattern in app-shell.svelte, mirroring skill:cli-missing) — but only the first time. Subsequent launches with the same persistent failure keep retrying (the environment may get fixed, e.g. bun gets installed) but log at DEBUG only, so a standing failure never becomes per-launch log/UI spam. A later success clears the flag.

Testing

• bun run build:skill — confirmed packages/skill/dist/install.js and the staged packages/desktop-app/src-tauri/resources/skill/dist/install.js both get produced.
• Ran the built installer directly against an isolated fake $HOME (never touching the real ~/.nodespace or ~/.claude): env HOME=<fake> bun dist/install.js install — correctly detected a fake .claude dir and installed SKILL.md + the claude-code shim into <fake>/.claude/skills/nodespace/. Verified identically for both the source-checkout dist/ and the staged bundle copy.
• cargo check -p nodespace-app / cargo clippy -p nodespace-app --all-targets -- -D warnings -D clippy::unused-async / cargo fmt --all -- --check — all clean.
• cargo test -p nodespace-app --lib — 69 passed (67 pre-existing + 2 new: resolve_installer_path resolves to the real built dist/install.js, and a regression guard asserting no Command::new invocation in this file ever shells out to npx/npm).
• Confirmed empirically that tauri_build::build() copies the new resources/skill/**/* entry into target/debug/resources/skill/dist/install.js at cargo build time — the exact path BaseDirectory::Resource resolves to in dev mode.
• bun run test:skill — 24/24 passed (untouched installer/shim-parity logic).
• bun run test (desktop-app frontend) — 4742/4743 passed; the one failure (architecture-benchmarks.test.ts, a perf-timing threshold) is pre-existing full-suite flakiness unrelated to this change — passes cleanly in isolation, and a second full run failed a different, also-unrelated test (core-plugins.test.ts), also passing in isolation.
• bunx eslint / svelte-check on desktop-app — 0 errors, 0 warnings.
• bun run --cwd packages/skill quality:fix (tsc --noEmit) — clean.
• bun run test:scripts — 93/93 passed.

Acceptance criteria

• packages/skill is built as part of producing a runnable app; dist/install.js exists
• Skill installation succeeds on first launch with no exit-127 warning
• Installer invocation does not shell out to npx/npm
• Decision recorded: bundled installer (see above)
• A genuine install failure surfaces once and actionably, not as a silent per-launch warning
• Verified on a fresh, isolated $HOME — SKILL.md and agent shims land in the expected locations

Closes #2113

[mstomar125]

Adversarial review requested (high effort). It found 4 real issues + 2 nits; addressed the real ones in d28c0b1e:

1. Concurrency: ~/.nodespace/setup.json's read-modify-write had no locking across the 3 call sites (startup, onboarding, manual retry) — added a process-wide tokio::sync::Mutex serializing install_skill's full body.
2. Double-toast: the emit was firing from all 3 call sites, but configure_skill/manual install_skill already return their result synchronously to an already-awaiting UI (Settings shows result.error inline) — a manual retry failure showed the error twice. Moved the emit to the one call site that actually needs it (the fire-and-forget startup call), gated on a new failure_is_new field.
3. Leaked build path: resolve_installer_path's "not found" error embedded the build machine's absolute path + a dev-only instruction — meaningless/leaky if ever shown to a packaged end user. Made it generic; detail still goes to a debug log.
4. Unhelpful bun-missing error: the most likely real-world failure (non-developer install without bun) surfaced a raw OS error. Now detects ErrorKind::NotFound specifically and points at https://bun.sh.

Also replaced the weak source-text self-scan test (gameable by non-literal command construction) with two behavioral tests against the actual installer_command() the production code calls: one inspects the constructed Command's real program via its Debug repr, the other actually runs it against an isolated $HOME and asserts SKILL.md + the shim land correctly.

Two remaining nits acknowledged, not fixed (low value / disproportionate effort for this issue's scope):

• bun not being bundled with the shipped app is a real, but architecturally out-of-scope, pre-release trade-off (see PR description's "bundled vs npm" reasoning) — now at least surfaced with an actionable message instead of a raw error.
• build-skill.ts does a full unconditional rebuild on every invocation (no change-detection) — matches the existing precedent of build-sidecar-binaries.ts/build-sidecars.ts, and the build itself is sub-second, so caching wasn't worth the added complexity here.

All checks re-verified after the fix: cargo test -p nodespace-app --lib (70/70), clippy/fmt clean, bun run test:coverage (4743/4743, 92.14% — matches main baseline), eslint/svelte-check clean.

[mstomar125]

Independent re-verification after the adversarial review's fixes were applied:

• cargo check/clippy/fmt clean on nodespace-app
• cargo test -p nodespace-app --lib: 70/70 (67 pre-existing + 3 new, including a real end-to-end test that runs the built installer against an isolated $HOME)
• bun run test:coverage: 4743/4743, 92.14% (matches main's baseline exactly)
• eslint/svelte-check: 0 errors, 0 warnings
• Full pre-push gate (test:all, cargo build --bin nodespaced, test:e2e, cargo test -p nodespace-app) passed on both pushed commits
• Diff is exactly the 11 intended files, no generated artifacts (resources/skill/, target/) committed
• Re-confirmed dist/install.js is produced by the wired build pipeline and lands identically in both the source checkout and the staged Tauri resource copy

GitHub blocks self-approving my own PR, so recording verification here instead. Merging per this repo's merge policy (core requires adversarial re-review before merge, which is done — see the review comment above).