From 3503e93b92b3d276a8a7bee9c6d7d4114739c097 Mon Sep 17 00:00:00 2001 From: kjgbot Date: Fri, 30 Oct 2026 17:10:06 -0700 Subject: [PATCH 01/11] feat(init): install the Agent Relay messaging skill for every injected tool MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A fresh session set up by relay init did not know how to find who is on Agent Relay or message them until a relayed message showed it a route, and it reached for the "Agent Relay in ChatGPT" connector instead. relay init now installs messaging-agents-on-the-relay from AgentWorkforce/skills, pinned to a commit, with the pinned skills CLI (`npx skills@1.7.4 add # --skill … --global --copy --yes --agent …`), for each tool the core injects into. It prompts for nothing and prints one line. A copy already in place is the person's and is left alone, and a failed install is a warning, not an init failure. What landed (each copy and its SKILL.md hash, the skills CLI lock entry, and the folders created) is recorded in core-settings.json. `relay init --uninstall` removes exactly those, and keeps a copy that changed since. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 1 + packages/cli/src/cli/commands/init.test.ts | 181 +++++++++++ packages/cli/src/cli/commands/init.ts | 56 +++- packages/cli/src/cli/lib/probe-core.ts | 5 + packages/cli/src/cli/lib/relay-skill.ts | 338 +++++++++++++++++++++ 5 files changed, 578 insertions(+), 3 deletions(-) create mode 100644 packages/cli/src/cli/lib/relay-skill.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 41ac1ede9..7fdfb2c84 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `relay init` lets two agents communicate without an account: it starts a private Relaycast server on this machine and creates a workspace. `relay init --hosted` creates a hosted workspace and prints the one command other machines run to join, and `relay init --url --key ` connects to your own server on any HTTPS port. See `docs/quickstart.md`. - `relay agent register` saves each agent's token, and `--as ` uses it on any messaging command, so agents no longer need `RELAY_AGENT_TOKEN` exported in every shell. - `relay init` is now a guided setup in a terminal: it asks where agents talk (this machine, hosted, or your own server), which agent tools to connect (Claude Code, Codex, OpenCode, Grok CLI, Pi and Devin, with the installed ones pre-checked), and whether to configure this project or every project, then adds the Relay MCP server to each tool through its own `mcp add` command or config file. `--tools`, `--scope` and `--yes` run it without prompts (the default when stdin is not a terminal), and `relay init --uninstall` removes only the entries it added. +- `relay init` installs the `messaging-agents-on-the-relay` agent skill globally for every tool it injects into (Claude Code, Codex, OpenCode, Pi, Grok, Devin), so a new session can list who is on Agent Relay and message them with no setup. It is pinned to a commit of AgentWorkforce/skills and installed with `npx skills` without prompting. A copy already there is left alone, and a failed install only prints a warning. `relay init --uninstall` removes only the copies init installed, unless they were changed since. ### Changed diff --git a/packages/cli/src/cli/commands/init.test.ts b/packages/cli/src/cli/commands/init.test.ts index ec7732756..a4b19aeb7 100644 --- a/packages/cli/src/cli/commands/init.test.ts +++ b/packages/cli/src/cli/commands/init.test.ts @@ -16,6 +16,12 @@ import { registerAgentCommands } from './agent.js'; import { DownloadHttpError, engineAssetName } from '../lib/relaycast-engine-install.js'; import { LOCAL_ENGINE_VERSION, registerInitCommand, type InitCommandDependencies } from './init.js'; import { PROBE_VERSION, type CoreClient } from '../lib/probe-core.js'; +import { + RELAY_SKILL_NAME, + RELAY_SKILL_REF, + SKILLS_CLI_PACKAGE, + type RunSkillsCli, +} from '../lib/relay-skill.js'; import { getProjectPaths } from '@agent-relay/config'; let home: string; @@ -109,6 +115,8 @@ function setup(overrides: Partial = {}) { throw new Error('offline'); }), probeVersion: vi.fn(() => undefined), + // Offline: the messaging skill cannot be fetched unless a test says so. + runSkillsCli: vi.fn(() => ({ status: 1, output: 'offline' })), ...overrides, }; const program = new Command().exitOverride(); @@ -1808,3 +1816,176 @@ describe('relay init message injection', () => { expect(fs.existsSync(path.join(project, '.mcp.json'))).toBe(false); }); }); + +/** + * The `skills` CLI as `add --global --copy` runs it: one copy per agent's + * global skills folder (Codex, OpenCode and Pi share ~/.agents/skills) and + * the skill's entry in its lock file. + */ +function fakeSkillsCli(status = 0) { + const dirs: Record = { + 'claude-code': path.join(toolHome, '.claude', 'skills'), + codex: path.join(toolHome, '.agents', 'skills'), + opencode: path.join(toolHome, '.agents', 'skills'), + pi: path.join(toolHome, '.agents', 'skills'), + grok: path.join(toolHome, '.grok', 'skills'), + devin: path.join(toolHome, '.config', 'devin', 'skills'), + }; + const lock = path.join(toolHome, '.agents', '.skill-lock.json'); + const run = vi.fn((args) => { + if (status !== 0) return { status, output: 'fatal: unable to access github.com' }; + args.forEach((arg, i) => { + if (args[i - 1] !== '--agent') return; + const dir = path.join(dirs[arg]!, RELAY_SKILL_NAME); + fs.mkdirSync(dir, { recursive: true }); + fs.writeFileSync(path.join(dir, 'SKILL.md'), '---\nname: messaging-agents-on-the-relay\n---\n'); + }); + const current = fs.existsSync(lock) + ? JSON.parse(fs.readFileSync(lock, 'utf8')) + : { version: 3, skills: {} }; + current.skills[RELAY_SKILL_NAME] = { source: 'AgentWorkforce/skills', ref: RELAY_SKILL_REF }; + fs.writeFileSync(lock, JSON.stringify(current)); + return { status: 0, output: 'Done!' }; + }); + return { run, lock, dirs }; +} + +/** Every file and folder under the tools' home, relative to it. */ +function toolHomeTree(): string[] { + const out: string[] = []; + const walk = (dir: string) => { + for (const entry of fs.readdirSync(dir, { withFileTypes: true })) { + const full = path.join(dir, entry.name); + out.push(path.relative(toolHome, full)); + if (entry.isDirectory()) walk(full); + } + }; + walk(toolHome); + return out.sort(); +} + +describe('relay init messaging skill', () => { + it('installs the pinned skill globally for each injected tool, records it, and --uninstall removes exactly that', async () => { + installedProbe(); + const core = fakeCore(); + const skills = fakeSkillsCli(); + const { run, logs } = setup({ ...core.deps, runSkillsCli: skills.run }); + await run('--tools', 'claude,codex'); + + expect(skills.run).toHaveBeenCalledTimes(1); + const [args, options] = skills.run.mock.calls[0]!; + // Pinned to a commit of AgentWorkforce/skills, never a moving branch. + expect(args).toEqual([ + 'add', + `https://github.com/AgentWorkforce/skills#${RELAY_SKILL_REF}`, + '--skill', + RELAY_SKILL_NAME, + '--global', + '--copy', + '--yes', + '--agent', + 'claude-code', + '--agent', + 'codex', + ]); + expect(RELAY_SKILL_REF).toMatch(/^[0-9a-f]{40}$/); + expect(SKILLS_CLI_PACKAGE).toMatch(/^skills@\d+\.\d+\.\d+$/); + expect(options.env).toMatchObject({ HOME: toolHome, DISABLE_TELEMETRY: '1', DO_NOT_TRACK: '1' }); + expect(logs.join('\n')).toContain( + `✓ Agent skill installed ${RELAY_SKILL_NAME} for Claude Code, Codex` + ); + const record = JSON.parse(fs.readFileSync(path.join(home, 'local', 'core-settings.json'), 'utf8')); + expect(record.relaySkill).toMatchObject({ + ref: RELAY_SKILL_REF, + copies: [ + { path: path.join(skills.dirs['claude-code']!, RELAY_SKILL_NAME) }, + { path: path.join(skills.dirs.codex!, RELAY_SKILL_NAME) }, + ], + lock: { file: skills.lock, created: true }, + }); + + // A re-run with the same pin changes nothing and fetches nothing. + const again = setup({ ...core.deps, runSkillsCli: skills.run }); + await again.run('--tools', 'claude,codex'); + expect(skills.run).toHaveBeenCalledTimes(1); + expect(again.logs.join('\n')).toContain(`installed for Claude Code, Codex`); + + const off = setup(core.deps); + await off.run('--uninstall'); + expect(off.logs.join('\n')).toContain(`removed the ${RELAY_SKILL_NAME} skill relay init installed`); + // The skill, its lock entry and the folders made for them are gone; only the core's own files remain. + expect(toolHomeTree().filter((entry) => entry.includes('skills') || entry.startsWith('.agents'))).toEqual( + [] + ); + expect(fs.existsSync(path.join(home, 'local', 'core-settings.json'))).toBe(false); + }); + + it('leaves a copy that was there before init, and installs only where there is none', async () => { + installedProbe(); + const own = path.join(toolHome, '.claude', 'skills', RELAY_SKILL_NAME, 'SKILL.md'); + fs.mkdirSync(path.dirname(own), { recursive: true }); + fs.writeFileSync(own, 'my own copy\n'); + const lock = path.join(toolHome, '.agents', '.skill-lock.json'); + fs.mkdirSync(path.dirname(lock), { recursive: true }); + fs.writeFileSync(lock, JSON.stringify({ version: 3, skills: { 'other-skill': { ref: 'x' } } })); + const core = fakeCore(); + const skills = fakeSkillsCli(); + const { run, logs } = setup({ ...core.deps, runSkillsCli: skills.run }); + await run('--tools', 'claude,codex'); + + const args = skills.run.mock.calls[0]![0]; + expect(args.filter((_, i) => args[i - 1] === '--agent')).toEqual(['codex']); + expect(logs.join('\n')).toContain( + 'installed messaging-agents-on-the-relay for Codex; already there for Claude Code, left as is' + ); + + await setup(core.deps).run('--uninstall'); + expect(fs.readFileSync(own, 'utf8')).toBe('my own copy\n'); + expect(fs.existsSync(path.join(toolHome, '.agents', 'skills'))).toBe(false); + // The lock file was the person's: only init's entry leaves it. + expect(JSON.parse(fs.readFileSync(lock, 'utf8')).skills).toEqual({ 'other-skill': { ref: 'x' } }); + }); + + it('keeps a copy changed after init on --uninstall', async () => { + installedProbe(); + const core = fakeCore(); + const skills = fakeSkillsCli(); + await setup({ ...core.deps, runSkillsCli: skills.run }).run('--tools', 'codex'); + const copy = path.join(skills.dirs.codex!, RELAY_SKILL_NAME, 'SKILL.md'); + fs.writeFileSync(copy, 'edited\n'); + const { run, logs } = setup(core.deps); + await run('--uninstall'); + expect(fs.readFileSync(copy, 'utf8')).toBe('edited\n'); + expect(logs.join('\n')).toContain('changed since'); + }); + + it('warns and finishes init when the skill cannot be installed, recording nothing', async () => { + installedProbe(); + const core = fakeCore(); + const skills = fakeSkillsCli(1); + const { run, logs } = setup({ ...core.deps, runSkillsCli: skills.run }); + await expect(run('--tools', 'codex')).resolves.toBeDefined(); + const output = logs.join('\n'); + expect(output).toContain( + '! Agent skill could not install messaging-agents-on-the-relay for Codex (fatal: unable to access github.com)' + ); + expect(output).toContain('Each session joins Relay on its own'); + const record = JSON.parse(fs.readFileSync(path.join(home, 'local', 'core-settings.json'), 'utf8')); + expect(record.relaySkill).toBeUndefined(); + }); + + it('installs nothing for tools on the Relay MCP server, which has its own tools', async () => { + const skills = fakeSkillsCli(); + await setup({ runSkillsCli: skills.run }).run('--tools', 'claude'); + expect(skills.run).not.toHaveBeenCalled(); + }); + + it('keeps the skill on --uninstall --scope project, like injection', async () => { + installedProbe(); + const core = fakeCore(); + const skills = fakeSkillsCli(); + await setup({ ...core.deps, runSkillsCli: skills.run }).run('--tools', 'codex'); + await setup(core.deps).run('--uninstall', '--scope', 'project'); + expect(fs.existsSync(path.join(skills.dirs.codex!, RELAY_SKILL_NAME, 'SKILL.md'))).toBe(true); + }); +}); diff --git a/packages/cli/src/cli/commands/init.ts b/packages/cli/src/cli/commands/init.ts index e636df625..7fcfe9ff8 100644 --- a/packages/cli/src/cli/commands/init.ts +++ b/packages/cli/src/cli/commands/init.ts @@ -87,6 +87,12 @@ import { type OpenCodeState, type PiState, } from '../lib/init-leftovers.js'; +import { + defaultRunSkillsCli, + installRelaySkill, + uninstallRelaySkill, + type RunSkillsCli, +} from '../lib/relay-skill.js'; import { persistWorkspaceSession } from '../lib/workspace-session.js'; import { readWorkspaceStore } from '../lib/workspace-store.js'; import { getProjectPaths } from '@agent-relay/config'; @@ -144,6 +150,8 @@ export interface InitCommandDependencies extends SdkCommandDeps { downloadProbe: DownloadFile; /** The version an installed `agent-relay-probe` reports for `--version`. */ probeVersion: (binary: string) => string | undefined; + /** Runs the pinned `skills` CLI that installs the Agent Relay messaging skill. */ + runSkillsCli: RunSkillsCli; } interface LocalServerState { @@ -316,6 +324,7 @@ export function withInitDefaults(overrides: Partial = { startCore: startCoreProcess, downloadProbe: fetchDownload, probeVersion: readProbeVersion, + runSkillsCli: defaultRunSkillsCli, ...overrides, }; } @@ -1026,6 +1035,7 @@ async function setupInjection( ? (current.openCode ?? files.openCode) : current.openCode, pi: touched.includes('pi_extension') ? (current.pi ?? files.pi) : current.pi, + relaySkill: current.relaySkill, }); }); try { @@ -1345,6 +1355,7 @@ function tidyLeftovers(record: DeliveryRecord): DeliveryRecord { ...(claudeFiles ? { claudeFiles } : {}), ...(openCode ? { openCode } : {}), ...(pi ? { pi } : {}), + ...(record.relaySkill ? { relaySkill: record.relaySkill } : {}), }; } @@ -1358,7 +1369,7 @@ function updateDeliveryRecord(change: (current: DeliveryRecord) => DeliveryRecor /** Save what init turned on; no record at all once nothing is. */ function writeDeliveryRecord(record: DeliveryRecord): void { - if (record.turnedOn.length === 0) { + if (record.turnedOn.length === 0 && !record.relaySkill) { fs.rmSync(deliveryRecordPath(), { force: true }); return; } @@ -1368,9 +1379,37 @@ function writeDeliveryRecord(record: DeliveryRecord): void { ...(record.claudeFiles ? { claudeFiles: record.claudeFiles } : {}), ...(record.openCode ? { openCode: record.openCode } : {}), ...(record.pi ? { pi: record.pi } : {}), + ...(record.relaySkill ? { relaySkill: record.relaySkill } : {}), }); } +/** + * Install the Agent Relay messaging skill for the tools the core injects + * into, with no prompt, and record what landed. Never fails init. + * @returns The one line to print, or undefined when no tool is injected + */ +function installSkill( + deps: InitCommandDependencies, + injected: ToolId[], + ctx: ToolContext +): string | undefined { + if (injected.length === 0) return undefined; + const { line, record } = installRelaySkill( + injected, + ctx, + readDeliveryRecord(deliveryRecordPath()).relaySkill, + deps.runSkillsCli + ); + try { + updateDeliveryRecord(({ relaySkill: _previous, ...rest }) => + record ? { ...rest, relaySkill: record } : rest + ); + } catch (error) { + return `${line} (not recorded for --uninstall: ${(error as Error).message})`; + } + return line; +} + /** * The recorded settings to turn off when `tools` leave injection: theirs, * and automatic activation once no tool init injects into is left. @@ -1531,7 +1570,15 @@ async function uninstall(deps: InitCommandDependencies, o: InitOptions): Promise '--scope project or --tools to turn it off.' ); } - if (shown.length === 0 && turnedOff.length === 0 && !injectionKept) { + // The skill is installed per user, like injection, and removed with it. + const skillRecord = readDeliveryRecord(deliveryRecordPath()).relaySkill; + const skillLine = + o.scope !== 'project' && o.tools === undefined ? uninstallRelaySkill(skillRecord) : undefined; + if (skillLine) { + deps.log(skillLine); + updateDeliveryRecord(({ relaySkill: _removed, ...rest }) => rest); + } + if (shown.length === 0 && turnedOff.length === 0 && !injectionKept && !skillRecord) { deps.log('relay init had not added Relay to any agent tool here; nothing to remove.'); return; } @@ -1622,6 +1669,8 @@ async function runPlain(deps: InitCommandDependencies, o: InitOptions): Promise< deps.log(''); deps.log('Injecting messages into your agent sessions:'); for (const line of injectionLines(plan, ctx)) deps.log(` ${line}`); + const skillLine = installSkill(deps, plan.injected, ctx); + if (skillLine) deps.log(` ${skillLine}`); } else if (plan.unavailable) { deps.log(''); deps.log(unavailableLine(plan.unavailable)); @@ -1819,7 +1868,8 @@ async function runGuided(deps: InitCommandDependencies, o: InitOptions): Promise ); if (plan.injection) { injecting.stop('Message injection is set up.'); - p.note(injectionLines(plan, ctx).join('\n'), 'Message injection'); + const skillLine = installSkill(deps, plan.injected, ctx); + p.note([...injectionLines(plan, ctx), ...(skillLine ? [skillLine] : [])].join('\n'), 'Message injection'); } else if (plan.unavailable) { injecting.stop('Message injection is not available.'); p.warn(unavailableLine(plan.unavailable)); diff --git a/packages/cli/src/cli/lib/probe-core.ts b/packages/cli/src/cli/lib/probe-core.ts index 7c0199dae..027311570 100644 --- a/packages/cli/src/cli/lib/probe-core.ts +++ b/packages/cli/src/cli/lib/probe-core.ts @@ -23,6 +23,7 @@ import path from 'node:path'; import type { ToolId } from './agent-tools-mcp.js'; import type { ClaudeFileState, OpenCodeState, PiState } from './init-leftovers.js'; import type { EnginePlatform } from './relaycast-engine-install.js'; +import { relaySkillRecordOf, type RelaySkillRecord } from './relay-skill.js'; /** The core `relay init` installs. Pinned so every machine runs the same core. */ export const PROBE_VERSION = '15.0.3'; @@ -589,6 +590,8 @@ export interface DeliveryRecord { openCode?: OpenCodeState; /** The Pi extension's place as init found it (`tidyPiExtension`). */ pi?: PiState; + /** The agent skill init installed (`relay-skill.ts`), removed by `--uninstall`. */ + relaySkill?: RelaySkillRecord; } const strings = (value: unknown): string[] => @@ -667,12 +670,14 @@ export function readDeliveryRecord(file: string): DeliveryRecord { const claudeFiles = claudeFileStatesOf(parsed?.claudeFiles); const openCode = openCodeStateOf(parsed?.openCode); const pi = piStateOf(parsed?.pi); + const relaySkill = relaySkillRecordOf(parsed?.relaySkill); return { turnedOn: strings(parsed?.turnedOn), claudeInbound: originals(parsed?.claudeInbound), ...(claudeFiles ? { claudeFiles } : {}), ...(openCode ? { openCode } : {}), ...(pi ? { pi } : {}), + ...(relaySkill ? { relaySkill } : {}), }; } catch { return { turnedOn: [], claudeInbound: {} }; diff --git a/packages/cli/src/cli/lib/relay-skill.ts b/packages/cli/src/cli/lib/relay-skill.ts new file mode 100644 index 000000000..0df392ae9 --- /dev/null +++ b/packages/cli/src/cli/lib/relay-skill.ts @@ -0,0 +1,338 @@ +/** + * The agent skill `relay init` installs so every session it injects into + * knows how to find who is on Agent Relay and message them: + * `messaging-agents-on-the-relay` from AgentWorkforce/skills, installed for + * each tool globally with the `skills` CLI (`npx skills add`). + * + * Both the skill and the CLI are pinned, so a change to the skills repo + * cannot change what init installs. Init records each copy it put in place + * in its record (`core-settings.json`), and `relay init --uninstall` removes + * exactly those, and only while they are unchanged. A copy that was already + * there before init, or that the person changed, is never touched. + */ + +import { spawnSync } from 'node:child_process'; +import { createHash } from 'node:crypto'; +import fs from 'node:fs'; +import path from 'node:path'; + +import { findToolTarget, type ToolContext, type ToolId } from './agent-tools-mcp.js'; + +/** The skill's name in AgentWorkforce/skills. */ +export const RELAY_SKILL_NAME = 'messaging-agents-on-the-relay'; +/** The skills repo commit init installs the skill from. */ +export const RELAY_SKILL_REF = 'd00c225fb3605406ef37ee87b42abcec4ceb9698'; +/** The pinned source, as `skills add` takes it (`#`). */ +export const RELAY_SKILL_SOURCE = `https://github.com/AgentWorkforce/skills#${RELAY_SKILL_REF}`; +/** The `skills` CLI version init runs through npx. */ +export const SKILLS_CLI_PACKAGE = 'skills@1.7.4'; + +/** The `skills` CLI's agent name for each tool init sets up. */ +const SKILLS_AGENT: Record = { + claude: 'claude-code', + codex: 'codex', + opencode: 'opencode', + grok: 'grok', + pi: 'pi', + devin: 'devin', +}; + +/** One copy of the skill init put in place. */ +export interface InstalledSkillCopy { + /** The skill's folder. */ + path: string; + /** sha256 of its SKILL.md as installed: --uninstall removes it only while it matches. */ + sha256: string; +} + +/** What init installed of the skill, kept in `core-settings.json` for `--uninstall`. */ +export interface RelaySkillRecord { + ref: string; + copies: InstalledSkillCopy[]; + /** Folders init's install created (the highest missing one per path), removed again once empty. */ + createdDirs: string[]; + /** The `skills` CLI's lock file, when init added the skill's entry to it. */ + lock?: { file: string; created: boolean }; +} + +/** Runs the `skills` CLI with `args`; replaced in tests. */ +export type RunSkillsCli = ( + args: string[], + options: { env: NodeJS.ProcessEnv; cwd: string } +) => { status: number | null; output: string }; + +/** + * The real `npx skills@ …`: no prompts, no telemetry, its output + * kept for a failure's reason. Windows starts npx (a .cmd) only through a + * shell; every argument is a fixed word or a path init computed. + */ +export const defaultRunSkillsCli: RunSkillsCli = (args, { env, cwd }) => { + const options = { + cwd, + env, + encoding: 'utf8' as const, + stdio: ['ignore', 'pipe', 'pipe'] as ['ignore', 'pipe', 'pipe'], + timeout: 180_000, + }; + const result = + process.platform === 'win32' + ? spawnSync(`npx -y ${SKILLS_CLI_PACKAGE} ${args.map((arg) => `"${arg}"`).join(' ')}`, { + ...options, + shell: true, + }) + : spawnSync('npx', ['-y', SKILLS_CLI_PACKAGE, ...args], options); + const output = `${result.stdout ?? ''}${result.stderr ?? ''}${result.error ? result.error.message : ''}`; + return { status: result.status, output }; +}; + +/** + * Where a tool loads global skills, as the `skills` CLI installs them: + * Claude Code `~/.claude/skills` (or `$CLAUDE_CONFIG_DIR/skills`); Codex, + * OpenCode and Pi the shared `~/.agents/skills`; Grok `~/.grok/skills`; + * Devin `$XDG_CONFIG_HOME/devin/skills`. + */ +export function skillsDirFor(id: ToolId, ctx: ToolContext): string { + const env = ctx.env; + switch (id) { + case 'claude': + return path.join(env.CLAUDE_CONFIG_DIR?.trim() || path.join(ctx.home, '.claude'), 'skills'); + case 'grok': + return path.join(env.GROK_HOME?.trim() || path.join(ctx.home, '.grok'), 'skills'); + case 'devin': + return path.join(env.XDG_CONFIG_HOME?.trim() || path.join(ctx.home, '.config'), 'devin', 'skills'); + case 'codex': + case 'opencode': + case 'pi': + return path.join(ctx.home, '.agents', 'skills'); + } +} + +/** The `skills` CLI's lock file (`$XDG_STATE_HOME/skills/.skill-lock.json`, else `~/.agents/.skill-lock.json`). */ +function lockFile(ctx: ToolContext): string { + const state = ctx.env.XDG_STATE_HOME?.trim(); + return state + ? path.join(state, 'skills', '.skill-lock.json') + : path.join(ctx.home, '.agents', '.skill-lock.json'); +} + +function readLock(file: string): { skills?: Record } & Record { + try { + const parsed = JSON.parse(fs.readFileSync(file, 'utf8')); + return parsed && typeof parsed === 'object' ? parsed : {}; + } catch { + return {}; + } +} + +function skillHash(dir: string): string | undefined { + try { + return createHash('sha256') + .update(fs.readFileSync(path.join(dir, 'SKILL.md'))) + .digest('hex'); + } catch { + return undefined; + } +} + +function exists(file: string): boolean { + return fs.lstatSync(file, { throwIfNoEntry: false }) !== undefined; +} + +/** The highest missing folder on the way to `file`, below `home`; undefined when its folder exists. */ +function highestMissing(file: string, home: string): string | undefined { + let missing: string | undefined; + for (let dir = path.dirname(file); dir !== path.dirname(dir) && dir !== home; dir = path.dirname(dir)) { + if (exists(dir)) break; + missing = dir; + } + return missing; +} + +/** Removes `dir` when it holds nothing but empty folders. */ +function removeIfEmpty(dir: string): void { + const empty = (current: string): boolean => { + const stat = fs.lstatSync(current, { throwIfNoEntry: false }); + if (!stat?.isDirectory()) return false; + return fs.readdirSync(current).every((entry) => empty(path.join(current, entry))); + }; + try { + if (empty(dir)) fs.rmSync(dir, { recursive: true }); + } catch { + // Best effort: a folder that cannot be read is left. + } +} + +/** The recorded install, or undefined when the value is not one. */ +export function relaySkillRecordOf(value: unknown): RelaySkillRecord | undefined { + if (!value || typeof value !== 'object') return undefined; + const record = value as Record; + const copies = Array.isArray(record.copies) + ? record.copies.filter( + (copy): copy is InstalledSkillCopy => + typeof copy?.path === 'string' && typeof copy?.sha256 === 'string' + ) + : []; + const createdDirs = Array.isArray(record.createdDirs) + ? record.createdDirs.filter((dir): dir is string => typeof dir === 'string') + : []; + const lock = record.lock as RelaySkillRecord['lock'] | undefined; + if (copies.length === 0 && !lock) return undefined; + return { + ref: typeof record.ref === 'string' ? record.ref : '', + copies, + createdDirs, + ...(lock && typeof lock.file === 'string' + ? { lock: { file: lock.file, created: lock.created === true } } + : {}), + }; +} + +/** One line about the skill, and the record to keep. */ +export interface SkillInstallResult { + line: string; + record: RelaySkillRecord | undefined; +} + +/** + * Install the skill globally for `tools`, where it is not there yet. A copy + * init installed earlier is refreshed when the pinned ref changed; any + * other copy already there is the person's and is left as it is. Never + * throws: a failure is the warning line, and what did land is recorded. + * @param tools - The tools whose sessions the core injects into + * @param ctx - Home and environment the tools resolve against + * @param recorded - What an earlier init installed + * @param run - Runs the `skills` CLI + */ +export function installRelaySkill( + tools: ToolId[], + ctx: ToolContext, + recorded: RelaySkillRecord | undefined, + run: RunSkillsCli +): SkillInstallResult { + const ours = new Map((recorded?.copies ?? []).map((copy) => [copy.path, copy.sha256])); + const byPath = new Map(); + for (const id of tools) { + const dir = path.join(skillsDirFor(id, ctx), RELAY_SKILL_NAME); + byPath.set(dir, [...(byPath.get(dir) ?? []), id]); + } + const install: string[] = []; + const kept: string[] = []; + for (const dir of byPath.keys()) { + const mine = ours.has(dir) && skillHash(dir) === ours.get(dir); + if (!exists(dir) || (mine && recorded?.ref !== RELAY_SKILL_REF)) install.push(dir); + else if (!mine) kept.push(dir); + } + const label = (dirs: string[]) => dirs.map((dir) => byPath.get(dir)!.map(toolLabel).join(', ')).join(', '); + const keptNote = kept.length > 0 ? `; already there for ${label(kept)}, left as is` : ''; + if (install.length === 0) { + return { + line: `✓ Agent skill ${RELAY_SKILL_NAME} ${kept.length > 0 ? 'already there' : 'installed'} for ${label([...byPath.keys()])}`, + record: recorded, + }; + } + + const lock = lockFile(ctx); + const lockExisted = exists(lock); + const lockHadEntry = Boolean(readLock(lock).skills?.[RELAY_SKILL_NAME]); + const createdDirs = [...install, lock] + .map((file) => + install.includes(file) + ? highestMissing(path.join(file, 'SKILL.md'), ctx.home) + : highestMissing(file, ctx.home) + ) + .filter((dir): dir is string => Boolean(dir)); + const agents = install.flatMap((dir) => byPath.get(dir)!.flatMap((id) => ['--agent', SKILLS_AGENT[id]])); + let result: ReturnType; + try { + result = run( + ['add', RELAY_SKILL_SOURCE, '--skill', RELAY_SKILL_NAME, '--global', '--copy', '--yes', ...agents], + // HOME is the tools' home, so the skill lands where they load it. + { env: { ...ctx.env, HOME: ctx.home, DISABLE_TELEMETRY: '1', DO_NOT_TRACK: '1' }, cwd: ctx.home } + ); + } catch (error) { + result = { status: null, output: (error as Error).message }; + } + + const landed = install.filter((dir) => skillHash(dir) !== undefined); + const copies = [ + ...(recorded?.copies ?? []).filter((copy) => !landed.includes(copy.path)), + ...landed.map((dir) => ({ path: dir, sha256: skillHash(dir)! })), + ]; + const lockAdded = !lockHadEntry && Boolean(readLock(lock).skills?.[RELAY_SKILL_NAME]); + const record: RelaySkillRecord | undefined = + copies.length > 0 || recorded?.lock || lockAdded + ? { + ref: landed.length > 0 ? RELAY_SKILL_REF : (recorded?.ref ?? RELAY_SKILL_REF), + copies, + createdDirs: [...new Set([...(recorded?.createdDirs ?? []), ...createdDirs.filter(exists)])], + ...(recorded?.lock + ? { lock: recorded.lock } + : lockAdded + ? { lock: { file: lock, created: !lockExisted } } + : {}), + } + : undefined; + const missed = install.filter((dir) => !landed.includes(dir)); + if (missed.length === 0) { + return { + line: `✓ Agent skill installed ${RELAY_SKILL_NAME} for ${label(landed)}${keptNote}`, + record, + }; + } + const reason = result.status === 0 ? 'it did not land' : lastLine(result.output) || `exit ${result.status}`; + return { + line: + `! Agent skill could not install ${RELAY_SKILL_NAME} for ${label(missed)} (${reason}). ` + + `Messaging still works; to add it: npx ${SKILLS_CLI_PACKAGE} add ${RELAY_SKILL_SOURCE} --skill ${RELAY_SKILL_NAME} --global`, + record, + }; +} + +/** + * Remove what init installed: each recorded copy still as installed, the + * lock entry it added, and the folders it created once they are empty. + * @returns The line to print, or undefined when nothing was recorded + */ +export function uninstallRelaySkill(recorded: RelaySkillRecord | undefined): string | undefined { + if (!recorded) return undefined; + const removed: string[] = []; + const changed: string[] = []; + for (const copy of recorded.copies) { + const hash = skillHash(copy.path); + if (hash === undefined) continue; + if (hash !== copy.sha256) { + changed.push(copy.path); + continue; + } + fs.rmSync(copy.path, { recursive: true, force: true }); + removed.push(copy.path); + } + if (recorded.lock) { + const lock = readLock(recorded.lock.file); + const entry = lock.skills?.[RELAY_SKILL_NAME]; + if (entry && entry.ref === recorded.ref && lock.skills) { + delete lock.skills[RELAY_SKILL_NAME]; + if (recorded.lock.created && Object.keys(lock.skills).length === 0) + fs.rmSync(recorded.lock.file, { force: true }); + else fs.writeFileSync(recorded.lock.file, `${JSON.stringify(lock, null, 2)}`); + } + } + for (const dir of recorded.createdDirs) removeIfEmpty(dir); + const kept = changed.length > 0 ? `; kept ${changed.join(', ')}, changed since` : ''; + return `✓ Agent skill removed the ${RELAY_SKILL_NAME} skill relay init installed${removed.length > 0 ? ` (${removed.join(', ')})` : ''}${kept}`; +} + +function toolLabel(id: ToolId): string { + return findToolTarget(id)?.label ?? id; +} + +/** The last non-empty line of the CLI's output, without terminal styling, for a failure's reason. */ +function lastLine(output: string): string { + const lines = output + // eslint-disable-next-line no-control-regex + .replace(/\u001b\[[0-9;?]*[A-Za-z]/g, '') + .split('\n') + .map((line) => line.replace(/^[\s│◇■└┌]+/, '').trim()) + .filter(Boolean); + return (lines.at(-1) ?? '').slice(0, 200); +} From 2658c0a646d6ad709ea879de05cf883acb1891c8 Mon Sep 17 00:00:00 2001 From: kjgbot Date: Fri, 30 Oct 2026 17:23:31 -0700 Subject: [PATCH 02/11] fix(init): pin the messaging skill to the commit with the older-core fallback Co-Authored-By: Claude Opus 5.5 --- packages/cli/src/cli/lib/relay-skill.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cli/src/cli/lib/relay-skill.ts b/packages/cli/src/cli/lib/relay-skill.ts index 0df392ae9..23ad8725d 100644 --- a/packages/cli/src/cli/lib/relay-skill.ts +++ b/packages/cli/src/cli/lib/relay-skill.ts @@ -21,7 +21,7 @@ import { findToolTarget, type ToolContext, type ToolId } from './agent-tools-mcp /** The skill's name in AgentWorkforce/skills. */ export const RELAY_SKILL_NAME = 'messaging-agents-on-the-relay'; /** The skills repo commit init installs the skill from. */ -export const RELAY_SKILL_REF = 'd00c225fb3605406ef37ee87b42abcec4ceb9698'; +export const RELAY_SKILL_REF = 'ac9fb90d0dd1a1c2fd03c8ba309efd4c62ad4a78'; /** The pinned source, as `skills add` takes it (`#`). */ export const RELAY_SKILL_SOURCE = `https://github.com/AgentWorkforce/skills#${RELAY_SKILL_REF}`; /** The `skills` CLI version init runs through npx. */ From 9445076caa582d2ab2507badca3c45b8e105823a Mon Sep 17 00:00:00 2001 From: kjgbot Date: Fri, 30 Oct 2026 17:25:38 -0700 Subject: [PATCH 03/11] fix(init): remove every folder the skill install created once empty The skills CLI lock under $XDG_STATE_HOME left an empty skills folder when another tool had filled the highest new folder meanwhile. Init now records each folder it created and removes them deepest first, each only while empty. Co-Authored-By: Claude Opus 5.5 --- packages/cli/src/cli/commands/init.test.ts | 22 ++++++++++-- packages/cli/src/cli/lib/relay-skill.ts | 42 +++++++++------------- 2 files changed, 36 insertions(+), 28 deletions(-) diff --git a/packages/cli/src/cli/commands/init.test.ts b/packages/cli/src/cli/commands/init.test.ts index a4b19aeb7..8918de028 100644 --- a/packages/cli/src/cli/commands/init.test.ts +++ b/packages/cli/src/cli/commands/init.test.ts @@ -1822,7 +1822,7 @@ describe('relay init message injection', () => { * global skills folder (Codex, OpenCode and Pi share ~/.agents/skills) and * the skill's entry in its lock file. */ -function fakeSkillsCli(status = 0) { +function fakeSkillsCli(status = 0, lockFile?: string) { const dirs: Record = { 'claude-code': path.join(toolHome, '.claude', 'skills'), codex: path.join(toolHome, '.agents', 'skills'), @@ -1831,7 +1831,7 @@ function fakeSkillsCli(status = 0) { grok: path.join(toolHome, '.grok', 'skills'), devin: path.join(toolHome, '.config', 'devin', 'skills'), }; - const lock = path.join(toolHome, '.agents', '.skill-lock.json'); + const lock = lockFile ?? path.join(toolHome, '.agents', '.skill-lock.json'); const run = vi.fn((args) => { if (status !== 0) return { status, output: 'fatal: unable to access github.com' }; args.forEach((arg, i) => { @@ -1844,6 +1844,7 @@ function fakeSkillsCli(status = 0) { ? JSON.parse(fs.readFileSync(lock, 'utf8')) : { version: 3, skills: {} }; current.skills[RELAY_SKILL_NAME] = { source: 'AgentWorkforce/skills', ref: RELAY_SKILL_REF }; + fs.mkdirSync(path.dirname(lock), { recursive: true }); fs.writeFileSync(lock, JSON.stringify(current)); return { status: 0, output: 'Done!' }; }); @@ -1946,6 +1947,23 @@ describe('relay init messaging skill', () => { expect(JSON.parse(fs.readFileSync(lock, 'utf8')).skills).toEqual({ 'other-skill': { ref: 'x' } }); }); + it("removes every folder its install created, even where a folder above holds the person's files", async () => { + installedProbe(); + const state = path.join(toolHome, 'xdg', 'state'); + const core = fakeCore(); + const skills = fakeSkillsCli(0, path.join(state, 'skills', '.skill-lock.json')); + const ctx = { projectRoot: project, home: toolHome, env: { PATH: '', XDG_STATE_HOME: state } }; + await setup({ ...core.deps, runSkillsCli: skills.run, toolContext: () => ctx }).run('--tools', 'codex'); + expect(fs.existsSync(skills.lock)).toBe(true); + // Another tool fills the folder init created above the lock's. + fs.mkdirSync(path.join(toolHome, 'xdg', 'cache'), { recursive: true }); + fs.writeFileSync(path.join(toolHome, 'xdg', 'cache', 'keep'), 'theirs'); + await setup({ ...core.deps, toolContext: () => ctx }).run('--uninstall'); + expect(fs.existsSync(state)).toBe(false); + expect(fs.existsSync(path.join(toolHome, '.agents'))).toBe(false); + expect(fs.readFileSync(path.join(toolHome, 'xdg', 'cache', 'keep'), 'utf8')).toBe('theirs'); + }); + it('keeps a copy changed after init on --uninstall', async () => { installedProbe(); const core = fakeCore(); diff --git a/packages/cli/src/cli/lib/relay-skill.ts b/packages/cli/src/cli/lib/relay-skill.ts index 23ad8725d..b6892c0c3 100644 --- a/packages/cli/src/cli/lib/relay-skill.ts +++ b/packages/cli/src/cli/lib/relay-skill.ts @@ -49,7 +49,7 @@ export interface InstalledSkillCopy { export interface RelaySkillRecord { ref: string; copies: InstalledSkillCopy[]; - /** Folders init's install created (the highest missing one per path), removed again once empty. */ + /** Folders init's install created, removed again once empty. */ createdDirs: string[]; /** The `skills` CLI's lock file, when init added the skill's entry to it. */ lock?: { file: string; created: boolean }; @@ -138,27 +138,23 @@ function exists(file: string): boolean { return fs.lstatSync(file, { throwIfNoEntry: false }) !== undefined; } -/** The highest missing folder on the way to `file`, below `home`; undefined when its folder exists. */ -function highestMissing(file: string, home: string): string | undefined { - let missing: string | undefined; - for (let dir = path.dirname(file); dir !== path.dirname(dir) && dir !== home; dir = path.dirname(dir)) { - if (exists(dir)) break; - missing = dir; +/** Every missing folder on the way to `file`, deepest first; empty when its folder exists. */ +function missingDirs(file: string): string[] { + const missing: string[] = []; + for (let dir = path.dirname(file); dir !== path.dirname(dir) && !exists(dir); dir = path.dirname(dir)) { + missing.push(dir); } return missing; } -/** Removes `dir` when it holds nothing but empty folders. */ -function removeIfEmpty(dir: string): void { - const empty = (current: string): boolean => { - const stat = fs.lstatSync(current, { throwIfNoEntry: false }); - if (!stat?.isDirectory()) return false; - return fs.readdirSync(current).every((entry) => empty(path.join(current, entry))); - }; - try { - if (empty(dir)) fs.rmSync(dir, { recursive: true }); - } catch { - // Best effort: a folder that cannot be read is left. +/** Removes the folders init created, deepest first, each only while it is empty. */ +function removeEmptyDirs(dirs: string[]): void { + for (const dir of [...dirs].sort((a, b) => b.length - a.length)) { + try { + fs.rmdirSync(dir); + } catch { + // Not empty, already gone, or unreadable: left as it is. + } } } @@ -234,13 +230,7 @@ export function installRelaySkill( const lock = lockFile(ctx); const lockExisted = exists(lock); const lockHadEntry = Boolean(readLock(lock).skills?.[RELAY_SKILL_NAME]); - const createdDirs = [...install, lock] - .map((file) => - install.includes(file) - ? highestMissing(path.join(file, 'SKILL.md'), ctx.home) - : highestMissing(file, ctx.home) - ) - .filter((dir): dir is string => Boolean(dir)); + const createdDirs = [...install.map((dir) => path.join(dir, 'SKILL.md')), lock].flatMap(missingDirs); const agents = install.flatMap((dir) => byPath.get(dir)!.flatMap((id) => ['--agent', SKILLS_AGENT[id]])); let result: ReturnType; try { @@ -317,7 +307,7 @@ export function uninstallRelaySkill(recorded: RelaySkillRecord | undefined): str else fs.writeFileSync(recorded.lock.file, `${JSON.stringify(lock, null, 2)}`); } } - for (const dir of recorded.createdDirs) removeIfEmpty(dir); + removeEmptyDirs(recorded.createdDirs); const kept = changed.length > 0 ? `; kept ${changed.join(', ')}, changed since` : ''; return `✓ Agent skill removed the ${RELAY_SKILL_NAME} skill relay init installed${removed.length > 0 ? ` (${removed.join(', ')})` : ''}${kept}`; } From bcafa5b8496b3336978e65634df603d5a1c0f0f6 Mon Sep 17 00:00:00 2001 From: kjgbot Date: Fri, 30 Oct 2026 17:33:22 -0700 Subject: [PATCH 04/11] fix(init): address review of the messaging-skill install - Uninstall compares a hash over every file in a copy, not only SKILL.md, so a file the person added or changed keeps the copy. - Each copy records its own ref. A copy counts as current only if this run wrote it, so a failed refresh is retried, and a copy of a tool not set up this run keeps its old ref until it is refreshed. - The skills CLI lock entry init replaced is recorded and put back on uninstall, while the entry still names the ref init wrote. - Uninstall only touches the skill's own folder in a tool's skills folder and the CLI's lock file, whatever core-settings.json says. - The skills CLI gets only plain-word arguments, so the Windows shell invocation cannot run anything else. - Any error in the skill step, including an unreadable state folder, is a warning, never an init failure. - JSDoc @param/@returns on the exported functions. - Pin the skill to skills#195's head after its review fixes. Co-Authored-By: Claude Opus 5.5 --- packages/cli/src/cli/commands/init.test.ts | 110 +++++++- packages/cli/src/cli/commands/init.ts | 21 +- packages/cli/src/cli/lib/relay-skill.ts | 293 +++++++++++++++------ 3 files changed, 323 insertions(+), 101 deletions(-) diff --git a/packages/cli/src/cli/commands/init.test.ts b/packages/cli/src/cli/commands/init.test.ts index 8918de028..a77536247 100644 --- a/packages/cli/src/cli/commands/init.test.ts +++ b/packages/cli/src/cli/commands/init.test.ts @@ -20,6 +20,7 @@ import { RELAY_SKILL_NAME, RELAY_SKILL_REF, SKILLS_CLI_PACKAGE, + defaultRunSkillsCli, type RunSkillsCli, } from '../lib/relay-skill.js'; import { getProjectPaths } from '@agent-relay/config'; @@ -1832,8 +1833,9 @@ function fakeSkillsCli(status = 0, lockFile?: string) { devin: path.join(toolHome, '.config', 'devin', 'skills'), }; const lock = lockFile ?? path.join(toolHome, '.agents', '.skill-lock.json'); + const state = { status }; const run = vi.fn((args) => { - if (status !== 0) return { status, output: 'fatal: unable to access github.com' }; + if (state.status !== 0) return { status: state.status, output: 'fatal: unable to access github.com' }; args.forEach((arg, i) => { if (args[i - 1] !== '--agent') return; const dir = path.join(dirs[arg]!, RELAY_SKILL_NAME); @@ -1848,7 +1850,15 @@ function fakeSkillsCli(status = 0, lockFile?: string) { fs.writeFileSync(lock, JSON.stringify(current)); return { status: 0, output: 'Done!' }; }); - return { run, lock, dirs }; + return { run, lock, dirs, fail: (next: number) => (state.status = next) }; +} + +/** Rewrite the recorded skill install, as an older init or a forged file would have left it. */ +function editSkillRecord(change: (record: Record) => void): void { + const file = path.join(home, 'local', 'core-settings.json'); + const settings = JSON.parse(fs.readFileSync(file, 'utf8')); + change(settings.relaySkill); + fs.writeFileSync(file, JSON.stringify(settings)); } /** Every file and folder under the tools' home, relative to it. */ @@ -1897,12 +1907,11 @@ describe('relay init messaging skill', () => { ); const record = JSON.parse(fs.readFileSync(path.join(home, 'local', 'core-settings.json'), 'utf8')); expect(record.relaySkill).toMatchObject({ - ref: RELAY_SKILL_REF, copies: [ - { path: path.join(skills.dirs['claude-code']!, RELAY_SKILL_NAME) }, - { path: path.join(skills.dirs.codex!, RELAY_SKILL_NAME) }, + { path: path.join(skills.dirs['claude-code']!, RELAY_SKILL_NAME), ref: RELAY_SKILL_REF }, + { path: path.join(skills.dirs.codex!, RELAY_SKILL_NAME), ref: RELAY_SKILL_REF }, ], - lock: { file: skills.lock, created: true }, + lock: { file: skills.lock, created: true, original: null, ref: RELAY_SKILL_REF }, }); // A re-run with the same pin changes nothing and fetches nothing. @@ -1964,6 +1973,95 @@ describe('relay init messaging skill', () => { expect(fs.readFileSync(path.join(toolHome, 'xdg', 'cache', 'keep'), 'utf8')).toBe('theirs'); }); + it('keeps a copy with a file added after init on --uninstall', async () => { + installedProbe(); + const core = fakeCore(); + const skills = fakeSkillsCli(); + await setup({ ...core.deps, runSkillsCli: skills.run }).run('--tools', 'codex'); + const notes = path.join(skills.dirs.codex!, RELAY_SKILL_NAME, 'notes.md'); + fs.writeFileSync(notes, 'mine\n'); + await setup(core.deps).run('--uninstall'); + expect(fs.readFileSync(notes, 'utf8')).toBe('mine\n'); + }); + + it('retries a refresh that failed, and refreshes each copy by its own ref', async () => { + installedProbe(); + const core = fakeCore(); + const skills = fakeSkillsCli(); + await setup({ ...core.deps, runSkillsCli: skills.run }).run('--tools', 'claude,codex'); + // An older pin installed both copies. + editSkillRecord((record) => { + for (const copy of record.copies) copy.ref = 'old'; + }); + + skills.fail(1); + const offline = setup({ ...core.deps, runSkillsCli: skills.run }); + await offline.run('--tools', 'claude,codex'); + expect(offline.logs.join('\n')).toContain( + 'could not install messaging-agents-on-the-relay for Claude Code, Codex' + ); + skills.fail(0); + // Only Codex is set up this time: Claude's copy keeps its own old ref. + await setup({ ...core.deps, runSkillsCli: skills.run }).run('--tools', 'codex'); + const refs = () => + JSON.parse( + fs.readFileSync(path.join(home, 'local', 'core-settings.json'), 'utf8') + ).relaySkill.copies.map((copy: { ref: string }) => copy.ref); + expect(refs()).toEqual(['old', RELAY_SKILL_REF]); + skills.run.mockClear(); + await setup({ ...core.deps, runSkillsCli: skills.run }).run('--tools', 'claude,codex'); + const args = skills.run.mock.calls[0]![0]; + expect(args.filter((_, i) => args[i - 1] === '--agent')).toEqual(['claude-code']); + expect(refs()).toEqual([RELAY_SKILL_REF, RELAY_SKILL_REF]); + }); + + it("puts back the person's own lock entry that the install replaced", async () => { + installedProbe(); + const lock = path.join(toolHome, '.agents', '.skill-lock.json'); + fs.mkdirSync(path.dirname(lock), { recursive: true }); + const own = { source: 'AgentWorkforce/skills', ref: 'their-ref' }; + fs.writeFileSync(lock, JSON.stringify({ version: 3, skills: { [RELAY_SKILL_NAME]: own } })); + const core = fakeCore(); + const skills = fakeSkillsCli(); + await setup({ ...core.deps, runSkillsCli: skills.run }).run('--tools', 'codex'); + expect(JSON.parse(fs.readFileSync(lock, 'utf8')).skills[RELAY_SKILL_NAME].ref).toBe(RELAY_SKILL_REF); + await setup(core.deps).run('--uninstall'); + expect(JSON.parse(fs.readFileSync(lock, 'utf8')).skills[RELAY_SKILL_NAME]).toEqual(own); + }); + + it('never removes a folder outside the skills folders, whatever the record says', async () => { + installedProbe(); + const core = fakeCore(); + const skills = fakeSkillsCli(); + await setup({ ...core.deps, runSkillsCli: skills.run }).run('--tools', 'codex'); + // A forged record names an unrelated folder with the same contents. + const unrelated = path.join(toolHome, 'work', RELAY_SKILL_NAME); + fs.cpSync(path.join(skills.dirs.codex!, RELAY_SKILL_NAME), unrelated, { recursive: true }); + editSkillRecord((record) => { + record.copies.push({ ...record.copies[0], path: unrelated }); + record.createdDirs.push(path.join(toolHome, 'work')); + }); + await setup(core.deps).run('--uninstall'); + expect(fs.existsSync(path.join(unrelated, 'SKILL.md'))).toBe(true); + }); + + it('finishes init with a warning when the skills state folder cannot be read', async () => { + installedProbe(); + const core = fakeCore(); + const skills = fakeSkillsCli(); + const blocked = path.join(toolHome, 'state-is-a-file'); + fs.writeFileSync(blocked, ''); + const ctx = { projectRoot: project, home: toolHome, env: { PATH: '', XDG_STATE_HOME: blocked } }; + const { run, logs } = setup({ ...core.deps, runSkillsCli: skills.run, toolContext: () => ctx }); + await expect(run('--tools', 'codex')).resolves.toBeDefined(); + expect(logs.join('\n')).toMatch(/! Agent skill +could not install/); + }); + + it('refuses to hand the skills CLI an argument with shell characters', () => { + const result = defaultRunSkillsCli(['add', 'x & calc'], { env: {}, cwd: toolHome }); + expect(result).toEqual({ status: null, output: 'refused an argument with shell characters: x & calc' }); + }); + it('keeps a copy changed after init on --uninstall', async () => { installedProbe(); const core = fakeCore(); diff --git a/packages/cli/src/cli/commands/init.ts b/packages/cli/src/cli/commands/init.ts index 7fcfe9ff8..c986b6797 100644 --- a/packages/cli/src/cli/commands/init.ts +++ b/packages/cli/src/cli/commands/init.ts @@ -1394,18 +1394,21 @@ function installSkill( ctx: ToolContext ): string | undefined { if (injected.length === 0) return undefined; - const { line, record } = installRelaySkill( - injected, - ctx, - readDeliveryRecord(deliveryRecordPath()).relaySkill, - deps.runSkillsCli - ); + let line: string; try { + const result = installRelaySkill( + injected, + ctx, + readDeliveryRecord(deliveryRecordPath()).relaySkill, + deps.runSkillsCli + ); + line = result.line; updateDeliveryRecord(({ relaySkill: _previous, ...rest }) => - record ? { ...rest, relaySkill: record } : rest + result.record ? { ...rest, relaySkill: result.record } : rest ); } catch (error) { - return `${line} (not recorded for --uninstall: ${(error as Error).message})`; + // The skill is optional: nothing about it may fail init. + return `! Agent skill could not be installed or recorded (${(error as Error).message})`; } return line; } @@ -1573,7 +1576,7 @@ async function uninstall(deps: InitCommandDependencies, o: InitOptions): Promise // The skill is installed per user, like injection, and removed with it. const skillRecord = readDeliveryRecord(deliveryRecordPath()).relaySkill; const skillLine = - o.scope !== 'project' && o.tools === undefined ? uninstallRelaySkill(skillRecord) : undefined; + o.scope !== 'project' && o.tools === undefined ? uninstallRelaySkill(skillRecord, ctx) : undefined; if (skillLine) { deps.log(skillLine); updateDeliveryRecord(({ relaySkill: _removed, ...rest }) => rest); diff --git a/packages/cli/src/cli/lib/relay-skill.ts b/packages/cli/src/cli/lib/relay-skill.ts index b6892c0c3..0c572059a 100644 --- a/packages/cli/src/cli/lib/relay-skill.ts +++ b/packages/cli/src/cli/lib/relay-skill.ts @@ -7,7 +7,7 @@ * Both the skill and the CLI are pinned, so a change to the skills repo * cannot change what init installs. Init records each copy it put in place * in its record (`core-settings.json`), and `relay init --uninstall` removes - * exactly those, and only while they are unchanged. A copy that was already + * exactly those, and only while every file in them is unchanged. A copy that was already * there before init, or that the person changed, is never touched. */ @@ -16,12 +16,12 @@ import { createHash } from 'node:crypto'; import fs from 'node:fs'; import path from 'node:path'; -import { findToolTarget, type ToolContext, type ToolId } from './agent-tools-mcp.js'; +import { TOOL_IDS, findToolTarget, type ToolContext, type ToolId } from './agent-tools-mcp.js'; /** The skill's name in AgentWorkforce/skills. */ export const RELAY_SKILL_NAME = 'messaging-agents-on-the-relay'; /** The skills repo commit init installs the skill from. */ -export const RELAY_SKILL_REF = 'ac9fb90d0dd1a1c2fd03c8ba309efd4c62ad4a78'; +export const RELAY_SKILL_REF = '46f8581307f79e8183c0dd2a5fd9847d68d46ee3'; /** The pinned source, as `skills add` takes it (`#`). */ export const RELAY_SKILL_SOURCE = `https://github.com/AgentWorkforce/skills#${RELAY_SKILL_REF}`; /** The `skills` CLI version init runs through npx. */ @@ -41,18 +41,30 @@ const SKILLS_AGENT: Record = { export interface InstalledSkillCopy { /** The skill's folder. */ path: string; - /** sha256 of its SKILL.md as installed: --uninstall removes it only while it matches. */ - sha256: string; + /** The skills repo commit this copy was installed from. */ + ref: string; + /** sha256 over every file in the folder as installed: --uninstall removes it only while it matches. */ + treeSha256: string; +} + +/** The skill's entry in the `skills` CLI's lock file, as init found and left it. */ +export interface SkillLockRecord { + file: string; + /** Init created the lock file. */ + created: boolean; + /** The entry before init first wrote it, or null when there was none: what --uninstall puts back. */ + original: unknown; + /** The ref init last wrote there: --uninstall touches the entry only while it still names it. */ + ref: string; } /** What init installed of the skill, kept in `core-settings.json` for `--uninstall`. */ export interface RelaySkillRecord { - ref: string; copies: InstalledSkillCopy[]; /** Folders init's install created, removed again once empty. */ createdDirs: string[]; - /** The `skills` CLI's lock file, when init added the skill's entry to it. */ - lock?: { file: string; created: boolean }; + /** The `skills` CLI's lock entry, when init wrote it. */ + lock?: SkillLockRecord; } /** Runs the `skills` CLI with `args`; replaced in tests. */ @@ -61,12 +73,24 @@ export type RunSkillsCli = ( options: { env: NodeJS.ProcessEnv; cwd: string } ) => { status: number | null; output: string }; +/** The only characters an argument to the `skills` CLI may hold: no shell syntax on any platform. */ +const PLAIN_ARGUMENT = /^[A-Za-z0-9@:/._#=+-]+$/; + /** * The real `npx skills@ …`: no prompts, no telemetry, its output * kept for a failure's reason. Windows starts npx (a .cmd) only through a - * shell; every argument is a fixed word or a path init computed. + * shell, so every argument must be a plain word (`PLAIN_ARGUMENT`); any + * other is refused before anything runs. + * @param args - The `skills` CLI's arguments + * @param options - Its environment and working folder + * @returns Its exit status and combined output */ export const defaultRunSkillsCli: RunSkillsCli = (args, { env, cwd }) => { + const words = ['-y', SKILLS_CLI_PACKAGE, ...args]; + const unsafe = words.find((word) => !PLAIN_ARGUMENT.test(word)); + if (unsafe !== undefined) { + return { status: null, output: `refused an argument with shell characters: ${unsafe}` }; + } const options = { cwd, env, @@ -76,11 +100,8 @@ export const defaultRunSkillsCli: RunSkillsCli = (args, { env, cwd }) => { }; const result = process.platform === 'win32' - ? spawnSync(`npx -y ${SKILLS_CLI_PACKAGE} ${args.map((arg) => `"${arg}"`).join(' ')}`, { - ...options, - shell: true, - }) - : spawnSync('npx', ['-y', SKILLS_CLI_PACKAGE, ...args], options); + ? spawnSync(`npx ${words.join(' ')}`, { ...options, shell: true }) + : spawnSync('npx', words, options); const output = `${result.stdout ?? ''}${result.stderr ?? ''}${result.error ? result.error.message : ''}`; return { status: result.status, output }; }; @@ -90,6 +111,9 @@ export const defaultRunSkillsCli: RunSkillsCli = (args, { env, cwd }) => { * Claude Code `~/.claude/skills` (or `$CLAUDE_CONFIG_DIR/skills`); Codex, * OpenCode and Pi the shared `~/.agents/skills`; Grok `~/.grok/skills`; * Devin `$XDG_CONFIG_HOME/devin/skills`. + * @param id - The tool + * @param ctx - Home and environment the tool resolves against + * @returns The tool's global skills folder */ export function skillsDirFor(id: ToolId, ctx: ToolContext): string { const env = ctx.env; @@ -107,6 +131,11 @@ export function skillsDirFor(id: ToolId, ctx: ToolContext): string { } } +/** The skill's own folder for `id`. */ +function skillDir(id: ToolId, ctx: ToolContext): string { + return path.join(skillsDirFor(id, ctx), RELAY_SKILL_NAME); +} + /** The `skills` CLI's lock file (`$XDG_STATE_HOME/skills/.skill-lock.json`, else `~/.agents/.skill-lock.json`). */ function lockFile(ctx: ToolContext): string { const state = ctx.env.XDG_STATE_HOME?.trim(); @@ -115,7 +144,9 @@ function lockFile(ctx: ToolContext): string { : path.join(ctx.home, '.agents', '.skill-lock.json'); } -function readLock(file: string): { skills?: Record } & Record { +type SkillLock = { skills?: Record } & Record; + +function readLock(file: string): SkillLock { try { const parsed = JSON.parse(fs.readFileSync(file, 'utf8')); return parsed && typeof parsed === 'object' ? parsed : {}; @@ -124,11 +155,33 @@ function readLock(file: string): { skills?: Record } & } } -function skillHash(dir: string): string | undefined { +/** + * sha256 over every entry under `dir` (its relative path, then a file's + * contents or a link's target), or undefined when `dir` is not a real + * folder or cannot be read. Any added, removed or changed file changes it. + */ +function treeHash(dir: string): string | undefined { + const hash = createHash('sha256'); + const walk = (current: string, relative: string): void => { + const entries = fs.readdirSync(current, { withFileTypes: true }); + entries.sort((a, b) => (a.name < b.name ? -1 : a.name > b.name ? 1 : 0)); + for (const entry of entries) { + const full = path.join(current, entry.name); + const name = path.posix.join(relative, entry.name); + if (entry.isDirectory()) { + hash.update(`d\0${name}\0`); + walk(full, name); + } else if (entry.isSymbolicLink()) { + hash.update(`l\0${name}\0${fs.readlinkSync(full)}\0`); + } else { + hash.update(`f\0${name}\0`).update(fs.readFileSync(full)).update('\0'); + } + } + }; try { - return createHash('sha256') - .update(fs.readFileSync(path.join(dir, 'SKILL.md'))) - .digest('hex'); + if (!fs.lstatSync(dir, { throwIfNoEntry: false })?.isDirectory()) return undefined; + walk(dir, ''); + return hash.digest('hex'); } catch { return undefined; } @@ -158,29 +211,32 @@ function removeEmptyDirs(dirs: string[]): void { } } -/** The recorded install, or undefined when the value is not one. */ +/** + * Read the recorded install. + * @param value - The `relaySkill` value of `core-settings.json` + * @returns The record, or undefined when the value is not one + */ export function relaySkillRecordOf(value: unknown): RelaySkillRecord | undefined { if (!value || typeof value !== 'object') return undefined; const record = value as Record; const copies = Array.isArray(record.copies) ? record.copies.filter( (copy): copy is InstalledSkillCopy => - typeof copy?.path === 'string' && typeof copy?.sha256 === 'string' + typeof copy?.path === 'string' && + typeof copy?.ref === 'string' && + typeof copy?.treeSha256 === 'string' ) : []; const createdDirs = Array.isArray(record.createdDirs) ? record.createdDirs.filter((dir): dir is string => typeof dir === 'string') : []; - const lock = record.lock as RelaySkillRecord['lock'] | undefined; + const raw = record.lock as Partial | undefined; + const lock = + raw && typeof raw.file === 'string' && typeof raw.ref === 'string' + ? { file: raw.file, created: raw.created === true, original: raw.original ?? null, ref: raw.ref } + : undefined; if (copies.length === 0 && !lock) return undefined; - return { - ref: typeof record.ref === 'string' ? record.ref : '', - copies, - createdDirs, - ...(lock && typeof lock.file === 'string' - ? { lock: { file: lock.file, created: lock.created === true } } - : {}), - }; + return { copies, createdDirs, ...(lock ? { lock } : {}) }; } /** One line about the skill, and the record to keep. */ @@ -191,13 +247,15 @@ export interface SkillInstallResult { /** * Install the skill globally for `tools`, where it is not there yet. A copy - * init installed earlier is refreshed when the pinned ref changed; any - * other copy already there is the person's and is left as it is. Never - * throws: a failure is the warning line, and what did land is recorded. + * init installed earlier is refreshed when it is from another pinned ref + * and unchanged; any other copy already there is the person's and is left + * as it is. Never throws: a failure is the warning line, and only what did + * land is recorded as current, so a failed refresh is tried again next time. * @param tools - The tools whose sessions the core injects into * @param ctx - Home and environment the tools resolve against * @param recorded - What an earlier init installed * @param run - Runs the `skills` CLI + * @returns The line to print and the record to keep */ export function installRelaySkill( tools: ToolId[], @@ -205,33 +263,73 @@ export function installRelaySkill( recorded: RelaySkillRecord | undefined, run: RunSkillsCli ): SkillInstallResult { - const ours = new Map((recorded?.copies ?? []).map((copy) => [copy.path, copy.sha256])); - const byPath = new Map(); - for (const id of tools) { - const dir = path.join(skillsDirFor(id, ctx), RELAY_SKILL_NAME); - byPath.set(dir, [...(byPath.get(dir) ?? []), id]); + try { + return install(tools, ctx, recorded, run); + } catch (error) { + return { line: failureLine(tools.map(toolLabel).join(', '), (error as Error).message), record: recorded }; } - const install: string[] = []; +} + +/** The folders to install into (absent, or init's own unchanged copy from another ref) and those left as they are. */ +function plan( + dirs: string[], + recorded: RelaySkillRecord | undefined +): { before: Map; kept: string[] } { + const ours = new Map((recorded?.copies ?? []).map((copy) => [copy.path, copy])); + const before = new Map(); const kept: string[] = []; - for (const dir of byPath.keys()) { - const mine = ours.has(dir) && skillHash(dir) === ours.get(dir); - if (!exists(dir) || (mine && recorded?.ref !== RELAY_SKILL_REF)) install.push(dir); - else if (!mine) kept.push(dir); + for (const dir of dirs) { + const copy = ours.get(dir); + if (!exists(dir)) { + before.set(dir, undefined); + continue; + } + const now = treeHash(dir); + if (!copy || now !== copy.treeSha256) kept.push(dir); + else if (copy.ref !== RELAY_SKILL_REF) before.set(dir, now); } + return { before, kept }; +} + +/** The lock entry to record after a run: kept from an earlier run, or the one this run wrote. */ +function lockAfter( + recorded: SkillLockRecord | undefined, + file: string, + existed: boolean, + entryBefore: unknown +): SkillLockRecord | undefined { + const entryAfter = readLock(file).skills?.[RELAY_SKILL_NAME]; + const wrote = + entryAfter?.ref === RELAY_SKILL_REF && + (entryBefore as { ref?: string } | undefined)?.ref !== RELAY_SKILL_REF; + if (recorded) return wrote ? { ...recorded, ref: RELAY_SKILL_REF } : recorded; + return wrote ? { file, created: !existed, original: entryBefore ?? null, ref: RELAY_SKILL_REF } : undefined; +} + +function install( + tools: ToolId[], + ctx: ToolContext, + recorded: RelaySkillRecord | undefined, + run: RunSkillsCli +): SkillInstallResult { + const byPath = new Map(); + for (const id of tools) byPath.set(skillDir(id, ctx), [...(byPath.get(skillDir(id, ctx)) ?? []), id]); + const { before, kept } = plan([...byPath.keys()], recorded); + const targets = [...before.keys()]; const label = (dirs: string[]) => dirs.map((dir) => byPath.get(dir)!.map(toolLabel).join(', ')).join(', '); - const keptNote = kept.length > 0 ? `; already there for ${label(kept)}, left as is` : ''; - if (install.length === 0) { + if (targets.length === 0) { + const state = kept.length > 0 ? 'already there' : 'installed'; return { - line: `✓ Agent skill ${RELAY_SKILL_NAME} ${kept.length > 0 ? 'already there' : 'installed'} for ${label([...byPath.keys()])}`, + line: `✓ Agent skill ${RELAY_SKILL_NAME} ${state} for ${label([...byPath.keys()])}`, record: recorded, }; } const lock = lockFile(ctx); const lockExisted = exists(lock); - const lockHadEntry = Boolean(readLock(lock).skills?.[RELAY_SKILL_NAME]); - const createdDirs = [...install.map((dir) => path.join(dir, 'SKILL.md')), lock].flatMap(missingDirs); - const agents = install.flatMap((dir) => byPath.get(dir)!.flatMap((id) => ['--agent', SKILLS_AGENT[id]])); + const entryBefore = readLock(lock).skills?.[RELAY_SKILL_NAME]; + const createdDirs = [...targets.map((dir) => path.join(dir, 'SKILL.md')), lock].flatMap(missingDirs); + const agents = targets.flatMap((dir) => byPath.get(dir)!.flatMap((id) => ['--agent', SKILLS_AGENT[id]])); let result: ReturnType; try { result = run( @@ -243,73 +341,96 @@ export function installRelaySkill( result = { status: null, output: (error as Error).message }; } - const landed = install.filter((dir) => skillHash(dir) !== undefined); + // A copy is current only if this run put it there: new, or written by a + // run that succeeded. An old copy a failed refresh left keeps its old ref. + const after = new Map(targets.map((dir) => [dir, treeHash(dir)])); + const landed = targets.filter((dir) => { + const hash = after.get(dir); + return ( + hash !== undefined && (before.get(dir) === undefined || result.status === 0 || hash !== before.get(dir)) + ); + }); const copies = [ ...(recorded?.copies ?? []).filter((copy) => !landed.includes(copy.path)), - ...landed.map((dir) => ({ path: dir, sha256: skillHash(dir)! })), + ...landed.map((dir) => ({ path: dir, ref: RELAY_SKILL_REF, treeSha256: after.get(dir)! })), ]; - const lockAdded = !lockHadEntry && Boolean(readLock(lock).skills?.[RELAY_SKILL_NAME]); + const lockRecord = lockAfter(recorded?.lock, lock, lockExisted, entryBefore); const record: RelaySkillRecord | undefined = - copies.length > 0 || recorded?.lock || lockAdded + copies.length > 0 || lockRecord ? { - ref: landed.length > 0 ? RELAY_SKILL_REF : (recorded?.ref ?? RELAY_SKILL_REF), copies, createdDirs: [...new Set([...(recorded?.createdDirs ?? []), ...createdDirs.filter(exists)])], - ...(recorded?.lock - ? { lock: recorded.lock } - : lockAdded - ? { lock: { file: lock, created: !lockExisted } } - : {}), + ...(lockRecord ? { lock: lockRecord } : {}), } : undefined; - const missed = install.filter((dir) => !landed.includes(dir)); - if (missed.length === 0) { - return { - line: `✓ Agent skill installed ${RELAY_SKILL_NAME} for ${label(landed)}${keptNote}`, - record, - }; + const missed = targets.filter((dir) => !landed.includes(dir)); + if (missed.length > 0) { + const reason = + result.status === 0 ? 'it did not land' : lastLine(result.output) || `exit ${result.status}`; + return { line: failureLine(label(missed), reason), record }; } - const reason = result.status === 0 ? 'it did not land' : lastLine(result.output) || `exit ${result.status}`; + const keptNote = kept.length > 0 ? `; already there for ${label(kept)}, left as is` : ''; return { - line: - `! Agent skill could not install ${RELAY_SKILL_NAME} for ${label(missed)} (${reason}). ` + - `Messaging still works; to add it: npx ${SKILLS_CLI_PACKAGE} add ${RELAY_SKILL_SOURCE} --skill ${RELAY_SKILL_NAME} --global`, + line: `✓ Agent skill installed ${RELAY_SKILL_NAME} for ${label(landed)}${keptNote}`, record, }; } +function failureLine(tools: string, reason: string): string { + return ( + `! Agent skill could not install ${RELAY_SKILL_NAME} for ${tools} (${reason}). ` + + `Messaging still works; to add it: npx ${SKILLS_CLI_PACKAGE} add ${RELAY_SKILL_SOURCE} --skill ${RELAY_SKILL_NAME} --global` + ); +} + /** - * Remove what init installed: each recorded copy still as installed, the - * lock entry it added, and the folders it created once they are empty. + * Remove what init installed: each recorded copy whose every file is as + * installed, the lock entry it wrote (putting back the one it replaced), + * and the folders it created once they are empty. Only the skill's own + * folder in a tool's skills folder and the `skills` CLI's lock file are + * ever touched, whatever the record says. + * @param recorded - What init installed + * @param ctx - Home and environment the tools resolve against * @returns The line to print, or undefined when nothing was recorded */ -export function uninstallRelaySkill(recorded: RelaySkillRecord | undefined): string | undefined { +export function uninstallRelaySkill( + recorded: RelaySkillRecord | undefined, + ctx: ToolContext +): string | undefined { if (!recorded) return undefined; + const allowed = new Set(TOOL_IDS.map((id) => skillDir(id, ctx))); + const lock = lockFile(ctx); const removed: string[] = []; const changed: string[] = []; for (const copy of recorded.copies) { - const hash = skillHash(copy.path); + if (!allowed.has(copy.path)) continue; + const hash = treeHash(copy.path); if (hash === undefined) continue; - if (hash !== copy.sha256) { + if (hash !== copy.treeSha256) { changed.push(copy.path); continue; } fs.rmSync(copy.path, { recursive: true, force: true }); removed.push(copy.path); } - if (recorded.lock) { - const lock = readLock(recorded.lock.file); - const entry = lock.skills?.[RELAY_SKILL_NAME]; - if (entry && entry.ref === recorded.ref && lock.skills) { - delete lock.skills[RELAY_SKILL_NAME]; - if (recorded.lock.created && Object.keys(lock.skills).length === 0) - fs.rmSync(recorded.lock.file, { force: true }); - else fs.writeFileSync(recorded.lock.file, `${JSON.stringify(lock, null, 2)}`); - } - } - removeEmptyDirs(recorded.createdDirs); + if (recorded.lock?.file === lock) restoreLock(recorded.lock); + const roots = [...allowed, lock]; + removeEmptyDirs( + recorded.createdDirs.filter((dir) => roots.some((root) => root.startsWith(dir + path.sep))) + ); const kept = changed.length > 0 ? `; kept ${changed.join(', ')}, changed since` : ''; - return `✓ Agent skill removed the ${RELAY_SKILL_NAME} skill relay init installed${removed.length > 0 ? ` (${removed.join(', ')})` : ''}${kept}`; + const list = removed.length > 0 ? ` (${removed.join(', ')})` : ''; + return `✓ Agent skill removed the ${RELAY_SKILL_NAME} skill relay init installed${list}${kept}`; +} + +/** Put the lock entry back as init found it, while it still names the ref init wrote. */ +function restoreLock(record: SkillLockRecord): void { + const lock = readLock(record.file); + if (!lock.skills || lock.skills[RELAY_SKILL_NAME]?.ref !== record.ref) return; + if (record.original) lock.skills[RELAY_SKILL_NAME] = record.original as { ref?: string }; + else delete lock.skills[RELAY_SKILL_NAME]; + if (record.created && Object.keys(lock.skills).length === 0) fs.rmSync(record.file, { force: true }); + else fs.writeFileSync(record.file, JSON.stringify(lock, null, 2)); } function toolLabel(id: ToolId): string { From 41022d66719297dc7371991245cad89adc3d68d2 Mon Sep 17 00:00:00 2001 From: kjgbot Date: Fri, 30 Oct 2026 18:07:11 -0700 Subject: [PATCH 05/11] fix(init): keep recording skill paths an --uninstall could not reach An --uninstall run without the CLAUDE_CONFIG_DIR, GROK_HOME or XDG overrides that relay init ran with resolves the tools' folders elsewhere, skipped the recorded copies and lock, and still dropped the record. Those paths now stay recorded, and the line names them and the settings to rerun --uninstall with. Co-Authored-By: Claude Opus 5.5 --- packages/cli/src/cli/commands/init.test.ts | 27 ++++++++++++ packages/cli/src/cli/commands/init.ts | 11 +++-- packages/cli/src/cli/lib/relay-skill.ts | 49 ++++++++++++++++++---- 3 files changed, 74 insertions(+), 13 deletions(-) diff --git a/packages/cli/src/cli/commands/init.test.ts b/packages/cli/src/cli/commands/init.test.ts index a77536247..7d04f484b 100644 --- a/packages/cli/src/cli/commands/init.test.ts +++ b/packages/cli/src/cli/commands/init.test.ts @@ -1973,6 +1973,33 @@ describe('relay init messaging skill', () => { expect(fs.readFileSync(path.join(toolHome, 'xdg', 'cache', 'keep'), 'utf8')).toBe('theirs'); }); + it('keeps recording what an --uninstall in another environment could not reach', async () => { + installedProbe(); + const core = fakeCore(); + const skills = fakeSkillsCli(); + const claudeDir = path.join(toolHome, 'claude-config'); + skills.dirs['claude-code'] = path.join(claudeDir, 'skills'); + const withDir = { projectRoot: project, home: toolHome, env: { PATH: '', CLAUDE_CONFIG_DIR: claudeDir } }; + await setup({ ...core.deps, runSkillsCli: skills.run, toolContext: () => withDir }).run( + '--tools', + 'claude' + ); + const copy = path.join(claudeDir, 'skills', RELAY_SKILL_NAME, 'SKILL.md'); + expect(fs.existsSync(copy)).toBe(true); + + // Without CLAUDE_CONFIG_DIR this environment cannot reach that copy. + const plain = setup(core.deps); + await plain.run('--uninstall'); + expect(fs.existsSync(copy)).toBe(true); + expect(plain.logs.join('\n')).toContain('Run relay init --uninstall again with the CLAUDE_CONFIG_DIR'); + const record = JSON.parse(fs.readFileSync(path.join(home, 'local', 'core-settings.json'), 'utf8')); + expect(record.relaySkill.copies.map((c: { path: string }) => c.path)).toEqual([path.dirname(copy)]); + + await setup({ ...core.deps, toolContext: () => withDir }).run('--uninstall'); + expect(fs.existsSync(copy)).toBe(false); + expect(fs.existsSync(path.join(home, 'local', 'core-settings.json'))).toBe(false); + }); + it('keeps a copy with a file added after init on --uninstall', async () => { installedProbe(); const core = fakeCore(); diff --git a/packages/cli/src/cli/commands/init.ts b/packages/cli/src/cli/commands/init.ts index c986b6797..16bda3454 100644 --- a/packages/cli/src/cli/commands/init.ts +++ b/packages/cli/src/cli/commands/init.ts @@ -1575,11 +1575,14 @@ async function uninstall(deps: InitCommandDependencies, o: InitOptions): Promise } // The skill is installed per user, like injection, and removed with it. const skillRecord = readDeliveryRecord(deliveryRecordPath()).relaySkill; - const skillLine = + const skill = o.scope !== 'project' && o.tools === undefined ? uninstallRelaySkill(skillRecord, ctx) : undefined; - if (skillLine) { - deps.log(skillLine); - updateDeliveryRecord(({ relaySkill: _removed, ...rest }) => rest); + if (skill) { + deps.log(skill.line); + // Paths this environment could not reach stay recorded for a later --uninstall. + updateDeliveryRecord(({ relaySkill: _removed, ...rest }) => + skill.remaining ? { ...rest, relaySkill: skill.remaining } : rest + ); } if (shown.length === 0 && turnedOff.length === 0 && !injectionKept && !skillRecord) { deps.log('relay init had not added Relay to any agent tool here; nothing to remove.'); diff --git a/packages/cli/src/cli/lib/relay-skill.ts b/packages/cli/src/cli/lib/relay-skill.ts index 0c572059a..395ca9e04 100644 --- a/packages/cli/src/cli/lib/relay-skill.ts +++ b/packages/cli/src/cli/lib/relay-skill.ts @@ -383,25 +383,39 @@ function failureLine(tools: string, reason: string): string { ); } +/** What `--uninstall` did with the skill, and what is still recorded. */ +export interface SkillUninstallResult { + line: string; + /** + * The paths it could not reach in this environment (recorded under + * another `CLAUDE_CONFIG_DIR`, `GROK_HOME`, `XDG_CONFIG_HOME` or + * `XDG_STATE_HOME`), kept for a later `--uninstall`; undefined when none are left. + */ + remaining: RelaySkillRecord | undefined; +} + /** * Remove what init installed: each recorded copy whose every file is as * installed, the lock entry it wrote (putting back the one it replaced), * and the folders it created once they are empty. Only the skill's own - * folder in a tool's skills folder and the `skills` CLI's lock file are - * ever touched, whatever the record says. + * folder in a tool's skills folder and the `skills` CLI's lock file, as this + * environment resolves them, are ever touched, whatever the record says. + * Recorded paths outside them stay recorded, and the line says how to + * remove them. * @param recorded - What init installed * @param ctx - Home and environment the tools resolve against - * @returns The line to print, or undefined when nothing was recorded + * @returns The line to print and the record to keep, or undefined when nothing was recorded */ export function uninstallRelaySkill( recorded: RelaySkillRecord | undefined, ctx: ToolContext -): string | undefined { +): SkillUninstallResult | undefined { if (!recorded) return undefined; const allowed = new Set(TOOL_IDS.map((id) => skillDir(id, ctx))); const lock = lockFile(ctx); const removed: string[] = []; const changed: string[] = []; + const elsewhere = recorded.copies.filter((copy) => !allowed.has(copy.path)); for (const copy of recorded.copies) { if (!allowed.has(copy.path)) continue; const hash = treeHash(copy.path); @@ -413,14 +427,31 @@ export function uninstallRelaySkill( fs.rmSync(copy.path, { recursive: true, force: true }); removed.push(copy.path); } - if (recorded.lock?.file === lock) restoreLock(recorded.lock); + const lockElsewhere = recorded.lock && recorded.lock.file !== lock ? recorded.lock : undefined; + if (recorded.lock && !lockElsewhere) restoreLock(recorded.lock); const roots = [...allowed, lock]; - removeEmptyDirs( - recorded.createdDirs.filter((dir) => roots.some((root) => root.startsWith(dir + path.sep))) - ); + const reachable = (dir: string) => roots.some((root) => root.startsWith(dir + path.sep)); + removeEmptyDirs(recorded.createdDirs.filter(reachable)); + const remaining: RelaySkillRecord | undefined = + elsewhere.length > 0 || lockElsewhere + ? { + copies: elsewhere, + createdDirs: recorded.createdDirs.filter((dir) => !reachable(dir) || exists(dir)), + ...(lockElsewhere ? { lock: lockElsewhere } : {}), + } + : undefined; const kept = changed.length > 0 ? `; kept ${changed.join(', ')}, changed since` : ''; const list = removed.length > 0 ? ` (${removed.join(', ')})` : ''; - return `✓ Agent skill removed the ${RELAY_SKILL_NAME} skill relay init installed${list}${kept}`; + const left = remaining + ? `. Not removed, because this environment resolves the tools' folders elsewhere: ` + + `${[...elsewhere.map((copy) => copy.path), ...(lockElsewhere ? [lockElsewhere.file] : [])].join(', ')}. ` + + 'Run relay init --uninstall again with the CLAUDE_CONFIG_DIR, GROK_HOME, XDG_CONFIG_HOME and ' + + 'XDG_STATE_HOME that relay init ran with to remove them' + : ''; + return { + line: `${remaining ? '!' : '✓'} Agent skill removed the ${RELAY_SKILL_NAME} skill relay init installed${list}${kept}${left}`, + remaining, + }; } /** Put the lock entry back as init found it, while it still names the ref init wrote. */ From c431310e2348b55f1c76d95b303bdf2ad295ea67 Mon Sep 17 00:00:00 2001 From: kjgbot Date: Fri, 30 Oct 2026 21:47:19 -0700 Subject: [PATCH 06/11] fix(init): track lock files separately and contain skill uninstall failures - Record one lock entry per lock file, so an init under a different XDG_STATE_HOME no longer loses the entry it wrote there. - When the record cannot be saved, take back out what this run created, so no copy is left that --uninstall would not know about. - A failed removal no longer stops the rest of uninstall. It stays recorded and is reported apart from paths in another environment. - Test fake reads the lock without an existsSync check (CodeQL js/file-system-race). Co-Authored-By: Claude Opus 5.5 --- packages/cli/src/cli/commands/init.test.ts | 86 +++++++++++- packages/cli/src/cli/commands/init.ts | 24 ++-- packages/cli/src/cli/lib/relay-skill.ts | 155 +++++++++++++-------- 3 files changed, 187 insertions(+), 78 deletions(-) diff --git a/packages/cli/src/cli/commands/init.test.ts b/packages/cli/src/cli/commands/init.test.ts index 7d04f484b..919b48414 100644 --- a/packages/cli/src/cli/commands/init.test.ts +++ b/packages/cli/src/cli/commands/init.test.ts @@ -1834,7 +1834,9 @@ function fakeSkillsCli(status = 0, lockFile?: string) { }; const lock = lockFile ?? path.join(toolHome, '.agents', '.skill-lock.json'); const state = { status }; - const run = vi.fn((args) => { + const lockIn = (env: NodeJS.ProcessEnv) => + env.XDG_STATE_HOME ? path.join(env.XDG_STATE_HOME, 'skills', '.skill-lock.json') : lock; + const run = vi.fn((args, options) => { if (state.status !== 0) return { status: state.status, output: 'fatal: unable to access github.com' }; args.forEach((arg, i) => { if (args[i - 1] !== '--agent') return; @@ -1842,12 +1844,15 @@ function fakeSkillsCli(status = 0, lockFile?: string) { fs.mkdirSync(dir, { recursive: true }); fs.writeFileSync(path.join(dir, 'SKILL.md'), '---\nname: messaging-agents-on-the-relay\n---\n'); }); - const current = fs.existsSync(lock) - ? JSON.parse(fs.readFileSync(lock, 'utf8')) - : { version: 3, skills: {} }; + let current = { version: 3, skills: {} as Record }; + try { + current = JSON.parse(fs.readFileSync(lockIn(options.env), 'utf8')); + } catch { + // No lock file yet. + } current.skills[RELAY_SKILL_NAME] = { source: 'AgentWorkforce/skills', ref: RELAY_SKILL_REF }; - fs.mkdirSync(path.dirname(lock), { recursive: true }); - fs.writeFileSync(lock, JSON.stringify(current)); + fs.mkdirSync(path.dirname(lockIn(options.env)), { recursive: true }); + fs.writeFileSync(lockIn(options.env), JSON.stringify(current)); return { status: 0, output: 'Done!' }; }); return { run, lock, dirs, fail: (next: number) => (state.status = next) }; @@ -1911,7 +1916,7 @@ describe('relay init messaging skill', () => { { path: path.join(skills.dirs['claude-code']!, RELAY_SKILL_NAME), ref: RELAY_SKILL_REF }, { path: path.join(skills.dirs.codex!, RELAY_SKILL_NAME), ref: RELAY_SKILL_REF }, ], - lock: { file: skills.lock, created: true, original: null, ref: RELAY_SKILL_REF }, + locks: [{ file: skills.lock, created: true, original: null, ref: RELAY_SKILL_REF }], }); // A re-run with the same pin changes nothing and fetches nothing. @@ -2000,6 +2005,73 @@ describe('relay init messaging skill', () => { expect(fs.existsSync(path.join(home, 'local', 'core-settings.json'))).toBe(false); }); + it('tracks each lock file it wrote when the state folder differs between runs', async () => { + installedProbe(); + const core = fakeCore(); + const skills = fakeSkillsCli(); + const under = (name: string) => { + const state = path.join(toolHome, name); + return { + lock: path.join(state, 'skills', '.skill-lock.json'), + ctx: { projectRoot: project, home: toolHome, env: { PATH: '', XDG_STATE_HOME: state } }, + }; + }; + const a = under('state-a'); + const b = under('state-b'); + await setup({ ...core.deps, runSkillsCli: skills.run, toolContext: () => a.ctx }).run('--tools', 'codex'); + await setup({ ...core.deps, toolContext: () => b.ctx }).run('--uninstall'); + expect(fs.existsSync(a.lock)).toBe(true); + + await setup({ ...core.deps, runSkillsCli: skills.run, toolContext: () => b.ctx }).run('--tools', 'codex'); + await setup({ ...core.deps, toolContext: () => b.ctx }).run('--uninstall'); + expect(fs.existsSync(b.lock)).toBe(false); + expect(fs.existsSync(a.lock)).toBe(true); + + await setup({ ...core.deps, toolContext: () => a.ctx }).run('--uninstall'); + expect(fs.existsSync(a.lock)).toBe(false); + expect(fs.existsSync(path.join(home, 'local', 'core-settings.json'))).toBe(false); + }); + + it("takes this run's copies back out when the record cannot be saved", async () => { + installedProbe(); + const core = fakeCore(); + const skills = fakeSkillsCli(); + const settings = path.join(home, 'local', 'core-settings.json'); + // A folder where the record goes: the next save fails. + const blocked: RunSkillsCli = (args, options) => { + const result = skills.run(args, options); + fs.rmSync(settings, { force: true }); + fs.mkdirSync(settings); + return result; + }; + const { run, logs } = setup({ ...core.deps, runSkillsCli: blocked }); + await expect(run('--tools', 'codex')).resolves.toBeDefined(); + expect(logs.join('\n')).toContain("so this run's copies were taken out again"); + expect(fs.existsSync(path.join(toolHome, '.agents'))).toBe(false); + }); + + it('removes the other copies and keeps recording one it could not remove', async () => { + installedProbe(); + const core = fakeCore(); + const skills = fakeSkillsCli(); + await setup({ ...core.deps, runSkillsCli: skills.run }).run('--tools', 'claude,codex'); + const stuck = path.join(skills.dirs['claude-code']!, RELAY_SKILL_NAME); + const other = path.join(skills.dirs.codex!, RELAY_SKILL_NAME); + const rmSync = fs.rmSync; + const spy = vi.spyOn(fs, 'rmSync').mockImplementation((target, options) => { + if (target === stuck) throw new Error('EACCES'); + rmSync(target, options); + }); + const { run, logs } = setup(core.deps); + await expect(run('--uninstall')).resolves.toBeDefined(); + spy.mockRestore(); + expect(fs.existsSync(stuck)).toBe(true); + expect(fs.existsSync(other)).toBe(false); + expect(logs.join('\n')).toContain(`Could not remove ${stuck}`); + await setup(core.deps).run('--uninstall'); + expect(fs.existsSync(stuck)).toBe(false); + }); + it('keeps a copy with a file added after init on --uninstall', async () => { installedProbe(); const core = fakeCore(); diff --git a/packages/cli/src/cli/commands/init.ts b/packages/cli/src/cli/commands/init.ts index 16bda3454..7bd143ce7 100644 --- a/packages/cli/src/cli/commands/init.ts +++ b/packages/cli/src/cli/commands/init.ts @@ -1394,23 +1394,25 @@ function installSkill( ctx: ToolContext ): string | undefined { if (injected.length === 0) return undefined; - let line: string; + const result = installRelaySkill( + injected, + ctx, + readDeliveryRecord(deliveryRecordPath()).relaySkill, + deps.runSkillsCli + ); try { - const result = installRelaySkill( - injected, - ctx, - readDeliveryRecord(deliveryRecordPath()).relaySkill, - deps.runSkillsCli - ); - line = result.line; updateDeliveryRecord(({ relaySkill: _previous, ...rest }) => result.record ? { ...rest, relaySkill: result.record } : rest ); } catch (error) { - // The skill is optional: nothing about it may fail init. - return `! Agent skill could not be installed or recorded (${(error as Error).message})`; + // The skill is optional, but what --uninstall would not know about is not left behind. + uninstallRelaySkill(result.undo, ctx); + return ( + `! Agent skill not installed: ${deliveryRecordPath()} could not be saved ` + + `(${(error as Error).message}), so this run's copies were taken out again` + ); } - return line; + return result.line; } /** diff --git a/packages/cli/src/cli/lib/relay-skill.ts b/packages/cli/src/cli/lib/relay-skill.ts index 395ca9e04..45677303e 100644 --- a/packages/cli/src/cli/lib/relay-skill.ts +++ b/packages/cli/src/cli/lib/relay-skill.ts @@ -63,8 +63,8 @@ export interface RelaySkillRecord { copies: InstalledSkillCopy[]; /** Folders init's install created, removed again once empty. */ createdDirs: string[]; - /** The `skills` CLI's lock entry, when init wrote it. */ - lock?: SkillLockRecord; + /** The `skills` CLI's lock entries init wrote, one per lock file. */ + locks: SkillLockRecord[]; } /** Runs the `skills` CLI with `args`; replaced in tests. */ @@ -230,19 +230,22 @@ export function relaySkillRecordOf(value: unknown): RelaySkillRecord | undefined const createdDirs = Array.isArray(record.createdDirs) ? record.createdDirs.filter((dir): dir is string => typeof dir === 'string') : []; - const raw = record.lock as Partial | undefined; - const lock = - raw && typeof raw.file === 'string' && typeof raw.ref === 'string' - ? { file: raw.file, created: raw.created === true, original: raw.original ?? null, ref: raw.ref } - : undefined; - if (copies.length === 0 && !lock) return undefined; - return { copies, createdDirs, ...(lock ? { lock } : {}) }; + const locks = (Array.isArray(record.locks) ? (record.locks as Partial[]) : []).flatMap( + (raw) => + typeof raw?.file === 'string' && typeof raw?.ref === 'string' + ? [{ file: raw.file, created: raw.created === true, original: raw.original ?? null, ref: raw.ref }] + : [] + ); + if (copies.length === 0 && locks.length === 0) return undefined; + return { copies, createdDirs, locks }; } /** One line about the skill, and the record to keep. */ export interface SkillInstallResult { line: string; record: RelaySkillRecord | undefined; + /** Only what this run newly created: what to take back out if `record` cannot be saved. */ + undo?: RelaySkillRecord; } /** @@ -291,19 +294,23 @@ function plan( return { before, kept }; } -/** The lock entry to record after a run: kept from an earlier run, or the one this run wrote. */ -function lockAfter( - recorded: SkillLockRecord | undefined, - file: string, - existed: boolean, - entryBefore: unknown -): SkillLockRecord | undefined { - const entryAfter = readLock(file).skills?.[RELAY_SKILL_NAME]; - const wrote = - entryAfter?.ref === RELAY_SKILL_REF && - (entryBefore as { ref?: string } | undefined)?.ref !== RELAY_SKILL_REF; - if (recorded) return wrote ? { ...recorded, ref: RELAY_SKILL_REF } : recorded; - return wrote ? { file, created: !existed, original: entryBefore ?? null, ref: RELAY_SKILL_REF } : undefined; +/** The lock entry this run wrote, as `--uninstall` would need it; undefined when it wrote none. */ +function lockWritten(file: string, existed: boolean, entryBefore: unknown): SkillLockRecord | undefined { + const before = (entryBefore as { ref?: string } | undefined)?.ref; + if (readLock(file).skills?.[RELAY_SKILL_NAME]?.ref !== RELAY_SKILL_REF || before === RELAY_SKILL_REF) { + return undefined; + } + return { file, created: !existed, original: entryBefore ?? null, ref: RELAY_SKILL_REF }; +} + +/** The recorded lock entries after `written`: an earlier entry for its file keeps what init first found. */ +function mergeLocks(recorded: SkillLockRecord[], written: SkillLockRecord | undefined): SkillLockRecord[] { + if (!written) return recorded; + const earlier = recorded.find((entry) => entry.file === written.file); + return [ + ...recorded.filter((entry) => entry.file !== written.file), + earlier ? { ...earlier, ref: written.ref } : written, + ]; } function install( @@ -354,25 +361,29 @@ function install( ...(recorded?.copies ?? []).filter((copy) => !landed.includes(copy.path)), ...landed.map((dir) => ({ path: dir, ref: RELAY_SKILL_REF, treeSha256: after.get(dir)! })), ]; - const lockRecord = lockAfter(recorded?.lock, lock, lockExisted, entryBefore); + const written = lockWritten(lock, lockExisted, entryBefore); + const locks = mergeLocks(recorded?.locks ?? [], written); + const made = createdDirs.filter(exists); const record: RelaySkillRecord | undefined = - copies.length > 0 || lockRecord - ? { - copies, - createdDirs: [...new Set([...(recorded?.createdDirs ?? []), ...createdDirs.filter(exists)])], - ...(lockRecord ? { lock: lockRecord } : {}), - } + copies.length > 0 || locks.length > 0 + ? { copies, createdDirs: [...new Set([...(recorded?.createdDirs ?? []), ...made])], locks } : undefined; + const undo: RelaySkillRecord = { + copies: copies.filter((copy) => landed.includes(copy.path) && before.get(copy.path) === undefined), + createdDirs: made, + locks: written ? [written] : [], + }; const missed = targets.filter((dir) => !landed.includes(dir)); if (missed.length > 0) { const reason = result.status === 0 ? 'it did not land' : lastLine(result.output) || `exit ${result.status}`; - return { line: failureLine(label(missed), reason), record }; + return { line: failureLine(label(missed), reason), record, undo }; } const keptNote = kept.length > 0 ? `; already there for ${label(kept)}, left as is` : ''; return { line: `✓ Agent skill installed ${RELAY_SKILL_NAME} for ${label(landed)}${keptNote}`, record, + undo, }; } @@ -387,21 +398,22 @@ function failureLine(tools: string, reason: string): string { export interface SkillUninstallResult { line: string; /** - * The paths it could not reach in this environment (recorded under - * another `CLAUDE_CONFIG_DIR`, `GROK_HOME`, `XDG_CONFIG_HOME` or - * `XDG_STATE_HOME`), kept for a later `--uninstall`; undefined when none are left. + * What it could not remove, kept for a later `--uninstall`: paths this + * environment resolves elsewhere (another `CLAUDE_CONFIG_DIR`, `GROK_HOME`, + * `XDG_CONFIG_HOME` or `XDG_STATE_HOME`) and paths that failed. Undefined + * when nothing is left. */ remaining: RelaySkillRecord | undefined; } /** * Remove what init installed: each recorded copy whose every file is as - * installed, the lock entry it wrote (putting back the one it replaced), + * installed, the lock entries it wrote (putting back what they replaced), * and the folders it created once they are empty. Only the skill's own * folder in a tool's skills folder and the `skills` CLI's lock file, as this * environment resolves them, are ever touched, whatever the record says. - * Recorded paths outside them stay recorded, and the line says how to - * remove them. + * Never throws: what it could not reach or remove stays recorded, and the + * line says so. * @param recorded - What init installed * @param ctx - Home and environment the tools resolve against * @returns The line to print and the record to keep, or undefined when nothing was recorded @@ -415,6 +427,7 @@ export function uninstallRelaySkill( const lock = lockFile(ctx); const removed: string[] = []; const changed: string[] = []; + const failed: InstalledSkillCopy[] = []; const elsewhere = recorded.copies.filter((copy) => !allowed.has(copy.path)); for (const copy of recorded.copies) { if (!allowed.has(copy.path)) continue; @@ -424,44 +437,66 @@ export function uninstallRelaySkill( changed.push(copy.path); continue; } - fs.rmSync(copy.path, { recursive: true, force: true }); - removed.push(copy.path); + try { + fs.rmSync(copy.path, { recursive: true, force: true }); + removed.push(copy.path); + } catch { + failed.push(copy); + } } - const lockElsewhere = recorded.lock && recorded.lock.file !== lock ? recorded.lock : undefined; - if (recorded.lock && !lockElsewhere) restoreLock(recorded.lock); + const locksElsewhere = recorded.locks.filter((entry) => entry.file !== lock); + const locksFailed = recorded.locks.filter((entry) => entry.file === lock && !restoreLock(entry)); const roots = [...allowed, lock]; - const reachable = (dir: string) => roots.some((root) => root.startsWith(dir + path.sep)); - removeEmptyDirs(recorded.createdDirs.filter(reachable)); + removeEmptyDirs( + recorded.createdDirs.filter((dir) => roots.some((root) => root.startsWith(dir + path.sep))) + ); + const left = [...elsewhere, ...failed]; + const leftLocks = [...locksElsewhere, ...locksFailed]; const remaining: RelaySkillRecord | undefined = - elsewhere.length > 0 || lockElsewhere + left.length > 0 || leftLocks.length > 0 ? { - copies: elsewhere, - createdDirs: recorded.createdDirs.filter((dir) => !reachable(dir) || exists(dir)), - ...(lockElsewhere ? { lock: lockElsewhere } : {}), + copies: left, + createdDirs: recorded.createdDirs.filter((dir) => fs.existsSync(dir)), + locks: leftLocks, } : undefined; - const kept = changed.length > 0 ? `; kept ${changed.join(', ')}, changed since` : ''; - const list = removed.length > 0 ? ` (${removed.join(', ')})` : ''; - const left = remaining - ? `. Not removed, because this environment resolves the tools' folders elsewhere: ` + - `${[...elsewhere.map((copy) => copy.path), ...(lockElsewhere ? [lockElsewhere.file] : [])].join(', ')}. ` + - 'Run relay init --uninstall again with the CLAUDE_CONFIG_DIR, GROK_HOME, XDG_CONFIG_HOME and ' + - 'XDG_STATE_HOME that relay init ran with to remove them' - : ''; + const notes = [ + ...(removed.length > 0 ? [` (${removed.join(', ')})`] : []), + ...(changed.length > 0 ? [`; kept ${changed.join(', ')}, changed since`] : []), + ]; + const away = [...elsewhere.map((copy) => copy.path), ...locksElsewhere.map((entry) => entry.file)]; + if (away.length > 0) { + notes.push( + `. Not removed, because this environment resolves the tools' folders elsewhere: ${away.join(', ')}. ` + + 'Run relay init --uninstall again with the CLAUDE_CONFIG_DIR, GROK_HOME, XDG_CONFIG_HOME and ' + + 'XDG_STATE_HOME that relay init ran with to remove them' + ); + } + const stuck = [...failed.map((copy) => copy.path), ...locksFailed.map((entry) => entry.file)]; + if (stuck.length > 0) + notes.push(`. Could not remove ${stuck.join(', ')}; run relay init --uninstall again`); return { - line: `${remaining ? '!' : '✓'} Agent skill removed the ${RELAY_SKILL_NAME} skill relay init installed${list}${kept}${left}`, + line: `${remaining ? '!' : '✓'} Agent skill removed the ${RELAY_SKILL_NAME} skill relay init installed${notes.join('')}`, remaining, }; } -/** Put the lock entry back as init found it, while it still names the ref init wrote. */ -function restoreLock(record: SkillLockRecord): void { +/** + * Put the lock entry back as init found it, while it still names the ref init wrote. + * @returns false when the lock file could not be changed + */ +function restoreLock(record: SkillLockRecord): boolean { const lock = readLock(record.file); - if (!lock.skills || lock.skills[RELAY_SKILL_NAME]?.ref !== record.ref) return; + if (!lock.skills || lock.skills[RELAY_SKILL_NAME]?.ref !== record.ref) return true; if (record.original) lock.skills[RELAY_SKILL_NAME] = record.original as { ref?: string }; else delete lock.skills[RELAY_SKILL_NAME]; - if (record.created && Object.keys(lock.skills).length === 0) fs.rmSync(record.file, { force: true }); - else fs.writeFileSync(record.file, JSON.stringify(lock, null, 2)); + try { + if (record.created && Object.keys(lock.skills).length === 0) fs.rmSync(record.file, { force: true }); + else fs.writeFileSync(record.file, JSON.stringify(lock, null, 2)); + return true; + } catch { + return false; + } } function toolLabel(id: ToolId): string { From c30996cdf7bdc1187dcaa12cc28fb31c2d434046 Mon Sep 17 00:00:00 2001 From: kjgbot Date: Fri, 30 Oct 2026 22:06:39 -0700 Subject: [PATCH 07/11] fix(init): pin the messaging skill to skills#195's head with the MCP-first step Co-Authored-By: Claude Opus 5.5 --- packages/cli/src/cli/lib/relay-skill.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cli/src/cli/lib/relay-skill.ts b/packages/cli/src/cli/lib/relay-skill.ts index 45677303e..c9c6f910b 100644 --- a/packages/cli/src/cli/lib/relay-skill.ts +++ b/packages/cli/src/cli/lib/relay-skill.ts @@ -21,7 +21,7 @@ import { TOOL_IDS, findToolTarget, type ToolContext, type ToolId } from './agent /** The skill's name in AgentWorkforce/skills. */ export const RELAY_SKILL_NAME = 'messaging-agents-on-the-relay'; /** The skills repo commit init installs the skill from. */ -export const RELAY_SKILL_REF = '46f8581307f79e8183c0dd2a5fd9847d68d46ee3'; +export const RELAY_SKILL_REF = '7615fcf3d37ee9e83d9f07a9894993e611bfd944'; /** The pinned source, as `skills add` takes it (`#`). */ export const RELAY_SKILL_SOURCE = `https://github.com/AgentWorkforce/skills#${RELAY_SKILL_REF}`; /** The `skills` CLI version init runs through npx. */ From 00994379f13df864ed8f94fbfe0672cbe2f22b78 Mon Sep 17 00:00:00 2001 From: kjgbot Date: Fri, 30 Oct 2026 22:07:26 -0700 Subject: [PATCH 08/11] fix(init): replace the skills lock atomically and report a failed rollback Co-Authored-By: Claude Opus 5.5 --- packages/cli/src/cli/commands/init.test.ts | 38 ++++++++++++++++++++++ packages/cli/src/cli/commands/init.ts | 11 +++++-- packages/cli/src/cli/lib/relay-skill.ts | 14 +++++++- 3 files changed, 60 insertions(+), 3 deletions(-) diff --git a/packages/cli/src/cli/commands/init.test.ts b/packages/cli/src/cli/commands/init.test.ts index 919b48414..08d6896f8 100644 --- a/packages/cli/src/cli/commands/init.test.ts +++ b/packages/cli/src/cli/commands/init.test.ts @@ -2050,6 +2050,44 @@ describe('relay init messaging skill', () => { expect(fs.existsSync(path.join(toolHome, '.agents'))).toBe(false); }); + it('names what it could not take back out when the record cannot be saved', async () => { + installedProbe(); + const core = fakeCore(); + const skills = fakeSkillsCli(); + const settings = path.join(home, 'local', 'core-settings.json'); + const copy = path.join(skills.dirs.codex!, RELAY_SKILL_NAME); + const rmSync = fs.rmSync; + const spy = vi.spyOn(fs, 'rmSync').mockImplementation((target, options) => { + if (target === copy) throw new Error('EACCES'); + rmSync(target, options); + }); + const blocked: RunSkillsCli = (args, options) => { + const result = skills.run(args, options); + rmSync(settings, { force: true }); + fs.mkdirSync(settings); + return result; + }; + const { run, logs } = setup({ ...core.deps, runSkillsCli: blocked }); + await run('--tools', 'codex'); + spy.mockRestore(); + expect(logs.join('\n')).toContain(`could not be taken out again, so remove them by hand: ${copy}`); + expect(logs.join('\n')).not.toContain("this run's copies were taken out again"); + }); + + it('replaces a shared lock file in one step, keeping its mode and leaving no temporary file', async () => { + installedProbe(); + const lock = path.join(toolHome, '.agents', '.skill-lock.json'); + fs.mkdirSync(path.dirname(lock), { recursive: true }); + fs.writeFileSync(lock, JSON.stringify({ version: 3, skills: { other: { ref: 'x' } } }), { mode: 0o640 }); + fs.chmodSync(lock, 0o640); + const core = fakeCore(); + const skills = fakeSkillsCli(); + await setup({ ...core.deps, runSkillsCli: skills.run }).run('--tools', 'codex'); + await setup(core.deps).run('--uninstall'); + expect(fs.statSync(lock).mode & 0o777).toBe(0o640); + expect(fs.readdirSync(path.dirname(lock))).toEqual(['.skill-lock.json']); + }); + it('removes the other copies and keeps recording one it could not remove', async () => { installedProbe(); const core = fakeCore(); diff --git a/packages/cli/src/cli/commands/init.ts b/packages/cli/src/cli/commands/init.ts index 7bd143ce7..d9006280e 100644 --- a/packages/cli/src/cli/commands/init.ts +++ b/packages/cli/src/cli/commands/init.ts @@ -1406,10 +1406,17 @@ function installSkill( ); } catch (error) { // The skill is optional, but what --uninstall would not know about is not left behind. - uninstallRelaySkill(result.undo, ctx); + const undone = uninstallRelaySkill(result.undo, ctx); + const left = undone?.remaining; + const outcome = left + ? `and these could not be taken out again, so remove them by hand: ${[ + ...left.copies.map((copy) => copy.path), + ...left.locks.map((lock) => lock.file), + ].join(', ')}` + : "so this run's copies were taken out again"; return ( `! Agent skill not installed: ${deliveryRecordPath()} could not be saved ` + - `(${(error as Error).message}), so this run's copies were taken out again` + `(${(error as Error).message}), ${outcome}` ); } return result.line; diff --git a/packages/cli/src/cli/lib/relay-skill.ts b/packages/cli/src/cli/lib/relay-skill.ts index c9c6f910b..4d31b2978 100644 --- a/packages/cli/src/cli/lib/relay-skill.ts +++ b/packages/cli/src/cli/lib/relay-skill.ts @@ -492,13 +492,25 @@ function restoreLock(record: SkillLockRecord): boolean { else delete lock.skills[RELAY_SKILL_NAME]; try { if (record.created && Object.keys(lock.skills).length === 0) fs.rmSync(record.file, { force: true }); - else fs.writeFileSync(record.file, JSON.stringify(lock, null, 2)); + else writeAtomically(record.file, JSON.stringify(lock, null, 2)); return true; } catch { return false; } } +/** Replace `file` in one step, keeping its mode: the lock is shared with the person's own `skills` runs. */ +function writeAtomically(file: string, text: string): void { + const temporary = `${file}.${process.pid}.tmp`; + try { + fs.writeFileSync(temporary, text, { mode: fs.statSync(file).mode & 0o777 }); + fs.renameSync(temporary, file); + } catch (error) { + fs.rmSync(temporary, { force: true }); + throw error; + } +} + function toolLabel(id: ToolId): string { return findToolTarget(id)?.label ?? id; } From f3f11f471de485695ce6aafb6bfa84729aba11e9 Mon Sep 17 00:00:00 2001 From: kjgbot Date: Fri, 30 Oct 2026 23:05:30 -0700 Subject: [PATCH 09/11] fix(init): keep skill records across parallel runs and failed saves - Each install now saves only what it changed, applied to the record as it is at save time, so a parallel relay init's copies stay recorded. - A refreshed copy is backed up first and put back if the record cannot be saved. If the backup fails, the copy is not changed. - Uninstall tells a missing copy or lock from one it cannot read. An unreadable one stays recorded and is reported. Co-Authored-By: Claude Opus 5.5 --- packages/cli/src/cli/commands/init.test.ts | 91 +++++++++++++- packages/cli/src/cli/commands/init.ts | 32 +++-- packages/cli/src/cli/lib/relay-skill.ts | 137 ++++++++++++++++++--- 3 files changed, 224 insertions(+), 36 deletions(-) diff --git a/packages/cli/src/cli/commands/init.test.ts b/packages/cli/src/cli/commands/init.test.ts index d30ecd2b9..1e0bbf68a 100644 --- a/packages/cli/src/cli/commands/init.test.ts +++ b/packages/cli/src/cli/commands/init.test.ts @@ -1981,7 +1981,7 @@ function fakeSkillsCli(status = 0, lockFile?: string) { devin: path.join(toolHome, '.config', 'devin', 'skills'), }; const lock = lockFile ?? path.join(toolHome, '.agents', '.skill-lock.json'); - const state = { status }; + const state = { status, content: '---\nname: messaging-agents-on-the-relay\n---\n' }; const lockIn = (env: NodeJS.ProcessEnv) => env.XDG_STATE_HOME ? path.join(env.XDG_STATE_HOME, 'skills', '.skill-lock.json') : lock; const run = vi.fn((args, options) => { @@ -1990,7 +1990,7 @@ function fakeSkillsCli(status = 0, lockFile?: string) { if (args[i - 1] !== '--agent') return; const dir = path.join(dirs[arg]!, RELAY_SKILL_NAME); fs.mkdirSync(dir, { recursive: true }); - fs.writeFileSync(path.join(dir, 'SKILL.md'), '---\nname: messaging-agents-on-the-relay\n---\n'); + fs.writeFileSync(path.join(dir, 'SKILL.md'), state.content); }); let current = { version: 3, skills: {} as Record }; try { @@ -2003,7 +2003,13 @@ function fakeSkillsCli(status = 0, lockFile?: string) { fs.writeFileSync(lockIn(options.env), JSON.stringify(current)); return { status: 0, output: 'Done!' }; }); - return { run, lock, dirs, fail: (next: number) => (state.status = next) }; + return { + run, + lock, + dirs, + fail: (next: number) => (state.status = next), + write: (content: string) => (state.content = content), + }; } /** Rewrite the recorded skill install, as an older init or a forged file would have left it. */ @@ -2302,6 +2308,85 @@ describe('relay init messaging skill', () => { expect(fs.statSync(lock).mode & 0o777).toBe(0o640); }); + it('keeps the copies a parallel relay init recorded while it ran', async () => { + installedProbe(); + const core = fakeCore(); + const skills = fakeSkillsCli(); + await setup({ ...core.deps, runSkillsCli: skills.run }).run('--tools', 'codex'); + const elsewhere = { + path: path.join(toolHome, '.grok', 'skills', RELAY_SKILL_NAME), + ref: 'r', + treeSha256: 't', + }; + const parallel: RunSkillsCli = (args, options) => { + editSkillRecord((record) => record.copies.push(elsewhere)); + return skills.run(args, options); + }; + await setup({ ...core.deps, runSkillsCli: parallel }).run('--tools', 'claude'); + const saved = JSON.parse(fs.readFileSync(path.join(home, 'local', 'core-settings.json'), 'utf8')); + expect(saved.relaySkill.copies.map((copy: { path: string }) => copy.path)).toEqual([ + path.join(skills.dirs.codex!, RELAY_SKILL_NAME), + elsewhere.path, + path.join(skills.dirs['claude-code']!, RELAY_SKILL_NAME), + ]); + }); + + it('puts a refreshed copy back when the record cannot be saved', async () => { + installedProbe(); + const core = fakeCore(); + const skills = fakeSkillsCli(); + skills.write('old\n'); + await setup({ ...core.deps, runSkillsCli: skills.run }).run('--tools', 'codex'); + editSkillRecord((record) => { + record.copies[0]!.ref = 'old'; + }); + skills.write('new\n'); + const settings = path.join(home, 'local', 'core-settings.json'); + const blocked: RunSkillsCli = (args, options) => { + const result = skills.run(args, options); + fs.rmSync(settings, { force: true }); + fs.mkdirSync(settings); + return result; + }; + const { run, logs } = setup({ ...core.deps, runSkillsCli: blocked }); + await run('--tools', 'codex'); + const copy = path.join(skills.dirs.codex!, RELAY_SKILL_NAME, 'SKILL.md'); + expect(fs.readFileSync(copy, 'utf8')).toBe('old\n'); + expect(logs.join('\n')).toContain("so this run's copies were taken out again"); + }); + + it('keeps recording a copy or lock it could not read on --uninstall', async () => { + installedProbe(); + const core = fakeCore(); + const skills = fakeSkillsCli(); + await setup({ ...core.deps, runSkillsCli: skills.run }).run('--tools', 'codex'); + const copy = path.join(skills.dirs.codex!, RELAY_SKILL_NAME); + const denied = Object.assign(new Error('EACCES'), { code: 'EACCES' }); + const { readdirSync, readFileSync } = fs; + const dirs = vi.spyOn(fs, 'readdirSync').mockImplementation((( + target: fs.PathLike, + ...rest: unknown[] + ) => { + if (target === copy) throw denied; + return (readdirSync as (...args: unknown[]) => unknown)(target, ...rest); + }) as typeof fs.readdirSync); + const files = vi.spyOn(fs, 'readFileSync').mockImplementation((( + target: fs.PathOrFileDescriptor, + ...rest: unknown[] + ) => { + if (target === skills.lock) throw denied; + return (readFileSync as (...args: unknown[]) => unknown)(target, ...rest); + }) as typeof fs.readFileSync); + const { run, logs } = setup(core.deps); + await run('--uninstall'); + dirs.mockRestore(); + files.mockRestore(); + expect(logs.join('\n')).toContain(`Could not remove ${copy}, ${skills.lock}`); + await setup(core.deps).run('--uninstall'); + expect(fs.existsSync(copy)).toBe(false); + expect(fs.existsSync(skills.lock)).toBe(false); + }); + it('removes the other copies and keeps recording one it could not remove', async () => { installedProbe(); const core = fakeCore(); diff --git a/packages/cli/src/cli/commands/init.ts b/packages/cli/src/cli/commands/init.ts index 23848ae10..bc2042457 100644 --- a/packages/cli/src/cli/commands/init.ts +++ b/packages/cli/src/cli/commands/init.ts @@ -88,6 +88,7 @@ import { type PiState, } from '../lib/init-leftovers.js'; import { + applySkillChange, defaultRunSkillsCli, installRelaySkill, uninstallRelaySkill, @@ -1523,21 +1524,26 @@ function installSkill( deps.runSkillsCli ); try { - updateDeliveryRecord(({ relaySkill: _previous, ...rest }) => - result.record ? { ...rest, relaySkill: result.record } : rest - ); + updateDeliveryRecord(({ relaySkill, ...rest }) => { + const next = applySkillChange(relaySkill, result.change); + return next ? { ...rest, relaySkill: next } : rest; + }); + result.settle?.(true); } catch (error) { // The skill is optional, but what --uninstall would not know about is not left behind. - const undone = uninstallRelaySkill(result.undo, ctx); - const left = undone?.remaining; - const outcome = left - ? `and these could not be taken out again, so remove them by hand: ${[ - ...left.copies.map((copy) => copy.path), - ...left.locks.map((lock) => lock.file), - ].join(', ')}` - : result.undo?.copies.length - ? "so this run's copies were taken out again" - : 'and this run added no copies to take out'; + const { restored, failed } = result.settle?.(false) ?? { restored: [], failed: [] }; + const left = uninstallRelaySkill(result.undo, ctx)?.remaining; + const stuck = [ + ...failed, + ...(left?.copies.map((copy) => copy.path) ?? []), + ...(left?.locks.map((lock) => lock.file) ?? []), + ]; + const outcome = + stuck.length > 0 + ? `and these could not be taken out again, so remove them by hand: ${stuck.join(', ')}` + : result.undo?.copies.length || restored.length > 0 + ? "so this run's copies were taken out again" + : 'and this run added no copies to take out'; return ( `! Agent skill not installed: ${deliveryRecordPath()} could not be saved ` + `(${(error as Error).message}), ${outcome}` diff --git a/packages/cli/src/cli/lib/relay-skill.ts b/packages/cli/src/cli/lib/relay-skill.ts index 1d950ec77..b6e4f1585 100644 --- a/packages/cli/src/cli/lib/relay-skill.ts +++ b/packages/cli/src/cli/lib/relay-skill.ts @@ -14,6 +14,7 @@ import { spawnSync } from 'node:child_process'; import { createHash } from 'node:crypto'; import fs from 'node:fs'; +import os from 'node:os'; import path from 'node:path'; import { @@ -197,6 +198,17 @@ function exists(file: string): boolean { return fs.lstatSync(file, { throwIfNoEntry: false }) !== undefined; } +/** Whether `file` is gone, a real folder, something else, or cannot be told. */ +function pathState(file: string): 'missing' | 'folder' | 'other' | 'unreadable' { + try { + const stat = fs.lstatSync(file, { throwIfNoEntry: false }); + if (!stat) return 'missing'; + return stat.isDirectory() ? 'folder' : 'other'; + } catch (error) { + return (error as NodeJS.ErrnoException).code === 'ENOTDIR' ? 'missing' : 'unreadable'; + } +} + /** Every missing folder on the way to `file`, deepest first; empty when its folder exists. */ function missingDirs(file: string): string[] { const missing: string[] = []; @@ -246,12 +258,48 @@ export function relaySkillRecordOf(value: unknown): RelaySkillRecord | undefined return { copies, createdDirs, locks }; } -/** One line about the skill, and the record to keep. */ +/** What one install changed, applied to the record as it is when saved (`applySkillChange`). */ +export interface SkillChange { + copies: InstalledSkillCopy[]; + createdDirs: string[]; + lock?: SkillLockRecord; +} + +/** One line about the skill, and what the run changed. */ export interface SkillInstallResult { line: string; - record: RelaySkillRecord | undefined; - /** Only what this run newly created: what to take back out if `record` cannot be saved. */ + change?: SkillChange; + /** Only what this run newly created: what to take back out if the change cannot be saved. */ undo?: RelaySkillRecord; + /** + * Called once the change is saved or not: drops the backups of refreshed + * copies, first putting them back when `saved` is false. + * @returns The folders put back, and those that could not be + */ + settle?: (saved: boolean) => { restored: string[]; failed: string[] }; +} + +/** + * Apply one install's change to the record as it is now, so parallel runs + * keep each other's copies. Copies this run did not write stay as recorded. + * @param current - The record just before saving + * @param change - What this run changed + * @returns The record to save, or undefined when there is nothing to record + */ +export function applySkillChange( + current: RelaySkillRecord | undefined, + change: SkillChange | undefined +): RelaySkillRecord | undefined { + if (!change) return current; + const written = new Set(change.copies.map((copy) => copy.path)); + const copies = [...(current?.copies ?? []).filter((copy) => !written.has(copy.path)), ...change.copies]; + const locks = mergeLocks(current?.locks ?? [], change.lock); + if (copies.length === 0 && locks.length === 0) return undefined; + return { + copies, + createdDirs: [...new Set([...(current?.createdDirs ?? []), ...change.createdDirs])], + locks, + }; } /** @@ -275,7 +323,7 @@ export function installRelaySkill( try { return install(tools, ctx, recorded, run); } catch (error) { - return { line: failureLine(tools.map(toolLabel).join(', '), (error as Error).message), record: recorded }; + return { line: failureLine(tools.map(toolLabel).join(', '), (error as Error).message) }; } } @@ -334,7 +382,6 @@ function install( const state = kept.length > 0 ? 'already there' : 'installed'; return { line: `✓ Agent skill ${RELAY_SKILL_NAME} ${state} for ${label([...byPath.keys()])}`, - record: recorded, }; } @@ -343,6 +390,8 @@ function install( const entryBefore = readLock(lock).skills?.[RELAY_SKILL_NAME]; const createdDirs = [...targets.map((dir) => path.join(dir, 'SKILL.md')), lock].flatMap(missingDirs); const agents = targets.flatMap((dir) => byPath.get(dir)!.flatMap((id) => ['--agent', SKILLS_AGENT[id]])); + // A copy is not changed unless it can be put back: a failure here is the warning line. + const backups = backupCopies(targets.filter((dir) => before.get(dir) !== undefined)); let result: ReturnType; try { result = run( @@ -363,36 +412,69 @@ function install( hash !== undefined && (before.get(dir) === undefined || result.status === 0 || hash !== before.get(dir)) ); }); - const copies = [ - ...(recorded?.copies ?? []).filter((copy) => !landed.includes(copy.path)), - ...landed.map((dir) => ({ path: dir, ref: RELAY_SKILL_REF, treeSha256: after.get(dir)! })), - ]; + const copies = landed.map((dir) => ({ path: dir, ref: RELAY_SKILL_REF, treeSha256: after.get(dir)! })); const written = lockWritten(lock, lockExisted, entryBefore); - const locks = mergeLocks(recorded?.locks ?? [], written); const made = createdDirs.filter(exists); - const record: RelaySkillRecord | undefined = - copies.length > 0 || locks.length > 0 - ? { copies, createdDirs: [...new Set([...(recorded?.createdDirs ?? []), ...made])], locks } - : undefined; + const change: SkillChange = { copies, createdDirs: made, ...(written ? { lock: written } : {}) }; const undo: RelaySkillRecord = { - copies: copies.filter((copy) => landed.includes(copy.path) && before.get(copy.path) === undefined), + copies: copies.filter((copy) => before.get(copy.path) === undefined), createdDirs: made, locks: written ? [written] : [], }; + const settle = (saved: boolean) => settleBackups(backups, saved ? [] : landed); const missed = targets.filter((dir) => !landed.includes(dir)); if (missed.length > 0) { const reason = result.status === 0 ? 'it did not land' : lastLine(result.output) || `exit ${result.status}`; - return { line: failureLine(label(missed), reason), record, undo }; + return { line: failureLine(label(missed), reason), change, undo, settle }; } const keptNote = kept.length > 0 ? `; already there for ${label(kept)}, left as is` : ''; return { line: `✓ Agent skill installed ${RELAY_SKILL_NAME} for ${label(landed)}${keptNote}`, - record, + change, undo, + settle, }; } +/** Copies of `dirs`, by folder, in a private temporary folder; empty when there are none. */ +function backupCopies(dirs: string[]): Map { + const backups = new Map(); + if (dirs.length === 0) return backups; + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'relay-skill-')); + dirs.forEach((dir, index) => { + const backup = path.join(root, String(index)); + fs.cpSync(dir, backup, { recursive: true, verbatimSymlinks: true }); + backups.set(dir, backup); + }); + return backups; +} + +/** Put back the `restore` folders that have a backup, then drop the backups. */ +function settleBackups( + backups: Map, + restore: string[] +): { restored: string[]; failed: string[] } { + const restored: string[] = []; + const failed: string[] = []; + for (const dir of restore) { + const backup = backups.get(dir); + if (!backup) continue; + try { + fs.rmSync(dir, { recursive: true, force: true }); + fs.cpSync(backup, dir, { recursive: true, verbatimSymlinks: true }); + restored.push(dir); + } catch { + failed.push(dir); + } + } + for (const backup of backups.values()) { + fs.rmSync(path.dirname(backup), { recursive: true, force: true }); + break; + } + return { restored, failed }; +} + function failureLine(tools: string, reason: string): string { return ( `! Agent skill could not install ${RELAY_SKILL_NAME} for ${tools} (${reason}). ` + @@ -437,8 +519,17 @@ export function uninstallRelaySkill( const elsewhere = recorded.copies.filter((copy) => !allowed.has(copy.path)); for (const copy of recorded.copies) { if (!allowed.has(copy.path)) continue; + const state = pathState(copy.path); + if (state === 'missing') continue; + if (state === 'unreadable') { + failed.push(copy); + continue; + } const hash = treeHash(copy.path); - if (hash === undefined) continue; + if (state === 'folder' && hash === undefined) { + failed.push(copy); + continue; + } if (hash !== copy.treeSha256) { changed.push(copy.path); continue; @@ -494,8 +585,14 @@ export function uninstallRelaySkill( * @returns false when the lock file could not be changed */ function restoreLock(record: SkillLockRecord): boolean { - const lock = readLock(record.file); - if (!lock.skills || lock.skills[RELAY_SKILL_NAME]?.ref !== record.ref) return true; + let lock: SkillLock; + try { + lock = JSON.parse(fs.readFileSync(record.file, 'utf8')) as SkillLock; + } catch (error) { + // Gone is done; an unreadable or damaged file stays recorded for a later --uninstall. + return (error as NodeJS.ErrnoException).code === 'ENOENT'; + } + if (!lock?.skills || lock.skills[RELAY_SKILL_NAME]?.ref !== record.ref) return true; if (record.original) lock.skills[RELAY_SKILL_NAME] = record.original as { ref?: string }; else delete lock.skills[RELAY_SKILL_NAME]; try { From 1b42725fb500c1f6526fd5e6fd0b7e7e2c01ad87 Mon Sep 17 00:00:00 2001 From: kjgbot Date: Fri, 30 Oct 2026 23:06:24 -0700 Subject: [PATCH 10/11] fix(init): pin the messaging skill to skills#200's merge commit Co-Authored-By: Claude Opus 5.5 --- packages/cli/src/cli/lib/relay-skill.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cli/src/cli/lib/relay-skill.ts b/packages/cli/src/cli/lib/relay-skill.ts index b6e4f1585..f8ef153b9 100644 --- a/packages/cli/src/cli/lib/relay-skill.ts +++ b/packages/cli/src/cli/lib/relay-skill.ts @@ -28,7 +28,7 @@ import { /** The skill's name in AgentWorkforce/skills. */ export const RELAY_SKILL_NAME = 'messaging-agents-on-the-relay'; /** The skills repo commit init installs the skill from. */ -export const RELAY_SKILL_REF = '410a9de7241f9e7305a562954e674997056cc701'; +export const RELAY_SKILL_REF = '88fdbefc92f6ca405b4d5243ef30d4e7b7b2203d'; /** The pinned source, as `skills add` takes it (`#`). */ export const RELAY_SKILL_SOURCE = `https://github.com/AgentWorkforce/skills#${RELAY_SKILL_REF}`; /** The `skills` CLI version init runs through npx. */ From 0d04850d744bff92feb802bab7c9cc6dc81f9089 Mon Sep 17 00:00:00 2001 From: kjgbot Date: Fri, 30 Oct 2026 23:10:32 -0700 Subject: [PATCH 11/11] fix(init): a backup that cannot be deleted never undoes a saved refresh Co-Authored-By: Claude Opus 5.5 --- packages/cli/src/cli/commands/init.test.ts | 25 ++++++++++++++++++++++ packages/cli/src/cli/commands/init.ts | 2 +- packages/cli/src/cli/lib/relay-skill.ts | 8 ++++--- 3 files changed, 31 insertions(+), 4 deletions(-) diff --git a/packages/cli/src/cli/commands/init.test.ts b/packages/cli/src/cli/commands/init.test.ts index 1e0bbf68a..5af2eab3e 100644 --- a/packages/cli/src/cli/commands/init.test.ts +++ b/packages/cli/src/cli/commands/init.test.ts @@ -2387,6 +2387,31 @@ describe('relay init messaging skill', () => { expect(fs.existsSync(skills.lock)).toBe(false); }); + it('keeps a saved refresh when its backup cannot be deleted', async () => { + installedProbe(); + const core = fakeCore(); + const skills = fakeSkillsCli(); + skills.write('old\n'); + await setup({ ...core.deps, runSkillsCli: skills.run }).run('--tools', 'codex'); + editSkillRecord((record) => { + record.copies[0]!.ref = 'old'; + }); + skills.write('new\n'); + const rmSync = fs.rmSync; + const spy = vi.spyOn(fs, 'rmSync').mockImplementation((target, options) => { + if (String(target).includes('relay-skill-')) throw new Error('EBUSY'); + rmSync(target, options); + }); + const { run, logs } = setup({ ...core.deps, runSkillsCli: skills.run }); + await expect(run('--tools', 'codex')).resolves.toBeDefined(); + spy.mockRestore(); + const copy = path.join(skills.dirs.codex!, RELAY_SKILL_NAME, 'SKILL.md'); + expect(fs.readFileSync(copy, 'utf8')).toBe('new\n'); + expect(logs.join('\n')).toContain(`installed ${RELAY_SKILL_NAME} for Codex`); + const saved = JSON.parse(fs.readFileSync(path.join(home, 'local', 'core-settings.json'), 'utf8')); + expect(saved.relaySkill.copies[0].ref).toBe(RELAY_SKILL_REF); + }); + it('removes the other copies and keeps recording one it could not remove', async () => { installedProbe(); const core = fakeCore(); diff --git a/packages/cli/src/cli/commands/init.ts b/packages/cli/src/cli/commands/init.ts index bc2042457..1a9b68b56 100644 --- a/packages/cli/src/cli/commands/init.ts +++ b/packages/cli/src/cli/commands/init.ts @@ -1528,7 +1528,6 @@ function installSkill( const next = applySkillChange(relaySkill, result.change); return next ? { ...rest, relaySkill: next } : rest; }); - result.settle?.(true); } catch (error) { // The skill is optional, but what --uninstall would not know about is not left behind. const { restored, failed } = result.settle?.(false) ?? { restored: [], failed: [] }; @@ -1549,6 +1548,7 @@ function installSkill( `(${(error as Error).message}), ${outcome}` ); } + result.settle?.(true); return result.line; } diff --git a/packages/cli/src/cli/lib/relay-skill.ts b/packages/cli/src/cli/lib/relay-skill.ts index f8ef153b9..c75cb148b 100644 --- a/packages/cli/src/cli/lib/relay-skill.ts +++ b/packages/cli/src/cli/lib/relay-skill.ts @@ -468,9 +468,11 @@ function settleBackups( failed.push(dir); } } - for (const backup of backups.values()) { - fs.rmSync(path.dirname(backup), { recursive: true, force: true }); - break; + const [first] = backups.values(); + try { + if (first) fs.rmSync(path.dirname(first), { recursive: true, force: true }); + } catch { + // Best effort: a leftover temporary folder changes nothing about the install. } return { restored, failed }; }