fix(cli): fix various issues found during bugbash by bespoyasov · Pull Request #112 · spotify/confidence-cli

bespoyasov · 2026-10-08T07:05:17Z

Summary

Skills downloaded to ~/.config/confidence/skills/ — decouples skill availability from plugin installation, fixing broken skill file resolution when the plugin registers as confidence@claude-plugins-official
Plugin install/update always downloads skills alongside the CLI plugin attempt, so prompts always have files to reference
Simplified plugin detection — detectPlugin() returns boolean, removed PluginInstallationMethod, InstalledPlugin, skillsDir from IdeIntegration, pluginInstallMethod from WizardSession
Moved getConfigDir() from auth/credentials to config/ module
referenceInstruction() / followInstruction() — onboarding uses skills as methodology references, CLI commands (migrate, warehouse) follow the skill instructions
Warehouse commands (confidence warehouse create|validate|setup) for managing data warehouse connections
Bug fixes: full plugin ID resolution for claude plugin update, .cmd shim resolution on Windows, git root lookup for MCP status, --from-file without --field for events, flag enablement schema compliance

Notes for reviewers

removeSkills / downloadSkills still take a skillsDir parameter — callers always pass getSkillsDir() now, so these could be simplified to use it internally. Left as-is to keep the diff focused; happy to clean up if you prefer.
detectPlugin on Cursor always returns false — Cursor doesn't support CLI plugin management yet, so its implementation is a no-op stub. Previously it checked for downloaded skills in .cursor/skills/, which was the only way it could return a truthy value.
Skills are downloaded on every install/update, even when the CLI plugin succeeds — this is intentional. The prompts reference the local files regardless of plugin status, so they must always exist. The download is cheap (12 small markdown files, skipped if already present).
PluginInstallationMethod removed entirely — the 'cli' | 'download' distinction no longer drives any behavior. detectPlugin() returning boolean is sufficient for the uninstall path. Telemetry for CLI failures now uses track() directly in plugin.ts instead of propagating the method to callers.
getConfigDir() moved from auth/credentials/paths.ts to config/paths.ts — it was imported by config, integrations, and MCP modules, none of which are auth-related. The auth module now imports it from config. Added @config path alias to core's tsconfig and vitest config.

The createFlag MCP call was sending variants without the required `value` field, causing the API to reject with "does not exist in schema". Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Prevents tests from crating real `mcp_preference` file

The MCP server's resolveFlag tool requires clientName, but the CLI treated it as optional, causing a server-side NPE when omitted. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

…ual-testing-bug-bash

The defaults callback in resolveInput threw before file data could be merged, so --from-file alone always failed. Move validation after resolveInput so the error only fires when the final schema is empty. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

resolveBin now returns shell: true when no .js shim is found on Windows, letting cmd.exe resolve npm-installed .cmd wrappers. Moved isWindows to shared-kernel so all packages share one constant. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

`claude plugin update` requires the full plugin ID (e.g. `confidence@claude-plugins-official`), unlike install/uninstall which accept the short name. Resolve the ID from `claude plugin list` first so the update succeeds via CLI instead of silently falling back to download. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

…absolute path Plugin skill file resolution broke when the CLI plugin registered as `confidence@claude-plugins-official` — command prompts inside the plugin couldn't resolve sibling skill files. Instead of depending on the plugin's internal resolution, skills are now always downloaded to `~/.config/confidence/skills/` and referenced via absolute path in prompts. Key changes: - installPlugin/updatePlugin always download skills to the shared dir, regardless of whether the CLI plugin install succeeds - referenceInstruction() produces an absolute file path; followInstruction() does the same for commands that should follow (not just reference) the skill - detectPlugin() returns boolean (is the CLI plugin installed?) - Removed PluginInstallationMethod, InstalledPlugin, skillsDir from IdeIntegration, and pluginInstallMethod from WizardSession - Moved getConfigDir() from auth to config module

filipmyllari

Review notes (AI-assisted), re-reviewed at the latest head. All findings are inline. I read all source changes in full and skimmed the tests; pnpm-lock.yaml was skipped. Nothing was tested on Windows.

filipmyllari · 2026-10-09T07:26:48Z

  }

-  return { command, args: [...args] };
+  return { command, args: [...args], shell: true };


Bug: On Windows, any command without a .js shim now returns shell: true. Node joins the args into a cmd.exe line without quoting them, so multi-line prompts passed to claude --append-system-prompt and to codex/cursor agent (in */chat.ts and */onboarding.ts) get split on spaces and newlines, and chat launch breaks on Windows. Fix: quote each arg for cmd, or run the resolved .cmd through cmd.exe /d /s /c with proper escaping.

Will remove shell and use cross-spawn instead.

filipmyllari · 2026-10-09T07:26:48Z

 ): ChildProcess {
  const resolved = resolveBin(command, args, { pathEnv: pathFromEnv(options?.env) });
-  return cpSpawn(resolved.command, resolved.args, options ?? {});
+  return cpSpawn(resolved.command, resolved.args, { ...options, shell: resolved.shell });


Security: This is the same shell: true path as the bug in resolve-bin.ts. On Windows, cmd metacharacters (&, |, >, ^) in arguments now run as shell syntax. Prompts include the project path and user-supplied userPrompt, so a directory named like foo&calc runs a command. The spread also silently overrides a caller's explicit shell option. Fix: don't use shell: true with untrusted args; escape them or call cmd.exe explicitly.

Will remove shell and use cross-spawn instead.

filipmyllari · 2026-10-09T07:26:48Z

-  }
+  await Promise.all([
+    integration.updatePlugin(projectDir, scope).catch(() => cliFailed('update', ide)),
+    downloadSkills(getSkillsDir()),


Bug: downloadSkills is called without force, so it returns early when SKILL.md already exists. That means confidence plugin update never refreshes skills that are already downloaded. Fix: pass true as before.

filipmyllari · 2026-10-09T07:26:48Z

-  if (method === 'download' || !scope || scope === 'project') {
-    await removeSkills(integration.skillsDir(projectDir));
-  }
+  await removeSkills(getSkillsDir());


Bug: The skills directory is now one global location under ~/.config/confidence shared by all IDEs. Uninstalling for one IDE (for example --ide cursor) still deletes all of it, which breaks migrate and warehouse setup for the other IDEs. scope is also ignored, so uninstalling at local scope wipes the global skills. Fix: only remove skills when no IDE still has the plugin, or stop removing them on per-IDE uninstall.

filipmyllari · 2026-10-09T07:26:48Z

+  await Promise.all([
+    integration.installPlugin(projectDir, scope).catch(() => cliFailed('install', ide)),
+    downloadSkills(getSkillsDir()),


Bug: CLI install errors are swallowed into telemetry, and downloadSkills skips any non-OK fetch silently. So install reports success even when the CLI failed and every download returned 404/500. Going the other way, one network error in fetch rejects the whole Promise.all even when the CLI install worked. Fix: fail only when both paths fail, and report which path succeeded.

filipmyllari · 2026-10-09T07:26:49Z


-  skills_dirs=("$PROJECT_DIR/.claude/skills" "$PROJECT_DIR/.cursor/skills" "$PROJECT_DIR/.agents/skills")
+  skills_dirs=("$config_dir/skills" "$PROJECT_DIR/.claude/skills" "$PROJECT_DIR/.cursor/skills" "$PROJECT_DIR/.agents/skills")
  confidence_skills=(


Minor: confidence_skills doesn't include explore-metric, onboard-confidence-dry-run or setup-session-recording, which are now in SKILL_NAMES, so the clean script leaves them behind.

filipmyllari · 2026-10-09T07:26:49Z

+    );
+}

 function readProjectMcpServers(projectDir: string): Record<string, McpServerEntry> {


Minor: reduce with an object spread on each iteration is O(n²) over projects in ~/.claude.json. Use Object.fromEntries. Also, resolveGitRoot (line 37) runs git synchronously on every detect, even when the direct key already matches. Compute it lazily.

filipmyllari · 2026-10-09T07:26:49Z

 }

+async function resolvePluginId(projectDir: string): Promise<string> {
+  const { stdout } = await execFile('claude', ['plugin', 'list', '--json'], { cwd: projectDir });


Minor: resolvePluginId takes the first confidence@* entry and ignores enabled and the project path. With several marketplaces or scopes, it can update the wrong entry.

…ution The shell: true fallback passed untrusted args (multi-line prompts, project paths) through cmd.exe, breaking on spaces/newlines and exposing cmd metacharacter injection. cross-spawn handles .cmd resolution and argument escaping without a shell. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

…pdate - downloadSkills uses Promise.allSettled and returns boolean so a single network error no longer rejects the whole install - install/update throw only when both CLI and download fail - updatePlugin passes force: true so existing skills get refreshed - uninstallPlugin only removes shared skills when no other IDE has the plugin installed Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

- Use Object.fromEntries instead of O(n²) reduce with spread - Defer resolveGitRoot until direct key lookups miss - resolvePluginId reuses isAvailable to check enabled and projectPath Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

bespoyasov and others added 11 commits October 8, 2026 09:04

fix(cli): include required value field in flag variant payload

fb6683e

The createFlag MCP call was sending variants without the required `value` field, causing the API to reject with "does not exist in schema". Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

fix(cli): comply with schema for flag enablement

a679e4a

fix(core): compute MCP preference location at runtime

ebd3fbc

Prevents tests from crating real `mcp_preference` file

fix(cli): make --client required for flags resolve command

592119f

The MCP server's resolveFlag tool requires clientName, but the CLI treated it as optional, causing a server-side NPE when omitted. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Merge branch 'main' of github.com:spotify/confidence-cli into fix/man…

020a087

…ual-testing-bug-bash

chore: update defaults for usage

9e5f640

fix: look up git root when detecting MCP status

746edc8

bespoyasov requested a review from filipmyllari October 8, 2026 16:57

bespoyasov marked this pull request as ready for review October 8, 2026 16:58

bespoyasov added 6 commits October 9, 2026 08:14

perf: parallelize CLI call and skills download

b8fb243

chore: mention plugin skills when launching chat session

5431394

test: update e2e

c0b979c

refactor: reduce duplication for mock integration

dc9304b

chore: add missing skills

7a17396

test: stub config dir

e304e56

filipmyllari reviewed Oct 9, 2026

View reviewed changes

bespoyasov and others added 5 commits October 9, 2026 09:48

test: fix spawn mocks

41bfcfd

chore: update clean script

db59177

bespoyasov requested a review from filipmyllari October 9, 2026 08:10

docs: update guidance about shared kernel

1ae504b

filipmyllari approved these changes Oct 9, 2026

View reviewed changes

bespoyasov merged commit 6b94c80 into main Oct 9, 2026
6 checks passed

bespoyasov deleted the fix/manual-testing-bug-bash branch October 9, 2026 08:36

github-actions Bot mentioned this pull request Oct 8, 2026

chore: release main #80

Merged

Navigation Menu

Repository navigation

fix(cli): fix various issues found during bugbash - #112

fix(cli): fix various issues found during bugbash#112
bespoyasov merged 23 commits into
mainfrom
fix/manual-testing-bug-bash

bespoyasov commented Oct 8, 2026 •

edited

Loading

Uh oh!

filipmyllari left a comment

Uh oh!

filipmyllari Oct 9, 2026

Uh oh!

bespoyasov Oct 9, 2026

Uh oh!

filipmyllari Oct 9, 2026

Uh oh!

bespoyasov Oct 9, 2026

Uh oh!

filipmyllari Oct 9, 2026

Uh oh!

filipmyllari Oct 9, 2026

Uh oh!

filipmyllari Oct 9, 2026

Uh oh!

Uh oh!

filipmyllari Oct 9, 2026

Uh oh!

filipmyllari Oct 9, 2026

Uh oh!

filipmyllari Oct 9, 2026

Uh oh!

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

bespoyasov commented Oct 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Notes for reviewers

Uh oh!

filipmyllari left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

bespoyasov commented Oct 8, 2026 •

edited

Loading