fix(cli): fix various issues found during bugbash

.claude/skills/architecture/SKILL.md

@@ -10,14 +10,14 @@ Structural rules, domain boundaries, and constraints for the Confidence CLI mono
 
 ## Monorepo Structure
 
-| Package                   | Published | Purpose                                                                                                  |
-| ------------------------- | --------- | -------------------------------------------------------------------------------------------------------- |
-| `packages/shared-kernel/` | No        | Cross-domain types and helpers (`noop`, `isDefined`). No runtime deps                                    |
-| `packages/eslint-config/` | No        | Shared ESLint config (base + react presets)                                                              |
-| `packages/core/`          | No        | Shared infrastructure (api, auth, config, session, telemetry, exec, system, sdk, mcp, frameworks, integrations, providers) |
-| `packages/testing/`       | No        | Test infrastructure (sub-paths: `/auth`, `/scaffold`, `/env`, `/terminal`, `/msw`, `/e2e`)               |
-| `packages/quickstart/`    | Yes       | TUI wizard — `@spotify-confidence/quickstart`                                                            |
-| `packages/cli/`           | Yes       | CLI — `@spotify-confidence/cli`                                                                          |
+| Package                   | Published | Purpose                                                                                                                      |
+| ------------------------- | --------- | ---------------------------------------------------------------------------------------------------------------------------- |
+| `packages/shared-kernel/` | No        | Cross-domain types and lightweight helpers (`noop`, `isDefined`, `isWindows`). Breaks circular deps between core and testing |
+| `packages/eslint-config/` | No        | Shared ESLint config (base + react presets)                                                                                  |
+| `packages/core/`          | No        | Shared infrastructure (api, auth, config, session, telemetry, exec, system, sdk, mcp, frameworks, integrations, providers)   |
+| `packages/testing/`       | No        | Test infrastructure (sub-paths: `/auth`, `/scaffold`, `/env`, `/terminal`, `/msw`, `/e2e`)                                   |
+| `packages/quickstart/`    | Yes       | TUI wizard — `@spotify-confidence/quickstart`                                                                                |
+| `packages/cli/`           | Yes       | CLI — `@spotify-confidence/cli`                                                                                              |
 
 ### Dependency Graph
 
@@ -39,15 +39,15 @@ import { buildTestJwt } from '@spotify-confidence/testing/auth';
 
 **Intra-package** — use path aliases for cross-domain, relative for within-domain:
 
-| Package    | Aliases                                                           |
-| ---------- | ----------------------------------------------------------------- |
-| quickstart | `@commands/*`, `@features/*`, `@ui/*`                                             |
+| Package    | Aliases                                                                                  |
+| ---------- | ---------------------------------------------------------------------------------------- |
+| quickstart | `@commands/*`, `@features/*`, `@ui/*`                                                    |
 | cli        | `@commands/*`, `@features/*`, `@input/*`, `@output/*`, `@network/*`, `@utils/*`, `@meta` |
-| core       | Relative imports in `src/`; tsconfig aliases in `__tests__/` only |
+| core       | Relative imports in `src/`; tsconfig aliases in `__tests__/` only                        |
 
 ## Domain Boundaries
 
-- **shared-kernel** — Types used by 3+ packages. Depends on nothing.
+- **shared-kernel** — Types and lightweight helpers shared by 3+ packages. Breaks circular dependencies between core and testing.
 - **core** — Shared infrastructure. Must not import from quickstart, cli, or testing.
 - **testing** — Test scaffolds. Depends on shared-kernel only.
 - **quickstart** — TUI wizard. See `ink-tui` skill for UI details.

AGENTS.md

@@ -8,7 +8,7 @@ pnpm workspace with six packages under `packages/`:
 
 | Package                   | Published                              | Purpose                                                                                                                                                                                                        |
 | ------------------------- | -------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
-| `packages/shared-kernel/` | No (private)                           | Cross-domain types (`AuthState`, `IdeId`, `OnboardingGoal`, etc.) and helpers (`noop`, `isDefined`). No runtime dependencies.                                                                                  |
+| `packages/shared-kernel/` | No (private)                           | Cross-domain types (`AuthState`, `IdeId`, `OnboardingGoal`, etc.) and lightweight helpers (`noop`, `isDefined`, `isWindows`). Breaks circular dependencies between core and testing.                           |
 | `packages/eslint-config/` | No (private)                           | Shared ESLint configuration. Exports base preset and `/react` preset with React Hooks rules.                                                                                                                   |
 | `packages/core/`          | No (private)                           | Shared infrastructure — api, auth, config, session, telemetry, exec, system, sdk, mcp, utils, constants, frameworks, integrations, providers. Depends on `shared-kernel`.                                      |
 | `packages/testing/`       | No (private)                           | Test infrastructure — auth scaffolds, project scaffolds, env helpers, terminal helpers, MSW handlers. Sub-path exports: `/auth`, `/scaffold`, `/env`, `/terminal`, `/msw`, `/e2e`. Depends on `shared-kernel`. |
@@ -18,7 +18,7 @@ pnpm workspace with six packages under `packages/`:
 ### Dependency graph
 
 ```
-shared-kernel (types-only leaf)
+shared-kernel (shared leaf)
     ▲
     ├── core (infrastructure)
     ├── testing (test scaffolds)

packages/cli/.prettierignore

@@ -1,2 +1,3 @@
 dist/
 node_modules/
+.claude/

packages/cli/__tests__/commands/flags.test.ts

@@ -282,55 +282,54 @@ describe('flags update', () => {
 });
 
 describe('flags toggle', () => {
-  it('enables a flag with --on', async () => {
+  it('enables a flag with --on via MCP', async () => {
     using _auth = prepareAuthTokens('valid');
     using output = captureOutput();
-    server.use(
-      http.patch(`${FLAGS_EU_BASE}/v1/flags/dark-mode`, () => {
-        return HttpResponse.json({ name: 'flags/dark-mode', flagId: 'dark-mode', enabled: true });
-      }),
-    );
+    mockMcpCallTool.mockResolvedValueOnce(textResult('Enabled flag dark-mode for client web-app'));
 
-    await run(['flags', 'toggle', 'dark-mode', '--on']);
+    await run(['flags', 'toggle', 'dark-mode', '--on', '--client', 'web-app']);
 
-    expect(output.stdout).toContain('Flag "dark-mode" enabled.');
+    expect(output.stdout).toContain('Enabled flag dark-mode');
+    expect(mockMcpCallTool).toHaveBeenCalledWith(
+      expect.anything(),
+      'addFlagToClient',
+      expect.objectContaining({ flagName: 'dark-mode', clientName: 'web-app' }),
+    );
   });
 
-  it('disables a flag with --off', async () => {
+  it('disables a flag with --off via MCP', async () => {
     using _auth = prepareAuthTokens('valid');
     using output = captureOutput();
-    server.use(
-      http.patch(`${FLAGS_EU_BASE}/v1/flags/dark-mode`, () => {
-        return HttpResponse.json({ name: 'flags/dark-mode', flagId: 'dark-mode', enabled: false });
-      }),
-    );
+    mockMcpCallTool.mockResolvedValueOnce(textResult('Disabled flag dark-mode for client web-app'));
 
-    await run(['flags', 'toggle', 'dark-mode', '--off']);
+    await run(['flags', 'toggle', 'dark-mode', '--off', '--client', 'web-app']);
 
-    expect(output.stdout).toContain('Flag "dark-mode" disabled.');
+    expect(output.stdout).toContain('Disabled flag dark-mode');
+    expect(mockMcpCallTool).toHaveBeenCalledWith(
+      expect.anything(),
+      'removeFlagFromClient',
+      expect.objectContaining({ flagName: 'dark-mode', clientName: 'web-app' }),
+    );
   });
 
   it('prints request body in dry-run mode', async () => {
     using _auth = prepareAuthTokens('valid');
     using output = captureOutput();
 
-    await run(['flags', 'toggle', 'dark-mode', '--on', '--dry-run']);
+    await run(['flags', 'toggle', 'dark-mode', '--on', '--client', 'web-app', '--dry-run']);
 
     const parsed = JSON.parse(output.stdout);
     expect(parsed.flagKey).toBe('dark-mode');
     expect(parsed.enabled).toBe(true);
+    expect(parsed.client).toBe('web-app');
   });
 
-  it('reports API errors', async () => {
+  it('reports MCP errors', async () => {
     using _auth = prepareAuthTokens('valid');
     using output = captureOutput();
-    server.use(
-      http.patch(`${FLAGS_EU_BASE}/v1/flags/dark-mode`, () => {
-        return HttpResponse.json({ code: 404, message: 'Flag not found' }, { status: 404 });
-      }),
-    );
+    mockMcpCallTool.mockRejectedValueOnce(new Error('Flag not found'));
 
-    await run(['flags', 'toggle', 'dark-mode', '--on']);
+    await run(['flags', 'toggle', 'dark-mode', '--on', '--client', 'web-app']);
 
     expect(output.stderr).toContain('Flag not found');
   });
@@ -352,14 +351,20 @@ describe('flags resolve', () => {
       'targeting_key',
       '--entity-value',
       'user-123',
+      '--client',
+      'web-app',
     ]);
 
     expect(output.stdout).toContain('dark-mode');
     expect(output.stdout).toContain('on');
     expect(mockMcpCallTool).toHaveBeenCalledWith(
       expect.anything(),
       'resolveFlag',
-      expect.objectContaining({ entity: 'targeting_key', entityValue: 'user-123' }),
+      expect.objectContaining({
+        clientName: 'web-app',
+        entity: 'targeting_key',
+        entityValue: 'user-123',
+      }),
     );
   });
 
@@ -376,6 +381,8 @@ describe('flags resolve', () => {
       'targeting_key',
       '--entity-value',
       'user-123',
+      '--client',
+      'web-app',
       '--context',
       'user=alice',
       '--context',
@@ -387,6 +394,7 @@ describe('flags resolve', () => {
       'resolveFlag',
       expect.objectContaining({
         flagName: 'dark-mode',
+        clientName: 'web-app',
         context: JSON.stringify({ user: 'alice', plan: 'premium' }),
       }),
     );
@@ -404,6 +412,8 @@ describe('flags resolve', () => {
       'targeting_key',
       '--entity-value',
       'u1',
+      '--client',
+      'web-app',
       '--context',
       'no-equals',
     ]);

packages/cli/__tests__/commands/migrate.test.ts

@@ -5,16 +5,21 @@ import { createMockIntegration } from '../helpers/mock-integration.js';
 import { createRunner } from '../helpers/run-command.js';
 
 const mockIntegration = createMockIntegration({
-  detectPlugin: vi.fn().mockResolvedValue('cli'),
+  detectPlugin: vi.fn().mockResolvedValue(true),
   detectMcpStatuses: vi.fn().mockResolvedValue({
     'confidence-flags': 'connected',
     'confidence-docs': 'connected',
   }),
 });
 
-const mockDetectProviders = vi
-  .fn()
-  .mockReturnValue([{ id: 'statsig', name: 'Statsig', skillName: 'migrate-statsig' }]);
+const mockHasSkills = vi.fn().mockReturnValue(true);
+const mockDetectProviders = vi.fn().mockReturnValue([
+  {
+    id: 'statsig',
+    name: 'Statsig',
+    skillName: 'migrate-statsig',
+  },
+]);
 
 const mockAuthenticate = vi.fn().mockResolvedValue({
   accessToken: buildTestJwt({ email: 'test@example.com' }),
@@ -27,6 +32,7 @@ vi.mock('@spotify-confidence/core', async (importOriginal) => {
     ...actual,
     getIntegration: () => mockIntegration,
     getIntegrations: () => [mockIntegration],
+    hasSkills: () => mockHasSkills(),
     detectProviders: (...args: unknown[]) => mockDetectProviders(...args),
     authenticate: (...args: unknown[]) => mockAuthenticate(...args),
   };
@@ -109,11 +115,11 @@ describe('migrate <provider>', () => {
   it('fails when plugin is not installed', async () => {
     using _auth = prepareAuthTokens('valid');
     using output = captureOutput();
-    vi.mocked(mockIntegration.detectPlugin).mockResolvedValueOnce(null);
+    mockHasSkills.mockReturnValueOnce(false);
 
     await run(['migrate', 'statsig', '--ide', 'claude']);
 
-    expect(output.stderr).toContain('Confidence AI plugin not installed');
+    expect(output.stderr).toContain('Confidence skills not installed');
     expect(output.stderr).toContain('confidence quickstart');
     expect(mockIntegration.launchChat).not.toHaveBeenCalled();
   });

packages/cli/__tests__/commands/warehouse-setup.test.ts

@@ -5,13 +5,14 @@ import { createMockIntegration } from '../helpers/mock-integration.js';
 import { createRunner } from '../helpers/run-command.js';
 
 const mockIntegration = createMockIntegration({
-  detectPlugin: vi.fn().mockResolvedValue('cli'),
+  detectPlugin: vi.fn().mockResolvedValue(true),
   detectMcpStatuses: vi.fn().mockResolvedValue({
     'confidence-flags': 'connected',
     'confidence-docs': 'connected',
   }),
 });
 
+const mockHasSkills = vi.fn().mockReturnValue(true);
 const mockAuthenticate = vi.fn().mockResolvedValue({
   accessToken: buildTestJwt({ email: 'test@example.com' }),
   region: 'EU',
@@ -23,6 +24,7 @@ vi.mock('@spotify-confidence/core', async (importOriginal) => {
     ...actual,
     getIntegration: () => mockIntegration,
     getIntegrations: () => [mockIntegration],
+    hasSkills: () => mockHasSkills(),
     authenticate: (...args: unknown[]) => mockAuthenticate(...args),
   };
 });
@@ -71,11 +73,11 @@ describe('warehouse setup', () => {
   it('fails when plugin is not installed', async () => {
     using _auth = prepareAuthTokens('valid');
     using output = captureOutput();
-    vi.mocked(mockIntegration.detectPlugin).mockResolvedValueOnce(null);
+    mockHasSkills.mockReturnValueOnce(false);
 
     await run(['warehouse', 'setup', '--warehouse-type', 'bigquery', '--ide', 'claude']);
 
-    expect(output.stderr).toContain('Confidence AI plugin not installed');
+    expect(output.stderr).toContain('Confidence skills not installed');
     expect(mockIntegration.launchChat).not.toHaveBeenCalled();
   });
 

packages/cli/__tests__/helpers/mock-integration.ts

@@ -7,8 +7,7 @@ export function createMockIntegration(overrides?: Partial<IdeIntegration>): IdeI
     launchChat: vi.fn(),
     runOnboarding: vi.fn().mockReturnValue(null),
     prepare: vi.fn().mockResolvedValue(undefined),
-    skillsDir: vi.fn().mockReturnValue('/project/.claude/skills'),
-    detectPlugin: vi.fn().mockResolvedValue(null),
+    detectPlugin: vi.fn().mockResolvedValue(false),
     installPlugin: vi.fn().mockResolvedValue(undefined),
     updatePlugin: vi.fn().mockResolvedValue(undefined),
     uninstallPlugin: vi.fn().mockResolvedValue(undefined),

packages/cli/src/commands/events.ts

@@ -76,7 +76,7 @@ export const eventsCommand = {
         (y: Argv) =>
           y
             .positional('name', { type: 'string', demandOption: true })
-            .option('days', { type: 'number', describe: 'Number of days (1-7)', default: 7 }),
+            .option('days', { type: 'number', describe: 'Number of days (1-7)', default: 1 }),
         safely(eventUsage),
       )
       .demandCommand(1)

packages/cli/src/commands/flags.ts

@@ -78,12 +78,17 @@ export const flagsCommand = {
       )
       .command(
         'toggle <flag-key>',
-        'Enable or disable a flag',
+        'Enable or disable a flag for a client',
         (y: Argv) =>
           y
             .positional('flag-key', { type: 'string', demandOption: true })
             .option('on', { type: 'boolean', describe: 'Enable the flag' })
             .option('off', { type: 'boolean', describe: 'Disable the flag' })
+            .option('client', {
+              type: 'string',
+              describe: 'Client name for the flag',
+              demandOption: true,
+            })
             .conflicts('on', 'off')
             .check((args) => {
               if (!args.on && !args.off) {
@@ -114,7 +119,11 @@ export const flagsCommand = {
               array: true,
               describe: 'Additional context key=value pair',
             })
-            .option('client', { type: 'string', describe: 'Client name for resolution' }),
+            .option('client', {
+              type: 'string',
+              describe: 'Client name for resolution',
+              demandOption: true,
+            }),
         safely(resolveFlagCmd),
       )
       .command(

packages/cli/src/features/events/create.ts

@@ -10,12 +10,13 @@ export const createEvent = withAuth(async function createEvent(argv, token) {
 
   const schema = resolveInput<JsonObject>(argv, () => {
     const fieldSpecs = (argv.field as string[] | undefined) ?? [];
-    if (fieldSpecs.length === 0) {
-      throw new Error('Provide at least one --field or --from-file.');
-    }
     return Object.fromEntries(fieldSpecs.map(parseFieldArg));
   });
 
+  if (Object.keys(schema).length === 0) {
+    throw new Error('Provide at least one --field or --from-file.');
+  }
+
   if (argv['dry-run']) {
     message(JSON.stringify({ eventDefinitionId, schema }, null, 2));
     return;

packages/cli/src/features/flags/resolve.ts

@@ -22,7 +22,7 @@ export const resolveFlagCmd = withAuth(async function resolveFlagCmd(argv, token
   const flagKey = argv['flag-key'] as string;
   const entity = argv.entity as string;
   const entityValue = argv['entity-value'] as string;
-  const client = argv.client as string | undefined;
+  const client = argv.client as string;
   const contextSpecs = (argv.context as string[] | undefined) ?? [];
 
   let context: Record<string, string> | undefined;

packages/cli/src/features/flags/toggle.ts

@@ -1,22 +1,17 @@
 import { toggleFlag } from '@network/index.js';
-import { fail, message } from '@output/index.js';
+import { message, printMcpResult } from '@output/index.js';
 import { withAuth } from '@utils/index.js';
 
 export const toggleFlagCmd = withAuth(async function toggleFlagCmd(argv, token) {
   const flagKey = argv['flag-key'] as string;
   const enabled = argv.on === true;
+  const client = argv.client as string;
 
   if (argv['dry-run']) {
-    message(JSON.stringify({ flagKey, enabled }, null, 2));
+    message(JSON.stringify({ flagKey, enabled, client }, null, 2));
     return;
   }
 
-  const result = await toggleFlag(token, flagKey, enabled);
-
-  if (!result.ok) {
-    fail(result.error.message);
-    return;
-  }
-
-  message(`Flag "${flagKey}" ${enabled ? 'enabled' : 'disabled'}.`);
+  const result = await toggleFlag(token, flagKey, { enabled, client });
+  printMcpResult(result, argv);
 });