Fix agents version stderr parsing

packages/cli/src/commands/agents.test.ts

@@ -1,11 +1,29 @@
 import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
 import { agentsCmd } from './agents.js';
+import { spawnSync } from 'node:child_process';
+
+vi.mock('node:child_process', () => ({
+  spawnSync: vi.fn(() => ({
+    status: 1,
+    stdout: '',
+    stderr: '',
+  })),
+}));
 
 describe('agents list --json', () => {
   let stdout: string[];
 
   beforeEach(() => {
     stdout = [];
+    vi.mocked(spawnSync).mockReset();
+    vi.mocked(spawnSync).mockReturnValue({
+      status: 1,
+      stdout: '',
+      stderr: '',
+      pid: 123,
+      output: [],
+      signal: null,
+    });
     vi.spyOn(console, 'log').mockImplementation((...args: unknown[]) => {
       stdout.push(args.map(String).join(' '));
     });
@@ -46,4 +64,25 @@ describe('agents list --json', () => {
       expect(typeof agent.setupCommand).toBe('string');
     }
   });
+
+  it('reads installed agent versions from stderr when stdout is empty', () => {
+    vi.mocked(spawnSync).mockImplementation((binary: string) => ({
+      status: 0,
+      stdout: '',
+      stderr: `${binary} 1.2.3\n`,
+      pid: 123,
+      output: [],
+      signal: null,
+    }));
+
+    agentsCmd.commands.find((c) => c.name() === 'list')!.parse(['--json'], { from: 'user' });
+    const output = stdout.join('\n');
+    const parsed = JSON.parse(output);
+
+    expect(parsed[0]).toMatchObject({
+      id: 'claude',
+      installed: true,
+      version: '1.2.3',
+    });
+  });
 });

packages/cli/src/commands/agents.ts

@@ -67,7 +67,7 @@ function probeAgent(desc: AgentDescriptor): AgentStatus {
     };
   }
 
-  const raw = (result.stdout ?? result.stderr ?? '').trim();
+  const raw = ((result.stdout?.trim() || result.stderr?.trim()) ?? '');
   // Extract semver-ish version from output like "claude 1.2.3" or "v1.2.3"
   const match = raw.match(/v?(\d+\.\d+(?:\.\d+)?(?:[-+][^\s]*)?)/);
   const version = match ? match[1] : raw.split('\n')[0]?.trim();

packages/targets/pkg-flatpak/src/index.test.ts

@@ -62,4 +62,28 @@ describe('Flatpak manifest generation', () => {
       appId: 'com.example.MyApp',
     })).resolves.toEqual({ id: 'dry-run' });
   });
+
+  it('rejects invalid app IDs before manifest generation', async () => {
+    const outDir = await mkdtemp(join(tmpdir(), 'sh1pt-flatpak-'));
+    tempDirs.push(outDir);
+    const ctx = fakeBuildContext({
+      outDir,
+      projectDir: '/repo/myapp',
+      version: '1.2.3',
+      channel: 'stable',
+    }) as any;
+
+    for (const appId of ['../escape', 'com.example', 'com.example.App-', 'com.123.App']) {
+      await expect(adapter.build(ctx, { appId })).rejects.toThrow('appId');
+    }
+  });
+
+  it('rejects invalid app IDs before shipping', async () => {
+    await expect(adapter.ship(fakeShipContext({
+      version: '1.2.3',
+      dryRun: true,
+    }) as any, {
+      appId: '../escape',
+    })).rejects.toThrow('appId');
+  });
 });

packages/targets/pkg-flatpak/src/index.ts

@@ -30,7 +30,7 @@ function renderList(values: string[], indent: string): string[] {
 /**
  * Validate a Flatpak application ID.
  * Must be a reverse-DNS string with at least 3 dot-separated segments,
- * each non-empty and containing only alphanumeric characters or hyphens/underscores.
+ * each starting with a letter and containing alphanumeric characters, hyphens, or underscores.
  * Must not contain path traversal characters.
  */
 function validateAppId(appId: string): void {
@@ -49,7 +49,7 @@ function validateAppId(appId: string): void {
     if (!seg) {
       throw new Error(`pkg-flatpak: invalid appId "${appId}" — segments must be non-empty`);
     }
-    if (!/^[A-Za-z0-9_-]+$/.test(seg)) {
+    if (!/^[A-Za-z][A-Za-z0-9_]*(?:-[A-Za-z0-9_]+)*$/.test(seg)) {
       throw new Error(`pkg-flatpak: invalid appId "${appId}" — segment "${seg}" contains invalid characters`);
     }
   }
@@ -107,26 +107,6 @@ function renderFlatpakManifest(ctx: { projectDir: string; version: string; chann
   return lines.join('\n');
 }
 
-/**
- * Validate a Flatpak app ID (reverse-DNS format): at least 3 dot-separated segments,
- * each containing alphanumeric or hyphens. e.g. "com.example.MyApp"
- */
-function validateAppId(appId: string): void {
-  if (!appId) throw new Error('pkg-flatpak: appId is required');
-  const segments = appId.split('.');
-  if (segments.length < 3) {
-    throw new Error(`pkg-flatpak: invalid appId "${appId}". Must have at least 3 reverse-DNS segments (e.g. "com.example.MyApp").`);
-  }
-  if (appId.includes('..') || appId.includes('/') || appId.includes('\\')) {
-    throw new Error(`pkg-flatpak: appId "${appId}" contains path traversal characters.`);
-  }
-  for (const seg of segments) {
-    if (!seg || !/^[A-Za-z0-9_-]+$/.test(seg)) {
-      throw new Error(`pkg-flatpak: invalid segment "${seg}" in appId "${appId}".`);
-    }
-  }
-}
-
 export default defineTarget<Config>({
   id: 'pkg-flatpak',
   kind: 'package-manager',

packages/targets/pkg-snap/src/index.test.ts

@@ -1,5 +1,5 @@
 import { fakeBuildContext, fakeShipContext, smokeTest } from '@profullstack/sh1pt-core/testing';
-import { mkdtemp, readFile, rm } from 'node:fs/promises';
+import { mkdir, mkdtemp, readFile, readdir, rm } from 'node:fs/promises';
 import { tmpdir } from 'node:os';
 import { join } from 'node:path';
 import { afterEach, describe, expect, it } from 'vitest';
@@ -60,4 +60,46 @@ describe('snapcraft manifest generation', () => {
       snapName: 'myapp',
     })).resolves.toEqual({ id: 'dry-run' });
   });
+
+  it('rejects invalid snap names before generating manifests', async () => {
+    const outDir = await mkdtemp(join(tmpdir(), 'sh1pt-snap-'));
+    tempDirs.push(outDir);
+
+    const ctx = fakeBuildContext({
+      outDir,
+      projectDir: '/repo/myapp',
+      version: '1.2.3',
+      channel: 'stable',
+    }) as any;
+
+    for (const snapName of ['Bad: Name', '-myapp', 'myapp-', 'my--app', '1234', 'a'.repeat(41)]) {
+      await expect(adapter.build(ctx, { snapName })).rejects.toThrow('snapName');
+    }
+  });
+
+  it('rejects invalid snap names before touching the output directory', async () => {
+    const outDir = await mkdtemp(join(tmpdir(), 'sh1pt-snap-'));
+    tempDirs.push(outDir);
+    await mkdir(outDir, { recursive: true });
+
+    await expect(adapter.build(fakeBuildContext({
+      outDir,
+      projectDir: '/repo/myapp',
+      version: '1.2.3',
+      channel: 'stable',
+    }) as any, {
+      snapName: 'my--app',
+    })).rejects.toThrow('snapName');
+
+    await expect(readdir(outDir)).resolves.toEqual([]);
+  });
+
+  it('rejects invalid snap names before shipping', async () => {
+    await expect(adapter.ship(fakeShipContext({
+      version: '1.2.3',
+      dryRun: true,
+    }) as any, {
+      snapName: 'Bad: Name',
+    })).rejects.toThrow('snapName');
+  });
 });

packages/targets/pkg-snap/src/index.ts

@@ -48,6 +48,9 @@ function validateSnapName(snapName: string): void {
   if (snapName.startsWith('-') || snapName.endsWith('-')) {
     throw new Error(`pkg-snap: snapName "${snapName}" must not start or end with a hyphen`);
   }
+  if (snapName.includes('--')) {
+    throw new Error(`pkg-snap: snapName "${snapName}" must not contain consecutive hyphens`);
+  }
   if (!/^[a-z0-9-]+$/.test(snapName)) {
     throw new Error(`pkg-snap: snapName "${snapName}" must contain only lowercase letters, digits, and hyphens`);
   }
@@ -57,6 +60,7 @@ function validateSnapName(snapName: string): void {
 }
 
 function renderSnapcraftYaml(ctx: { projectDir: string; version: string; channel: string }, config: Config): string {
+  validateSnapName(config.snapName);
   const grade = config.grade ?? (ctx.channel === 'stable' ? 'stable' : 'devel');
   const confinement = config.confinement ?? 'strict';
   const base = config.base ?? 'core22';
@@ -102,15 +106,6 @@ function renderSnapcraftYaml(ctx: { projectDir: string; version: string; channel
   return lines.join('\n');
 }
 
-/** Validate a snap package name: lowercase, alphanumeric, hyphens only (no leading/trailing hyphen). */
-function validateSnapName(name: string): void {
-  if (!name || !/^[a-z0-9]([a-z0-9-]*[a-z0-9])?$/.test(name)) {
-    throw new Error(
-      `pkg-snap: invalid snapName "${name}". Snap names must be lowercase alphanumeric with optional hyphens (no leading/trailing hyphen, no uppercase, no underscore).`,
-    );
-  }
-}
-
 export default defineTarget<Config>({
   id: 'pkg-snap',
   kind: 'package-manager',

packages/targets/pkg-winget/src/index.test.ts

@@ -82,4 +82,40 @@ describe('winget manifest generation', () => {
       ],
     })).resolves.toEqual({ id: 'dry-run' });
   });
+
+  it('rejects invalid package IDs before manifest generation', async () => {
+    const outDir = await mkdtemp(join(tmpdir(), 'sh1pt-winget-'));
+    tempDirs.push(outDir);
+    const ctx = fakeBuildContext({
+      outDir,
+      version: '1.2.3',
+    }) as any;
+    const installers = [
+      {
+        architecture: 'x64' as const,
+        url: 'https://downloads.example.com/my-tool-1.2.3-x64.exe',
+        sha256: 'c'.repeat(64),
+      },
+    ];
+
+    for (const packageId of ['NoDot', '.Acme.MyTool', 'Acme..MyTool', 'Acme/MyTool']) {
+      await expect(adapter.build(ctx, { packageId, installers })).rejects.toThrow('packageId');
+    }
+  });
+
+  it('rejects invalid package IDs before shipping', async () => {
+    await expect(adapter.ship(fakeShipContext({
+      version: '1.2.3',
+      dryRun: true,
+    }) as any, {
+      packageId: 'Acme/MyTool',
+      installers: [
+        {
+          architecture: 'x64',
+          url: 'https://downloads.example.com/my-tool-1.2.3-x64.exe',
+          sha256: 'c'.repeat(64),
+        },
+      ],
+    })).rejects.toThrow('packageId');
+  });
 });

packages/targets/pkg-winget/src/index.ts

@@ -124,28 +124,6 @@ function renderLocaleManifest(config: Config, version: string): string {
   return lines.join('\n');
 }
 
-/**
- * Validate a winget package identifier: must contain at least one dot,
- * each segment must be non-empty alphanumeric+hyphens/underscores/dots.
- * e.g. "Microsoft.WindowsTerminal"
- */
-function validatePackageId(packageId: string): void {
-  if (!packageId) throw new Error('pkg-winget: packageId is required');
-  const segments = packageId.split('.');
-  if (segments.length < 2) {
-    throw new Error(`pkg-winget: invalid packageId "${packageId}". Must be "Publisher.AppName" format with at least one dot.`);
-  }
-  for (const seg of segments) {
-    if (!seg) throw new Error(`pkg-winget: empty segment in packageId "${packageId}".`);
-    if (!/^[A-Za-z0-9_\-]+$/.test(seg)) {
-      throw new Error(`pkg-winget: invalid segment "${seg}" in packageId "${packageId}". Segments must be alphanumeric with hyphens or underscores.`);
-    }
-  }
-  if (packageId.includes('..') || packageId.includes('/') || packageId.includes('\\')) {
-    throw new Error(`pkg-winget: packageId "${packageId}" contains path traversal characters.`);
-  }
-}
-
 export default defineTarget<Config>({
   id: 'pkg-winget',
   kind: 'package-manager',