From a0b275bce6b9bd1d1a6a1b82c58a6e9418f18418 Mon Sep 17 00:00:00 2001 From: A Ibrahim Date: Fri, 21 Aug 2026 10:25:40 +0200 Subject: [PATCH] fix(cli): skip editors the skills installer rejects instead of failing init MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `init` hands every selected editor to the upstream `skills` installer in one call, and that tool validates all `-a` names before doing any work — so one name it doesn't know fails the step for every editor. Selecting Zed against a cached older installer produced `Invalid agents: zed` and no skills anywhere. - Pin the installer to `skills@latest`; npx otherwise reuses a cached release that predates newer agent names. - When it does reject an editor, warn about that one and retry with the rest. - Suppress the postinstall banner in children spawned to install or update the CLI (TIGRIS_NO_BANNER). It writes straight to /dev/tty, escaping captured stdio, and was painting a second banner over the init wizard. Assisted-by: Claude Opus 5 (1M context) via Claude Code --- .../cli-init-skip-unsupported-editors.md | 24 ++ packages/cli/postinstall.cjs | 6 + packages/cli/src/constants.ts | 7 + packages/cli/src/lib/init/index.ts | 7 + packages/cli/src/lib/init/interactive.ts | 99 ++++++-- packages/cli/src/lib/init/shared.ts | 75 +++++- packages/cli/src/lib/update.ts | 5 +- .../cli/test/lib/init/interactive.test.ts | 229 ++++++++++++++++++ packages/cli/test/lib/init/shared.test.ts | 134 ++++++++++ 9 files changed, 567 insertions(+), 19 deletions(-) create mode 100644 .changeset/cli-init-skip-unsupported-editors.md create mode 100644 packages/cli/test/lib/init/interactive.test.ts diff --git a/.changeset/cli-init-skip-unsupported-editors.md b/.changeset/cli-init-skip-unsupported-editors.md new file mode 100644 index 00000000..8c60ea17 --- /dev/null +++ b/.changeset/cli-init-skip-unsupported-editors.md @@ -0,0 +1,24 @@ +--- +'@tigrisdata/cli': patch +--- + +Fix `tigris init` failing the whole skills step over one unsupported editor, and +the install banner appearing twice + +Two things went wrong in a single `npx tigris@latest init` run: + +- **Skills installed for nobody.** `init` passes every selected editor to the + upstream `skills` installer in one call, and that tool validates all of its + `-a` names before doing any work — so one name it doesn't know fails the step + for every editor. A user with Zed selected got + `Invalid agents: zed` and no skills at all, in any editor. Two changes: the + installer is now pinned to `skills@latest`, since npx otherwise reuses a + cached release that predates the newer agent names; and when the installer + does reject an editor, `init` warns about that editor and installs for the + rest instead of giving up. +- **The banner printed mid-wizard.** The package's postinstall banner is written + straight to `/dev/tty`, so it escaped the captured stdio of the `npm install + -g` that `init` and `tigris update` run and was painted over the wizard's own + prompts. Children the CLI spawns to install or update itself now set + `TIGRIS_NO_BANNER`, which postinstall honours — the banner still greets a + first-time install. diff --git a/packages/cli/postinstall.cjs b/packages/cli/postinstall.cjs index 6b8c96b1..c508f674 100644 --- a/packages/cli/postinstall.cjs +++ b/packages/cli/postinstall.cjs @@ -21,6 +21,12 @@ try { } // --- Show banner --- +// Skipped when the CLI is installing or updating itself (`tigris init` and +// `tigris update` set TIGRIS_NO_BANNER — see src/constants.ts). The banner goes +// straight to /dev/tty, so it escapes the parent's captured stdio and would be +// painted over the `init` wizard, and "To get started" is wrong for an update. +if (process.env.TIGRIS_NO_BANNER === '1') process.exit(0); + try { const tty = openSync('/dev/tty', 'w'); diff --git a/packages/cli/src/constants.ts b/packages/cli/src/constants.ts index 24676687..af5fa9ec 100644 --- a/packages/cli/src/constants.ts +++ b/packages/cli/src/constants.ts @@ -6,6 +6,13 @@ export const NPM_REGISTRY_URL = export const UPDATE_CHECK_INTERVAL_MS = 6 * 60 * 60 * 1000; // Check for updates every 6 hours export const UPDATE_NOTIFY_INTERVAL_MS = 1 * 60 * 60 * 1000; // Show update notification every 1 hour +// Set to '1' for children the CLI spawns to install or update itself, so the +// package's postinstall banner ("To get started: tigris login") stays out of +// their output. It writes straight to /dev/tty, so it escapes captured stdio and +// would land in the middle of the `init` wizard. Read by postinstall.cjs, which +// is plain CJS outside the TS build and so repeats the literal. +export const NO_BANNER_ENV = 'TIGRIS_NO_BANNER'; + // Sentry DSN for CLI error telemetry, embedded at build time. A DSN is not a // secret (it only permits sending events), so shipping it in the published CLI // is expected. Overridable via TIGRIS_SENTRY_DSN. Empty keeps telemetry inert. diff --git a/packages/cli/src/lib/init/index.ts b/packages/cli/src/lib/init/index.ts index f73c276d..16444a4e 100644 --- a/packages/cli/src/lib/init/index.ts +++ b/packages/cli/src/lib/init/index.ts @@ -1,5 +1,6 @@ import { getOption } from '@utils/options.js'; +import { NO_BANNER_ENV } from '../../constants.js'; import { runInteractive } from './interactive.js'; import { buildAgentSetup } from './plan.js'; import { getInstalledCliVersion, withoutEphemeralBins } from './shared.js'; @@ -19,6 +20,12 @@ export default async function init(options: Record) { // a TTY, would print mid-wizard or pollute the --agent recipe on stdout. process.env.TIGRIS_NO_UPDATE_CHECK = '1'; + // Likewise for the package's postinstall banner, which the CLI install and + // update below would each trigger. It is written straight to /dev/tty, so it + // escapes the captured stdio of those children and lands on top of the + // wizard's own prompts. Inherited by every child from here on. + process.env[NO_BANNER_ENV] = '1'; + // Under `npx tigris init` this process *is* the CLI, reached through a bin // directory npx drops from PATH as soon as it exits. Strip those entries for // the whole command so no probe, update or handoff below can mistake that diff --git a/packages/cli/src/lib/init/interactive.ts b/packages/cli/src/lib/init/interactive.ts index add24459..1d4bb91d 100644 --- a/packages/cli/src/lib/init/interactive.ts +++ b/packages/cli/src/lib/init/interactive.ts @@ -17,6 +17,7 @@ import { SUPPORTED_EDITORS, skillsDirsFor, spawnOpts, + splitRejectedEditors, TIGRIS_SKILLS, upsertTomlServer, } from './shared.js'; @@ -118,25 +119,11 @@ export async function runInteractive() { } } - // 7. Install the chosen Tigris agent skills. Output is captured (the skills - // tool prints a big banner); we report the destination dirs ourselves. + // 7. Install the chosen Tigris agent skills. if (skillsLocation === 'skip' || skillIds.length === 0) { p.log.info('Agent skills: skipped'); } else { - const agents = editors.map((e) => e.skillsAgent); - const args = buildSkillsArgs(skillIds, agents, skillsLocation === 'global'); - const result = runCommand( - 'npx', - args, - `Installing ${skillIds.length} Tigris skill(s) (${skillsLocation})` - ); - if (result.ok) { - for (const dir of skillsDirsFor(editors, skillsLocation, cwd)) { - p.log.success(`Skills → ${prettyPath(dir, home)}`); - } - } else if (result.output) { - p.log.error(result.output.split('\n').slice(-6).join('\n')); - } + installSkills(editors, skillIds, skillsLocation, cwd, home); } // 8. Hand off to the agent — use the installed CLI, or npx if unavailable. @@ -238,6 +225,79 @@ function writeMcp( } } +/** + * Install the chosen skills for the chosen editors, via the upstream `skills` + * installer. Output is captured (the tool prints a big banner of its own); we + * report the destination dirs ourselves. + * + * The installer takes every editor in one call and validates all of the `-a` + * names up front, so a single name it doesn't recognise — an older release that + * predates one of the editors we support — means nobody gets skills. When that + * happens, drop the editors it named and install for the rest: an editor the + * installer can't reach is worth a warning, not a failed step. + * + * `run` is the one seam the tests need: everything else here is a decision about + * what to run next and what to report, and only the spawn has to be faked. + */ +export function installSkills( + editors: EditorInfo[], + skillIds: string[], + scope: 'global' | 'project', + cwd: string, + home: string, + run: RunCommand = runCommand +): void { + const attempt = (targets: EditorInfo[], startMsg: string) => + run( + 'npx', + buildSkillsArgs( + skillIds, + [...new Set(targets.map((e) => e.skillsAgent))], + scope === 'global' + ), + startMsg + ); + + let targets = editors; + let result = attempt( + targets, + `Installing ${skillIds.length} Tigris skill(s) (${scope})` + ); + + if (!result.ok) { + const { kept, dropped } = splitRejectedEditors( + targets, + result.output ?? '' + ); + if (dropped.length > 0) { + p.log.warn( + `Skills: installer has no support for ${labels(dropped)} — skipped.` + ); + if (kept.length === 0) { + p.log.info('Agent skills: skipped (no supported editor selected)'); + return; + } + targets = kept; + result = attempt( + targets, + `Installing ${skillIds.length} Tigris skill(s) for ${labels(targets)}` + ); + } + } + + if (result.ok) { + for (const dir of skillsDirsFor(targets, scope, cwd)) { + p.log.success(`Skills → ${prettyPath(dir, home)}`); + } + } else if (result.output) { + p.log.error(result.output.split('\n').slice(-6).join('\n')); + } +} + +function labels(editors: EditorInfo[]): string { + return editors.map((e) => e.label).join(', '); +} + /** Keep only string-valued fields (TOML upsert writes `k = "v"`). */ function stringFields(entry: Record): Record { const out: Record = {}; @@ -312,6 +372,13 @@ function installedCliCanHandOff(installed: string): boolean { return true; } +/** What `installSkills` needs of a command runner, so a test can stand in. */ +export type RunCommand = ( + cmd: string, + args: string[], + startMsg: string +) => { ok: boolean; output?: string }; + /** Run a command under a spinner; capture output and surface it on failure. */ function runCommand( cmd: string, diff --git a/packages/cli/src/lib/init/shared.ts b/packages/cli/src/lib/init/shared.ts index 3397d86c..3408180f 100644 --- a/packages/cli/src/lib/init/shared.ts +++ b/packages/cli/src/lib/init/shared.ts @@ -708,10 +708,18 @@ export const TIGRIS_SKILLS: SkillInfo[] = [ { id: 'tigris-python-sdk', label: 'Python SDK', recommended: false }, ]; +/** + * The upstream installer, pinned to `@latest`. Given a bare `skills`, npx reuses + * whatever version is already in its cache without consulting the registry — and + * a stale one rejects agent names added since (`Invalid agents: zed`), which + * fails the install for every editor, not just the unknown one. + */ +const SKILLS_PACKAGE = 'skills@latest'; + /** * Args for the `npx` skills installer — non-interactive: installs the chosen * skills to the given agents in one call. Run as `npx `, e.g. - * `npx -y skills add github.com/tigrisdata/skills --skill tigris-sdk-guide -a claude-code`. + * `npx -y skills@latest add github.com/tigrisdata/skills --skill tigris-sdk-guide -a claude-code`. * `global` adds `-g` (user directory) instead of the default project scope. */ export function buildSkillsArgs( @@ -721,10 +729,73 @@ export function buildSkillsArgs( ): string[] { // Leading `-y` is npx's auto-install; trailing `--yes` makes the skills tool // itself non-interactive (it prompts by default). - const args = ['-y', 'skills', 'add', TIGRIS_SKILLS_REPO]; + const args = ['-y', SKILLS_PACKAGE, 'add', TIGRIS_SKILLS_REPO]; for (const skill of skillIds) args.push('--skill', skill); if (global) args.push('-g'); for (const agent of skillsAgents) args.push('-a', agent); args.push('--yes'); return args; } + +/** + * Which of `agents` the skills installer refused, read off its failure output + * (`Invalid agents: zed`). It validates every `-a` name before doing any work, + * so one name it doesn't know — an installer predating an editor, or a name + * renamed upstream — installs nothing for anybody; init drops these and retries + * with the rest. + * + * Scoped to the `Invalid agents:` line, because the installer follows it with a + * `Valid agents:` list naming most of ours. Matched against the names we passed + * rather than by parsing the list, so an unrelated failure (no network, clone + * refused) drops nobody. + */ +export function rejectedAgents(output: string, agents: string[]): string[] { + const plain = output.replace(ANSI_ESCAPE, ''); + const start = plain.search(/Invalid agents?:/i); + if (start === -1) return []; + const lineEnd = plain.indexOf('\n', start); + let line = plain.slice(start, lineEnd === -1 ? undefined : lineEnd); + // Should the valid list ever share the line, stop before it. `\b` keeps this + // off the `Invalid agents:` header itself — "nv" is not a word boundary. + const validList = line.search(/\bvalid agents?:/i); + if (validList !== -1) line = line.slice(0, validList); + // Word-ish boundaries so `antigravity` can't match `antigravity-cli`. + return agents.filter((agent) => + new RegExp(`(? e.skillsAgent) + ); + const isRejected = (e: EditorInfo) => rejected.includes(e.skillsAgent); + return { + kept: editors.filter((e) => !isRejected(e)), + dropped: editors.filter(isRejected), + }; +} + +function escapeRegExp(s: string): string { + return s.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); +} + +/** + * SGR colour sequences, which the installer wraps its output in. They have to + * go before matching agent names: a sequence ends in a letter (`ESC[36m`), so + * `zed` in a coloured list would look like part of a longer word. Built from the + * escape's code point rather than written as a literal control character. + */ +const ANSI_ESCAPE = new RegExp(`${String.fromCharCode(27)}\\[[0-9;]*m`, 'g'); diff --git a/packages/cli/src/lib/update.ts b/packages/cli/src/lib/update.ts index 04211d60..535f4944 100644 --- a/packages/cli/src/lib/update.ts +++ b/packages/cli/src/lib/update.ts @@ -12,8 +12,8 @@ import { getUpdateCommand, isNewerVersion, } from '@utils/update-check.js'; - import { version as currentVersion } from '../../package.json'; +import { NO_BANNER_ENV } from '../constants.js'; const context = msg('update'); @@ -50,6 +50,9 @@ export default async function update( console.log('Updating...'); execSync(updateCommand, { stdio: 'inherit', + // The npm path re-runs our postinstall; its banner greets a first-time + // install and only repeats what this command already reports. + env: { ...process.env, [NO_BANNER_ENV]: '1' }, ...(process.platform === 'win32' ? { shell: 'powershell.exe' } : {}), }); printSuccess(context, { latestVersion }); diff --git a/packages/cli/test/lib/init/interactive.test.ts b/packages/cli/test/lib/init/interactive.test.ts new file mode 100644 index 00000000..b763474e --- /dev/null +++ b/packages/cli/test/lib/init/interactive.test.ts @@ -0,0 +1,229 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +/** + * Captures what `installSkills` reports, in place of clack's terminal output. + * Hoisted because the module factory below is lifted above the imports. + */ +const { logged } = vi.hoisted(() => ({ + logged: { + warn: [] as string[], + info: [] as string[], + success: [] as string[], + error: [] as string[], + }, +})); + +vi.mock('@clack/prompts', () => ({ + log: { + warn: (m: string) => logged.warn.push(m), + info: (m: string) => logged.info.push(m), + success: (m: string) => logged.success.push(m), + error: (m: string) => logged.error.push(m), + }, +})); + +import { installSkills } from '../../../src/lib/init/interactive.js'; +import { + type AgentTarget, + SUPPORTED_EDITORS, +} from '../../../src/lib/init/shared.js'; + +/** Verbatim shape of a real `skills add` rejection, for `zed` specifically. */ +const REJECTS_ZED = [ + '✖ Invalid agents: zed', + '➜ Valid agents: amp, antigravity, claude-code, cline, codex, cursor, roo', +].join('\n'); + +describe('installSkills', () => { + const CWD = '/repo'; + const HOME = '/home/tester'; + const SKILLS = ['tigris-sdk-guide', 'tigris-agent-kit']; + + const editors = (...ids: AgentTarget[]) => + SUPPORTED_EDITORS.filter((e) => ids.includes(e.id)); + + /** + * A stand-in runner that answers each call from `results` in order (and `ok` + * once they run out), recording what it was asked to run. + */ + function runner(...results: { ok: boolean; output?: string }[]) { + const calls: { cmd: string; args: string[]; startMsg: string }[] = []; + const run = (cmd: string, args: string[], startMsg: string) => { + calls.push({ cmd, args, startMsg }); + return results[calls.length - 1] ?? { ok: true }; + }; + return { calls, run }; + } + + /** The `-a ` names of one recorded invocation. */ + const agentsOf = (args: string[]) => + args.filter((_, i) => args[i - 1] === '-a'); + + beforeEach(() => { + for (const list of Object.values(logged)) list.length = 0; + }); + + it('installs once and reports every destination when nothing is rejected', () => { + const { calls, run } = runner({ ok: true }); + + installSkills( + editors('claude-code', 'zed'), + SKILLS, + 'project', + CWD, + HOME, + run + ); + + expect(calls).toHaveLength(1); + expect(calls[0].cmd).toBe('npx'); + expect(calls[0].args).toContain('skills@latest'); + expect(agentsOf(calls[0].args)).toEqual(['claude-code', 'zed']); + expect(logged.success).toEqual([ + 'Skills → /repo/.claude/skills', + 'Skills → /repo/.agents/skills', + ]); + expect(logged.warn).toEqual([]); + expect(logged.error).toEqual([]); + }); + + it('retries without the rejected editor and reports only what was installed', () => { + // The reported failure: an installer that predates Zed support must not cost + // Claude Code its skills. + const { calls, run } = runner( + { ok: false, output: REJECTS_ZED }, + { ok: true } + ); + + installSkills( + editors('claude-code', 'zed'), + SKILLS, + 'project', + CWD, + HOME, + run + ); + + expect(calls).toHaveLength(2); + expect(agentsOf(calls[0].args)).toEqual(['claude-code', 'zed']); + expect(agentsOf(calls[1].args)).toEqual(['claude-code']); + // Same skills, still non-interactive, on the retry. + expect(calls[1].args.filter((a) => a === '--skill')).toHaveLength(2); + expect(calls[1].args).toContain('--yes'); + expect(logged.warn).toEqual([ + 'Skills: installer has no support for Zed — skipped.', + ]); + // Zed's directory (.agents/skills) is not claimed as installed. + expect(logged.success).toEqual(['Skills → /repo/.claude/skills']); + expect(logged.error).toEqual([]); + }); + + it('skips the step without a second attempt when every editor is rejected', () => { + const { calls, run } = runner({ ok: false, output: REJECTS_ZED }); + + installSkills(editors('zed'), SKILLS, 'project', CWD, HOME, run); + + expect(calls).toHaveLength(1); + expect(logged.warn).toEqual([ + 'Skills: installer has no support for Zed — skipped.', + ]); + expect(logged.info).toEqual([ + 'Agent skills: skipped (no supported editor selected)', + ]); + expect(logged.success).toEqual([]); + // Nothing was installed, but nothing failed either — no error dump. + expect(logged.error).toEqual([]); + }); + + it('reports a failure with another cause as-is, dropping no editor', () => { + // Reading a network failure as an unsupported editor would silently install + // less than the user asked for. + const { calls, run } = runner({ + ok: false, + output: 'ERROR Failed to clone repository: network unreachable', + }); + + installSkills( + editors('claude-code', 'zed'), + SKILLS, + 'project', + CWD, + HOME, + run + ); + + expect(calls).toHaveLength(1); + expect(logged.warn).toEqual([]); + expect(logged.success).toEqual([]); + expect(logged.error).toEqual([ + 'ERROR Failed to clone repository: network unreachable', + ]); + }); + + it('surfaces a retry that fails for its own reason', () => { + const { calls, run } = runner( + { ok: false, output: REJECTS_ZED }, + { ok: false, output: 'ERROR EACCES: permission denied' } + ); + + installSkills( + editors('claude-code', 'zed'), + SKILLS, + 'project', + CWD, + HOME, + run + ); + + expect(calls).toHaveLength(2); + expect(logged.warn).toHaveLength(1); + expect(logged.success).toEqual([]); + expect(logged.error).toEqual(['ERROR EACCES: permission denied']); + }); + + it('trims a long failure to its last lines', () => { + const { run } = runner({ + ok: false, + output: Array.from({ length: 20 }, (_, i) => `line ${i + 1}`).join('\n'), + }); + + installSkills(editors('claude-code'), SKILLS, 'project', CWD, HOME, run); + + expect(logged.error).toEqual([ + ['line 15', 'line 16', 'line 17', 'line 18', 'line 19', 'line 20'].join( + '\n' + ), + ]); + }); + + it('asks for a user-level install at global scope', () => { + const { calls, run } = runner({ ok: true }); + + installSkills(editors('claude-code'), SKILLS, 'global', CWD, HOME, run); + + expect(calls[0].args).toContain('-g'); + // Global destinations sit outside the fake home, so they print in full. + expect(logged.success).toHaveLength(1); + expect(logged.success[0]).not.toContain('/repo'); + }); + + it('passes each agent once when editors share one', () => { + // Cursor, Codex and Zed all install through `.agents/skills`, but each has + // its own installer name; a repeated name would be a wasted `-a`. + const { calls, run } = runner({ ok: true }); + + installSkills( + editors('cursor', 'codex', 'zed'), + SKILLS, + 'project', + CWD, + HOME, + run + ); + + const agents = agentsOf(calls[0].args); + expect(agents).toEqual([...new Set(agents)]); + // One shared destination, reported once. + expect(logged.success).toEqual(['Skills → /repo/.agents/skills']); + }); +}); diff --git a/packages/cli/test/lib/init/shared.test.ts b/packages/cli/test/lib/init/shared.test.ts index 6bff5397..1f97eba8 100644 --- a/packages/cli/test/lib/init/shared.test.ts +++ b/packages/cli/test/lib/init/shared.test.ts @@ -2,7 +2,12 @@ import { delimiter } from 'node:path'; import { describe, expect, it } from 'vitest'; import { + type AgentTarget, + buildSkillsArgs, defaultsHint, + rejectedAgents, + SUPPORTED_EDITORS, + splitRejectedEditors, withoutEphemeralBins, } from '../../../src/lib/init/shared.js'; @@ -123,3 +128,132 @@ describe('withoutEphemeralBins', () => { expect(byPattern(undefined)).toBe(''); }); }); + +describe('buildSkillsArgs', () => { + it('pins the installer to @latest', () => { + // A bare `skills` lets npx reuse a cached release that may not know every + // agent name we pass, which fails the install for every editor at once. + expect( + buildSkillsArgs(['tigris-sdk-guide'], ['claude-code'], false) + ).toEqual([ + '-y', + 'skills@latest', + 'add', + 'github.com/tigrisdata/skills', + '--skill', + 'tigris-sdk-guide', + '-a', + 'claude-code', + '--yes', + ]); + }); + + it('adds -g for a global install and repeats each skill and agent', () => { + const args = buildSkillsArgs(['a', 'b'], ['claude-code', 'zed'], true); + expect(args.filter((a) => a === '--skill')).toHaveLength(2); + expect(args.filter((a) => a === '-a')).toHaveLength(2); + expect(args).toContain('-g'); + }); +}); + +describe('rejectedAgents', () => { + const OURS = ['claude-code', 'cursor', 'zed', 'antigravity-cli']; + + it("picks the agents named on the installer's invalid-agents line", () => { + // The real failure: an older `skills` release predating Zed support. + const output = [ + 'ERROR Invalid agents: zed', + 'Valid agents: amp, antigravity, claude-code, cline, codex, cursor, roo', + ].join('\n'); + expect(rejectedAgents(output, OURS)).toEqual(['zed']); + }); + + it('never harvests names from the valid-agents list', () => { + const output = [ + 'Invalid agents: zed', + 'Valid agents: claude-code, cursor, antigravity-cli', + ].join('\n'); + expect(rejectedAgents(output, OURS)).toEqual(['zed']); + }); + + it('stops at a valid-agents list that shares the line', () => { + const output = 'Invalid agents: zed. Valid agents: claude-code, cursor'; + expect(rejectedAgents(output, OURS)).toEqual(['zed']); + }); + + it('reads several rejected agents', () => { + const output = 'Invalid agents: zed, antigravity-cli\n'; + expect(rejectedAgents(output, OURS)).toEqual(['zed', 'antigravity-cli']); + }); + + it('reads the singular form', () => { + expect(rejectedAgents('Invalid agent: zed', OURS)).toEqual(['zed']); + }); + + it("sees through the installer's colour codes", () => { + // A colour sequence ends in a letter, so without stripping it the name + // beside it fails the whole-word check. + const esc = String.fromCharCode(27); + const output = `${esc}[31mInvalid agents:${esc}[39m ${esc}[36mzed${esc}[39m`; + expect(rejectedAgents(output, OURS)).toEqual(['zed']); + }); + + it('matches whole names, so a prefix of one is not the other', () => { + // `antigravity` (upstream's name) must not condemn our `antigravity-cli`. + expect(rejectedAgents('Invalid agents: antigravity', OURS)).toEqual([]); + expect( + rejectedAgents('Invalid agents: antigravity-cli', ['antigravity-cli']) + ).toEqual(['antigravity-cli']); + }); + + it('returns nothing for unrelated failures', () => { + // A network or clone failure must not be read as an unsupported editor — + // dropping editors then would silently install less than asked. + for (const output of [ + '', + 'ERROR fatal: could not read from remote repository', + 'Valid agents: claude-code, cursor, zed', + ]) { + expect(rejectedAgents(output, OURS)).toEqual([]); + } + }); +}); + +describe('splitRejectedEditors', () => { + const editors = (...ids: AgentTarget[]) => + SUPPORTED_EDITORS.filter((e) => ids.includes(e.id)); + const ids = (list: { id: AgentTarget }[]) => list.map((e) => e.id); + + // Verbatim from `skills add -a bogus-editor`; `zed` stands in for the agent an + // older release doesn't know yet. + const failure = [ + '✖ Invalid agents: zed', + '➜ Valid agents: amp, antigravity, antigravity-cli, claude-code, cline, codex, cursor, roo, windsurf, opencode', + ].join('\n'); + + it('keeps the editors the installer supports and drops the rest', () => { + const { kept, dropped } = splitRejectedEditors( + editors('claude-code', 'cursor', 'zed'), + failure + ); + expect(ids(kept)).toEqual(['claude-code', 'cursor']); + expect(ids(dropped)).toEqual(['zed']); + }); + + it('drops everything when no selected editor is supported', () => { + // The caller reports a skip rather than retrying with an empty agent list. + const { kept, dropped } = splitRejectedEditors(editors('zed'), failure); + expect(kept).toEqual([]); + expect(ids(dropped)).toEqual(['zed']); + }); + + it('keeps every editor when the failure has another cause', () => { + const selected = editors('claude-code', 'zed'); + const { kept, dropped } = splitRejectedEditors( + selected, + 'ERROR Failed to clone repository: network unreachable' + ); + expect(kept).toEqual(selected); + expect(dropped).toEqual([]); + }); +});