From 35dd82426326311802667f49c90633931d4452a6 Mon Sep 17 00:00:00 2001 From: Copilot Date: Wed, 8 Apr 2026 19:23:13 +0300 Subject: [PATCH 1/3] fix: address post-merge review findings (#876, #900, #875) - Add YAML value escaping helper for skill metadata - Replace catch(err: any) with catch(err: unknown) + narrowing - Add type guards to replace unsafe type assertions - Standardize deprecation messages on `gh copilot` - Fix unsafe exports type cast in cross-package test Closes #924, #925, #926 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- packages/squad-cli/src/cli-entry.ts | 4 +- packages/squad-cli/src/cli/commands/plugin.ts | 5 +- packages/squad-cli/src/cli/commands/skill.ts | 54 +++++++++++++------ packages/squad-cli/src/cli/core/init.ts | 8 +-- test/cross-package-exports.test.ts | 19 +++++-- 5 files changed, 63 insertions(+), 27 deletions(-) diff --git a/packages/squad-cli/src/cli-entry.ts b/packages/squad-cli/src/cli-entry.ts index 345ddde95..4841d1877 100644 --- a/packages/squad-cli/src/cli-entry.ts +++ b/packages/squad-cli/src/cli-entry.ts @@ -719,7 +719,7 @@ async function main(): Promise { if (cmd === 'start') { console.log(`\n${YELLOW}⚠ DEPRECATED:${RESET} "squad start" is deprecated and will be removed in a future release.`); - console.log(` Use the GitHub Copilot CLI directly: ${BOLD}copilot${RESET} or ${BOLD}gh copilot${RESET}\n`); + console.log(` Use the GitHub Copilot CLI directly: ${BOLD}gh copilot${RESET}\n`); const { runStart } = await import('./cli/commands/start.js'); const hasTunnel = args.includes('--tunnel'); if (hasTunnel) { @@ -805,7 +805,7 @@ async function main(): Promise { if (cmd === 'rc' || cmd === 'remote-control') { console.log(`\n${YELLOW}⚠ DEPRECATED:${RESET} "squad rc" is deprecated and will be removed in a future release.`); - console.log(` Use the GitHub Copilot CLI directly: ${BOLD}copilot${RESET} or ${BOLD}gh copilot${RESET}\n`); + console.log(` Use the GitHub Copilot CLI directly: ${BOLD}gh copilot${RESET}\n`); const { runRC } = await import('./cli/commands/rc.js'); const hasTunnel = args.includes('--tunnel'); const portIdx = args.indexOf('--port'); diff --git a/packages/squad-cli/src/cli/commands/plugin.ts b/packages/squad-cli/src/cli/commands/plugin.ts index 691f8dd28..b51a7d26f 100644 --- a/packages/squad-cli/src/cli/commands/plugin.ts +++ b/packages/squad-cli/src/cli/commands/plugin.ts @@ -156,8 +156,9 @@ export async function runPlugin(dest: string, args: string[]): Promise { { timeout: TIMEOUTS.PLUGIN_FETCH_MS } ); entries = JSON.parse(stdout.trim()); - } catch (err: any) { - fatal(`Could not browse ${marketplace.source} — ${err.message}`); + } catch (err: unknown) { + const message = err instanceof Error ? err.message : String(err); + fatal(`Could not browse ${marketplace.source} — ${message}`); } if (!entries || entries.length === 0) { diff --git a/packages/squad-cli/src/cli/commands/skill.ts b/packages/squad-cli/src/cli/commands/skill.ts index cd527c0e5..1e7885fe3 100644 --- a/packages/squad-cli/src/cli/commands/skill.ts +++ b/packages/squad-cli/src/cli/commands/skill.ts @@ -42,6 +42,26 @@ export interface ApmManifest { // ── Helpers ─────────────────────────────────────────────────────────────────── +/** Wrap a YAML scalar in single quotes if it contains special characters. */ +export function escapeYamlValue(val: string): string { + if (/[:#'"\n\r]/.test(val)) { + return `'${val.replace(/'/g, "''")}'`; + } + return val; +} + +/** Type guard for objects with string `name` and `path` properties. */ +function isNamedPath(obj: unknown): obj is { name: string; path: string } { + return ( + typeof obj === 'object' && + obj !== null && + 'name' in obj && + 'path' in obj && + typeof (obj as Record).name === 'string' && + typeof (obj as Record).path === 'string' + ); +} + /** Parse `---\nkey: value\n---` front-matter from a Markdown file. */ function parseFrontMatter(content: string): Record { const match = content.match(/^---\r?\n([\s\S]*?)\r?\n---/); @@ -144,14 +164,14 @@ async function publish(dest: string, skillName?: string): Promise { // Build the skill's own apm.yml inside its directory const apmSkillPath = join(skillsDir, skillName, 'apm.yml'); const skillApm = [ - `name: ${fm['name'] ?? skillName}`, - `version: ${fm['version'] ?? '1.0.0'}`, - fm['description'] ? `description: "${fm['description']}"` : null, + `name: ${escapeYamlValue(fm['name'] ?? skillName)}`, + `version: ${escapeYamlValue(fm['version'] ?? '1.0.0')}`, + fm['description'] ? `description: ${escapeYamlValue(fm['description'])}` : null, ``, `skills:`, - ` - name: ${fm['name'] ?? skillName}`, + ` - name: ${escapeYamlValue(fm['name'] ?? skillName)}`, ` path: skill.md`, - fm['description'] ? ` description: "${fm['description']}"` : null, + fm['description'] ? ` description: ${escapeYamlValue(fm['description'])}` : null, ] .filter(l => l !== null) .join('\n'); @@ -186,17 +206,17 @@ async function publish(dest: string, skillName?: string): Promise { `# apm.yml — Agent Package Manager manifest`, `# See: https://github.com/microsoft/apm`, ``, - `name: ${existing.name ?? projectName}`, + `name: ${escapeYamlValue(existing.name ?? projectName)}`, `version: 1.0.0`, ``, `# Skills exported from ${relPrefix}/`, `skills:`, ...skills.map(s => [ - ` - name: ${s.name}`, - s.description ? ` description: "${s.description}"` : null, + ` - name: ${escapeYamlValue(s.name)}`, + s.description ? ` description: ${escapeYamlValue(s.description)}` : null, ` path: ${s.path}`, - s.version ? ` version: ${s.version}` : null, + s.version ? ` version: ${escapeYamlValue(s.version)}` : null, ] .filter(l => l !== null) .join('\n') @@ -281,8 +301,9 @@ async function installFromUrl(url: string, skillsDir: string, relPrefix: string) fatal(`Failed to fetch ${url}: ${res.status} ${res.statusText}`); } content = await res.text(); - } catch (err: any) { - fatal(`Failed to fetch ${url}: ${err.message}`); + } catch (err: unknown) { + const message = err instanceof Error ? err.message : String(err); + fatal(`Failed to fetch ${url}: ${message}`); return; } @@ -341,7 +362,7 @@ async function installFromGitHub( } if (inSkills && line.match(/^[a-z]/i) && !line.startsWith(' ')) { // New top-level key — exit skills section - if (currentSkill.name && currentSkill.path) skillPaths.push(currentSkill as { name: string; path: string }); + if (isNamedPath(currentSkill)) skillPaths.push(currentSkill); currentSkill = {}; inSkills = false; } @@ -349,13 +370,13 @@ async function installFromGitHub( const nameMatch = line.match(/^\s+- name:\s*(.+)$/); const pathMatch = line.match(/^\s+path:\s*(.+)$/); if (nameMatch) { - if (currentSkill.name && currentSkill.path) skillPaths.push(currentSkill as { name: string; path: string }); + if (isNamedPath(currentSkill)) skillPaths.push(currentSkill); currentSkill = { name: nameMatch[1]!.trim() }; } if (pathMatch) currentSkill.path = pathMatch[1]!.trim(); } } - if (currentSkill.name && currentSkill.path) skillPaths.push(currentSkill as { name: string; path: string }); + if (isNamedPath(currentSkill)) skillPaths.push(currentSkill); // Filter by skill name if specified const toInstall = skillFilter @@ -395,8 +416,9 @@ async function installFromGitHub( success(`Installed skill '${skill.name}'`); info(` ${DIM}Source: ${owner}/${repo}${skill.path}${RESET}`); installed++; - } catch (err: any) { - warn(`Failed to install '${skill.name}': ${err.message}`); + } catch (err: unknown) { + const message = err instanceof Error ? err.message : String(err); + warn(`Failed to install '${skill.name}': ${message}`); } } diff --git a/packages/squad-cli/src/cli/core/init.ts b/packages/squad-cli/src/cli/core/init.ts index 22ec44af3..708a1a2b6 100644 --- a/packages/squad-cli/src/cli/core/init.ts +++ b/packages/squad-cli/src/cli/core/init.ts @@ -13,6 +13,7 @@ import { fatal } from './errors.js'; import { detectProjectType } from './project-type.js'; import { getPackageVersion, stampVersion } from './version.js'; import { initSquad as sdkInitSquad, cleanupOrphanInitPrompt, ensurePersonalSquadDir, resolvePersonalSquadDir, type InitOptions } from '@bradygaster/squad-sdk'; +import { escapeYamlValue } from '../commands/skill.js'; const storage = new FSStorageProvider(); @@ -40,7 +41,7 @@ export function generateApmYml(dest: string, projectName: string): void { `# This file makes your Squad skills versioned, portable, and community-shareable.`, `# Run 'squad skill publish' to populate the skills section after adding skills.`, ``, - `name: ${projectName}`, + `name: ${escapeYamlValue(projectName)}`, `version: 1.0.0`, ``, `# Skills — add entries here or run 'squad skill publish' to auto-populate`, @@ -292,9 +293,10 @@ export async function runInit(dest: string, options: RunInitOptions = {}): Promi let result; try { result = await sdkInitSquad(initOptions); - } catch (err: any) { + } catch (err: unknown) { process.off('SIGINT', sigintHandler); - fatal(`Failed to initialize squad: ${err.message}`); + const message = err instanceof Error ? err.message : String(err); + fatal(`Failed to initialize squad: ${message}`); return; // Unreachable but makes TS happy } diff --git a/test/cross-package-exports.test.ts b/test/cross-package-exports.test.ts index 613aafe6a..081fd2579 100644 --- a/test/cross-package-exports.test.ts +++ b/test/cross-package-exports.test.ts @@ -15,6 +15,8 @@ * add a corresponding assertion here. The grep one-liner in the test * description shows how to audit. * + * grep -rn "from '@bradygaster/squad-sdk" packages/squad-cli/src/ + * * Related incident: v0.9.3-insider.1 shipped with FSStorageProvider missing * from the SDK barrel — broke users at runtime while tests passed locally. */ @@ -264,7 +266,13 @@ describe('cross-package exports — CLI → SDK', () => { const pkg = JSON.parse( fs.readFileSync(resolve(sdkRoot!, 'package.json'), 'utf8'), ); - const exportsMap = pkg.exports as Record>; + expect(pkg.exports, 'SDK package.json should have an exports field').toBeDefined(); + expect( + typeof pkg.exports === 'object' && pkg.exports !== null, + 'exports should be an object', + ).toBe(true); + + const exportsMap = pkg.exports as Record; const missing: string[] = []; for (const [subpath, targets] of Object.entries(exportsMap)) { @@ -274,9 +282,12 @@ describe('cross-package exports — CLI → SDK', () => { } continue; } - for (const [condition, file] of Object.entries(targets)) { - if (!existsSync(resolve(sdkRoot!, file))) { - missing.push(`${subpath}[${condition}] → ${file}`); + if (typeof targets === 'object' && targets !== null) { + for (const [condition, file] of Object.entries(targets as Record)) { + if (typeof file !== 'string') continue; + if (!existsSync(resolve(sdkRoot!, file))) { + missing.push(`${subpath}[${condition}] → ${file}`); + } } } } From 489c73ba188bd2bf2f11a6b9f713a10fb9b162c9 Mon Sep 17 00:00:00 2001 From: Copilot Date: Wed, 8 Apr 2026 19:24:58 +0300 Subject: [PATCH 2/3] chore: add changeset for review findings fix Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .changeset/review-findings-fix.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changeset/review-findings-fix.md diff --git a/.changeset/review-findings-fix.md b/.changeset/review-findings-fix.md new file mode 100644 index 000000000..3e50d8330 --- /dev/null +++ b/.changeset/review-findings-fix.md @@ -0,0 +1,6 @@ +--- +'@bradygaster/squad-cli': patch +--- + +fix: address post-merge review findings — YAML escaping, type safety, deprecation messages + From 4aa6c4ef1d6b908efb79d5c5ff553310289e0444 Mon Sep 17 00:00:00 2001 From: Copilot Date: Thu, 9 Apr 2026 07:53:55 +0300 Subject: [PATCH 3/3] chore: trigger CI Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .changeset/review-findings-fix.md | 1 - 1 file changed, 1 deletion(-) diff --git a/.changeset/review-findings-fix.md b/.changeset/review-findings-fix.md index 3e50d8330..e557bf007 100644 --- a/.changeset/review-findings-fix.md +++ b/.changeset/review-findings-fix.md @@ -3,4 +3,3 @@ --- fix: address post-merge review findings — YAML escaping, type safety, deprecation messages -