From c1fac205bf8b78f00667f8663159613e6c5f69de Mon Sep 17 00:00:00 2001 From: "Garen J. Torikian" Date: Thu, 10 Sep 2026 10:33:34 -0400 Subject: [PATCH 1/3] feat(skills): offer to update stale skills after commands MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Skills ship inside the binary and are copied into each agent's skills dir, so every CLI release that bumps @workos/skills silently leaves installed copies behind. The only signal was `workos doctor`, which nobody runs routinely, and the setup offer never re-fires once completed — so a user who set up once would never learn their skills were stale. After each successful command in a human TTY, offer to refresh agents whose version marker trails the bundled version. Mirrors wrangler's update prompt (cloudflare/workers-sdk#14872) minus the network: the latest version is whatever this binary bundles, so there is no remote fetch, cache, or cooldown to manage. The prompt defaults to No (AUTH-6734: an absent-minded Enter never writes to an agent dir), honours a prior setup decline, and is asked at most once per bundled version — a "no" is remembered until a newer CLI ships newer skills. Ctrl-C is not remembered, matching setup. --- src/bin.ts | 5 +++ src/commands/setup.spec.ts | 91 +++++++++++++++++++++++++++++++++++++- src/commands/setup.ts | 53 ++++++++++++++++++++++ src/lib/preferences.ts | 15 +++++++ 4 files changed, 163 insertions(+), 1 deletion(-) diff --git a/src/bin.ts b/src/bin.ts index d3a2506c..73cf49b4 100644 --- a/src/bin.ts +++ b/src/bin.ts @@ -3473,6 +3473,11 @@ async function runCli(): Promise { try { await parser.parseAsync(rawArgs); + // Offer to refresh stale CLI-installed skills. Self-gating (human TTY only, + // once per bundled version) and best-effort — never affects the outcome. + const { maybeOfferSkillsUpdate } = await import('./commands/setup.js'); + await maybeOfferSkillsUpdate(commandName); + process.exitCode = 0; commandOutcome = { success: true, diff --git a/src/commands/setup.spec.ts b/src/commands/setup.spec.ts index f92324bc..92e7a2d1 100644 --- a/src/commands/setup.spec.ts +++ b/src/commands/setup.spec.ts @@ -39,6 +39,8 @@ vi.mock('../lib/preferences.js', () => ({ recordSetupDeclined: vi.fn(), recordSetupCompleted: vi.fn(), clearSetupDecline: vi.fn(), + getSkillsUpdateOfferedVersion: vi.fn(() => undefined), + recordSkillsUpdateOffered: vi.fn(), })); vi.mock('./install-skill.js', () => ({ @@ -50,6 +52,10 @@ vi.mock('./install-skill.js', () => ({ refreshWorkOSSkills: vi.fn(), })); +vi.mock('../doctor/checks/skills.js', () => ({ + checkSkills: vi.fn(), +})); + vi.mock('../lib/mcp-clients.js', () => ({ detectMcpClients: vi.fn(), MCP_AGENT_KEYS: ['claude-code', 'codex', 'cursor'], @@ -76,10 +82,11 @@ const { isJsonMode, outputSuccess } = await import('../utils/output.js'); const { isPromptAllowed } = await import('../utils/interaction-mode.js'); const prefs = await import('../lib/preferences.js'); const { detectAgents, refreshWorkOSSkills } = await import('./install-skill.js'); +const { checkSkills } = await import('../doctor/checks/skills.js'); const { detectMcpClients } = await import('../lib/mcp-clients.js'); const { analytics } = await import('../utils/analytics.js'); -const { runSetup, maybeRunSetupAfter } = await import('./setup.js'); +const { runSetup, maybeRunSetupAfter, maybeOfferSkillsUpdate } = await import('./setup.js'); // ── Fixtures ────────────────────────────────────────────────────────────────── const claudeAgent = { name: 'claude-code', displayName: 'Claude Code', globalSkillsDir: '/x', detect: () => true }; @@ -110,6 +117,8 @@ beforeEach(() => { vi.mocked(isJsonMode).mockReturnValue(false); vi.mocked(prefs.isSetupDeclined).mockReturnValue(false); vi.mocked(prefs.isSetupCompleted).mockReturnValue(false); + vi.mocked(prefs.getSkillsUpdateOfferedVersion).mockReturnValue(undefined); + vi.mocked(checkSkills).mockResolvedValue(null); vi.mocked(refreshWorkOSSkills).mockResolvedValue({ agents: [claudeAgent as any], skills: ['workos', 'workos-widgets'], @@ -431,3 +440,83 @@ describe('maybeRunSetupAfter', () => { expect(confirmArgs.signal).toBeUndefined(); }); }); + +describe('maybeOfferSkillsUpdate', () => { + function staleClaude() { + vi.mocked(checkSkills).mockResolvedValue({ + bundledVersion: '2.0.0', + agents: [ + { agent: 'Claude Code', installedVersion: '1.0.0', stale: true }, + { agent: 'Cursor', installedVersion: '2.0.0', stale: false }, + ], + }); + } + + it('is silent for exempt commands, non-human mode, JSON mode, and after a setup decline', async () => { + staleClaude(); + + await maybeOfferSkillsUpdate('skills.install'); + await maybeOfferSkillsUpdate('root'); + vi.mocked(isPromptAllowed).mockReturnValue(false); + await maybeOfferSkillsUpdate('organization.list'); + vi.mocked(isPromptAllowed).mockReturnValue(true); + vi.mocked(isJsonMode).mockReturnValue(true); + await maybeOfferSkillsUpdate('organization.list'); + vi.mocked(isJsonMode).mockReturnValue(false); + vi.mocked(prefs.isSetupDeclined).mockReturnValue(true); + await maybeOfferSkillsUpdate('organization.list'); + + expect(ui.confirm).not.toHaveBeenCalled(); + }); + + it('does not prompt when nothing is stale or this bundled version was already offered', async () => { + await maybeOfferSkillsUpdate('organization.list'); + staleClaude(); + vi.mocked(prefs.getSkillsUpdateOfferedVersion).mockReturnValue('2.0.0'); + await maybeOfferSkillsUpdate('organization.list'); + + expect(ui.confirm).not.toHaveBeenCalled(); + }); + + it('refreshes only the stale agents on accept and remembers the bundled version', async () => { + staleClaude(); + vi.mocked(ui.confirm).mockResolvedValue(true); + + await maybeOfferSkillsUpdate('organization.list'); + + expect(ui.confirm).toHaveBeenCalledWith(expect.objectContaining({ initialValue: false })); + expect(refreshWorkOSSkills).toHaveBeenCalledWith({ + agents: [expect.objectContaining({ name: 'claude-code' })], + }); + expect(prefs.recordSkillsUpdateOffered).toHaveBeenCalledWith('2.0.0'); + expect(ui.log.success).toHaveBeenCalled(); + }); + + it('installs nothing on decline but remembers the version so it is not asked again', async () => { + staleClaude(); + vi.mocked(ui.confirm).mockResolvedValue(false); + + await maybeOfferSkillsUpdate('organization.list'); + + expect(refreshWorkOSSkills).not.toHaveBeenCalled(); + expect(prefs.recordSkillsUpdateOffered).toHaveBeenCalledWith('2.0.0'); + }); + + it('treats cancel (ctrl-c) as skip without remembering the version', async () => { + staleClaude(); + vi.mocked(ui.confirm).mockResolvedValue(CANCEL); + + await maybeOfferSkillsUpdate('organization.list'); + + expect(refreshWorkOSSkills).not.toHaveBeenCalled(); + expect(prefs.recordSkillsUpdateOffered).not.toHaveBeenCalled(); + }); + + it('never throws; failures are reported to telemetry', async () => { + vi.mocked(checkSkills).mockRejectedValue(new Error('boom')); + + await expect(maybeOfferSkillsUpdate('organization.list')).resolves.toBeUndefined(); + + expect(analytics.captureException).toHaveBeenCalledWith(expect.any(Error), { 'setup.trigger': 'skills-update' }); + }); +}); diff --git a/src/commands/setup.ts b/src/commands/setup.ts index efd2a6aa..d58acd07 100644 --- a/src/commands/setup.ts +++ b/src/commands/setup.ts @@ -34,8 +34,11 @@ import { recordSetupDeclined, recordSetupCompleted, clearSetupDecline, + getSkillsUpdateOfferedVersion, + recordSkillsUpdateOffered, } from '../lib/preferences.js'; import { createAgents, detectAgents, refreshWorkOSSkills, type AgentConfig } from './install-skill.js'; +import { checkSkills } from '../doctor/checks/skills.js'; import { detectMcpClients, MCP_AGENT_KEYS, @@ -334,3 +337,53 @@ export async function maybeRunSetupAfter(trigger: 'login' | 'install'): Promise< }); } } + +/** + * Commands after which the stale-skills prompt is never shown: the bare root + * (`--help` / `--version`) and the commands that manage or report on skills. + */ +const SKILLS_UPDATE_EXEMPT_COMMANDS = new Set(['root', 'skills', 'setup', 'doctor']); + +/** + * Best-effort offer to refresh CLI-installed skills that trail the version + * bundled with this binary. Runs after every successful command (see `runCli`), + * gated like the automatic setup offer: human TTY only, never after a setup + * decline. Asked at most once per bundled skills version — a "no" (or a failed + * refresh) is remembered until a newer CLI ships newer skills; a cancel + * (ctrl-c) is not. Never throws into the parent command. + */ +export async function maybeOfferSkillsUpdate(commandName: string): Promise { + try { + if (SKILLS_UPDATE_EXEMPT_COMMANDS.has(commandName.split('.')[0])) return; + if (isJsonMode() || !isPromptAllowed() || isSetupDeclined()) return; + + const info = await checkSkills(); + const bundled = info?.bundledVersion; + const staleNames = new Set(info?.agents.filter((a) => a.stale).map((a) => a.agent)); + if (!bundled || staleNames.size === 0 || getSkillsUpdateOfferedVersion() === bundled) return; + + ui.log.info( + `WorkOS skills for ${[...staleNames].join(', ')} are older than the ones bundled with this CLI (${bundled}).`, + ); + const answer = await ui.confirm({ message: 'Update them now?', initialValue: false }); + if (isCancel(answer)) return; + + recordSkillsUpdateOffered(bundled); + if (!answer) { + ui.log.hint(`Skipped. Run \`${formatWorkOSCommand('skills install')}\` to update later.`); + return; + } + + const agents = Object.values(createAgents(homedir())).filter((a) => staleNames.has(a.displayName)); + const result = await refreshWorkOSSkills({ agents }); + if (result) { + ui.log.success(`Updated WorkOS skills for ${result.agents.map((a) => a.displayName).join(', ')}.`); + } else { + ui.log.error(`Couldn't update WorkOS skills. Run \`${formatWorkOSCommand('skills install')}\` to retry.`); + } + } catch (error) { + analytics.captureException(error instanceof Error ? error : new Error(String(error)), { + 'setup.trigger': 'skills-update', + }); + } +} diff --git a/src/lib/preferences.ts b/src/lib/preferences.ts index 9b2c412b..5a778b3d 100644 --- a/src/lib/preferences.ts +++ b/src/lib/preferences.ts @@ -51,6 +51,11 @@ export interface CliPreferences { declined?: boolean; /** ISO timestamp the user completed a setup run. */ completedAt?: string; + /** + * Bundled skills version the automatic stale-skills update prompt was last + * answered for. Suppresses re-asking until a newer CLI ships newer skills. + */ + skillsUpdateOfferedVersion?: string; }; } @@ -206,6 +211,16 @@ export function recordSetupCompleted(): void { savePreferences({ setup: { completedAt: new Date().toISOString() } }); } +/** Bundled skills version the stale-skills update prompt was last answered for. */ +export function getSkillsUpdateOfferedVersion(): string | undefined { + return getPreferences().setup?.skillsUpdateOfferedVersion; +} + +/** Persist that the stale-skills update prompt was answered for `version`. */ +export function recordSkillsUpdateOffered(version: string): void { + savePreferences({ setup: { skillsUpdateOfferedVersion: version } }); +} + /** Clear the setup decline (new + legacy) so automatic offers resume. For `workos setup --reset`. */ export function clearSetupDecline(): void { savePreferences({ setup: { declined: false }, mcp: { promptDeclined: false } }); From da996faea996f9aa15b8fe08a9b312318745341a Mon Sep 17 00:00:00 2001 From: "Garen J. Torikian" Date: Thu, 10 Sep 2026 10:52:27 -0400 Subject: [PATCH 2/3] fix(skills): report partial skill refreshes and skip the offer on redirected stdin MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two review findings on the stale-skills offer: refreshWorkOSSkills is truthy as soon as one agent lands, so a multi-agent refresh where one target failed was reported as a clean success. The offered version is recorded before the refresh — deliberately, so a persistently broken refresh cannot nag after every command — which meant the failed agent stayed stale with no second offer and no mention of it. Partial refreshes now name what did not update and point at `workos skills install`. Interaction mode is resolved from stdout/stderr, so `workos org list ({ default: { heading: vi.fn(), note: vi.fn(), - log: { info: vi.fn(), success: vi.fn(), error: vi.fn(), hint: vi.fn() }, + log: { info: vi.fn(), success: vi.fn(), error: vi.fn(), hint: vi.fn(), warn: vi.fn() }, confirm: vi.fn(), }, isCancel: (v: unknown) => v === CANCEL, @@ -442,6 +442,18 @@ describe('maybeRunSetupAfter', () => { }); describe('maybeOfferSkillsUpdate', () => { + const realStdinIsTTY = process.stdin.isTTY; + + beforeEach(() => { + // Interaction mode is mocked, but the stdin gate reads the real stream — and + // vitest runs with stdin redirected. Stub an answerable terminal. + process.stdin.isTTY = true; + }); + + afterEach(() => { + process.stdin.isTTY = realStdinIsTTY; + }); + function staleClaude() { vi.mocked(checkSkills).mockResolvedValue({ bundledVersion: '2.0.0', @@ -512,6 +524,57 @@ describe('maybeOfferSkillsUpdate', () => { expect(prefs.recordSkillsUpdateOffered).not.toHaveBeenCalled(); }); + it('stays silent when stdin is redirected, so it never asks a question nobody can answer', async () => { + staleClaude(); + process.stdin.isTTY = undefined; + + await maybeOfferSkillsUpdate('organization.list'); + + // The message must not print either: `ui.confirm` would throw on the + // redirected stdin, leaving the version unrecorded and the offer to repeat + // after every later command. + expect(ui.log.info).not.toHaveBeenCalled(); + expect(ui.confirm).not.toHaveBeenCalled(); + }); + + it('reports a partial refresh as incomplete rather than a success', async () => { + vi.mocked(checkSkills).mockResolvedValue({ + bundledVersion: '2.0.0', + agents: [ + { agent: 'Claude Code', installedVersion: '1.0.0', stale: true }, + { agent: 'Cursor', installedVersion: '1.0.0', stale: true }, + ], + }); + vi.mocked(ui.confirm).mockResolvedValue(true); + // Only Claude Code lands — `refreshWorkOSSkills` is truthy all the same. + vi.mocked(refreshWorkOSSkills).mockResolvedValue({ + agents: [claudeAgent as any], + skills: ['workos'], + version: '2.0.0', + perAgentBefore: {}, + perAgentAfter: {}, + }); + + await maybeOfferSkillsUpdate('organization.list'); + + expect(ui.log.success).not.toHaveBeenCalled(); + const warning = vi.mocked(ui.log.warn).mock.calls[0][0]; + expect(warning).toContain('Claude Code'); + expect(warning).toContain('Cursor'); + expect(warning).toContain('workos skills install'); + }); + + it('points at the retry command when nothing could be refreshed', async () => { + staleClaude(); + vi.mocked(ui.confirm).mockResolvedValue(true); + vi.mocked(refreshWorkOSSkills).mockResolvedValue(null); + + await maybeOfferSkillsUpdate('organization.list'); + + expect(ui.log.success).not.toHaveBeenCalled(); + expect(ui.log.error).toHaveBeenCalledWith(expect.stringContaining('workos skills install')); + }); + it('never throws; failures are reported to telemetry', async () => { vi.mocked(checkSkills).mockRejectedValue(new Error('boom')); diff --git a/src/commands/setup.ts b/src/commands/setup.ts index d58acd07..9a528e39 100644 --- a/src/commands/setup.ts +++ b/src/commands/setup.ts @@ -355,7 +355,13 @@ const SKILLS_UPDATE_EXEMPT_COMMANDS = new Set(['root', 'skills', 'setup', 'docto export async function maybeOfferSkillsUpdate(commandName: string): Promise { try { if (SKILLS_UPDATE_EXEMPT_COMMANDS.has(commandName.split('.')[0])) return; - if (isJsonMode() || !isPromptAllowed() || isSetupDeclined()) return; + // Interaction mode is resolved from stdout/stderr, which says nothing about + // whether anyone can answer: with stdin redirected (`workos org list < /dev/null`) + // the mode is still `human`, so the offer prints, `ui.confirm` throws on the + // non-TTY stdin, and the catch below swallows it before the offered version is + // recorded — so every later command re-prints an offer nobody can answer. + // Check stdin before emitting any prompt-related output. + if (isJsonMode() || !isPromptAllowed() || !process.stdin.isTTY || isSetupDeclined()) return; const info = await checkSkills(); const bundled = info?.bundledVersion; @@ -376,10 +382,23 @@ export async function maybeOfferSkillsUpdate(commandName: string): Promise const agents = Object.values(createAgents(homedir())).filter((a) => staleNames.has(a.displayName)); const result = await refreshWorkOSSkills({ agents }); - if (result) { - ui.log.success(`Updated WorkOS skills for ${result.agents.map((a) => a.displayName).join(', ')}.`); - } else { + // `refreshWorkOSSkills` is truthy as soon as ONE agent lands, so a truthy + // result is not the same as "all of them updated". Reporting it as a clean + // success would leave an agent silently stale, and the offered version above + // is already recorded — deliberately, so a persistently broken refresh can't + // nag after every command — which makes the explicit retry command the only + // remaining path for whatever failed. Say so instead of claiming success. + const updated = result?.agents.map((a) => a.displayName) ?? []; + const failed = agents.map((a) => a.displayName).filter((name) => !updated.includes(name)); + + if (updated.length === 0) { ui.log.error(`Couldn't update WorkOS skills. Run \`${formatWorkOSCommand('skills install')}\` to retry.`); + } else if (failed.length > 0) { + ui.log.warn( + `Updated WorkOS skills for ${updated.join(', ')}, but ${failed.join(', ')} ${failed.length === 1 ? 'is' : 'are'} still on an older version. Run \`${formatWorkOSCommand('skills install')}\` to finish.`, + ); + } else { + ui.log.success(`Updated WorkOS skills for ${updated.join(', ')}.`); } } catch (error) { analytics.captureException(error instanceof Error ? error : new Error(String(error)), { From e46bf33bfbfe823e998b084547dd438e8bace4e2 Mon Sep 17 00:00:00 2001 From: "Garen J. Torikian" Date: Thu, 10 Sep 2026 11:12:32 -0400 Subject: [PATCH 3/3] fix(skills): give an incomplete skill refresh one more offer A partial refresh left the failed agent stale with no further automatic offer: the bundled version is recorded before the refresh runs, so the warning plus `workos skills install` was the only remaining path. Some agents landing proves the refresh mechanism works, so the miss is worth retrying. A partial outcome now clears the recorded version and marks the version retried, which re-opens the offer for exactly one more command; a second incomplete refresh lets the recorded version stand. The pre-refresh record stays: a refresh that throws, or a process killed mid-way, must not turn into an offer after every subsequent command. A refresh that lands nothing at all is still remembered outright, since nothing about it suggests a retry would fare better. --- src/commands/setup.spec.ts | 39 +++++++++++++++++++++++++++++-- src/commands/setup.ts | 46 ++++++++++++++++++++++++++----------- src/lib/preferences.spec.ts | 36 +++++++++++++++++++++++++++++ src/lib/preferences.ts | 23 +++++++++++++++++++ 4 files changed, 128 insertions(+), 16 deletions(-) diff --git a/src/commands/setup.spec.ts b/src/commands/setup.spec.ts index 52d6801f..50fe9c5a 100644 --- a/src/commands/setup.spec.ts +++ b/src/commands/setup.spec.ts @@ -41,6 +41,8 @@ vi.mock('../lib/preferences.js', () => ({ clearSetupDecline: vi.fn(), getSkillsUpdateOfferedVersion: vi.fn(() => undefined), recordSkillsUpdateOffered: vi.fn(), + hasSkillsUpdateRetried: vi.fn(() => false), + recordSkillsUpdateRetry: vi.fn(), })); vi.mock('./install-skill.js', () => ({ @@ -118,6 +120,7 @@ beforeEach(() => { vi.mocked(prefs.isSetupDeclined).mockReturnValue(false); vi.mocked(prefs.isSetupCompleted).mockReturnValue(false); vi.mocked(prefs.getSkillsUpdateOfferedVersion).mockReturnValue(undefined); + vi.mocked(prefs.hasSkillsUpdateRetried).mockReturnValue(false); vi.mocked(checkSkills).mockResolvedValue(null); vi.mocked(refreshWorkOSSkills).mockResolvedValue({ agents: [claudeAgent as any], @@ -537,7 +540,8 @@ describe('maybeOfferSkillsUpdate', () => { expect(ui.confirm).not.toHaveBeenCalled(); }); - it('reports a partial refresh as incomplete rather than a success', async () => { + /** Both agents stale, but only Claude Code lands — a truthy result all the same. */ + function partialRefresh() { vi.mocked(checkSkills).mockResolvedValue({ bundledVersion: '2.0.0', agents: [ @@ -546,7 +550,6 @@ describe('maybeOfferSkillsUpdate', () => { ], }); vi.mocked(ui.confirm).mockResolvedValue(true); - // Only Claude Code lands — `refreshWorkOSSkills` is truthy all the same. vi.mocked(refreshWorkOSSkills).mockResolvedValue({ agents: [claudeAgent as any], skills: ['workos'], @@ -554,6 +557,10 @@ describe('maybeOfferSkillsUpdate', () => { perAgentBefore: {}, perAgentAfter: {}, }); + } + + it('reports a partial refresh as incomplete rather than a success', async () => { + partialRefresh(); await maybeOfferSkillsUpdate('organization.list'); @@ -564,6 +571,30 @@ describe('maybeOfferSkillsUpdate', () => { expect(warning).toContain('workos skills install'); }); + it('leaves a partial refresh eligible for exactly one more offer', async () => { + partialRefresh(); + + await maybeOfferSkillsUpdate('organization.list'); + + // The version is recorded up front so a throwing refresh can't nag forever, + // then re-opened by the retry marker because some agents did land. + expect(prefs.recordSkillsUpdateOffered).toHaveBeenCalledWith('2.0.0'); + expect(prefs.recordSkillsUpdateRetry).toHaveBeenCalledWith('2.0.0'); + }); + + it('goes quiet after a second incomplete refresh for the same version', async () => { + partialRefresh(); + vi.mocked(prefs.hasSkillsUpdateRetried).mockReturnValue(true); + + await maybeOfferSkillsUpdate('organization.list'); + + expect(ui.log.warn).toHaveBeenCalled(); + // Retry already spent: the recorded version stands, so only the explicit + // `workos skills install` remains. + expect(prefs.recordSkillsUpdateRetry).not.toHaveBeenCalled(); + expect(prefs.recordSkillsUpdateOffered).toHaveBeenCalledWith('2.0.0'); + }); + it('points at the retry command when nothing could be refreshed', async () => { staleClaude(); vi.mocked(ui.confirm).mockResolvedValue(true); @@ -573,6 +604,10 @@ describe('maybeOfferSkillsUpdate', () => { expect(ui.log.success).not.toHaveBeenCalled(); expect(ui.log.error).toHaveBeenCalledWith(expect.stringContaining('workos skills install')); + // Nothing landed, so nothing suggests a retry would fare better: remembered, + // not re-offered. + expect(prefs.recordSkillsUpdateOffered).toHaveBeenCalledWith('2.0.0'); + expect(prefs.recordSkillsUpdateRetry).not.toHaveBeenCalled(); }); it('never throws; failures are reported to telemetry', async () => { diff --git a/src/commands/setup.ts b/src/commands/setup.ts index 9a528e39..e494c24e 100644 --- a/src/commands/setup.ts +++ b/src/commands/setup.ts @@ -35,7 +35,9 @@ import { recordSetupCompleted, clearSetupDecline, getSkillsUpdateOfferedVersion, + hasSkillsUpdateRetried, recordSkillsUpdateOffered, + recordSkillsUpdateRetry, } from '../lib/preferences.js'; import { createAgents, detectAgents, refreshWorkOSSkills, type AgentConfig } from './install-skill.js'; import { checkSkills } from '../doctor/checks/skills.js'; @@ -348,9 +350,11 @@ const SKILLS_UPDATE_EXEMPT_COMMANDS = new Set(['root', 'skills', 'setup', 'docto * Best-effort offer to refresh CLI-installed skills that trail the version * bundled with this binary. Runs after every successful command (see `runCli`), * gated like the automatic setup offer: human TTY only, never after a setup - * decline. Asked at most once per bundled skills version — a "no" (or a failed - * refresh) is remembered until a newer CLI ships newer skills; a cancel - * (ctrl-c) is not. Never throws into the parent command. + * decline. Asked at most once per bundled skills version — a "no" (or a refresh + * that landed nothing) is remembered until a newer CLI ships newer skills; a + * cancel (ctrl-c) is not, and a refresh that landed for some agents but not all + * gets exactly one more offer before going quiet. Never throws into the parent + * command. */ export async function maybeOfferSkillsUpdate(commandName: string): Promise { try { @@ -374,32 +378,46 @@ export async function maybeOfferSkillsUpdate(commandName: string): Promise const answer = await ui.confirm({ message: 'Update them now?', initialValue: false }); if (isCancel(answer)) return; - recordSkillsUpdateOffered(bundled); if (!answer) { + recordSkillsUpdateOffered(bundled); ui.log.hint(`Skipped. Run \`${formatWorkOSCommand('skills install')}\` to update later.`); return; } + // Recorded BEFORE the refresh on purpose: if the refresh throws (or the + // process dies mid-way) the version is already remembered, so a hard failure + // can never turn into an offer after every subsequent command. A recoverable + // partial miss downgrades this to retry-eligible below. + const alreadyRetried = hasSkillsUpdateRetried(bundled); + recordSkillsUpdateOffered(bundled); + const agents = Object.values(createAgents(homedir())).filter((a) => staleNames.has(a.displayName)); const result = await refreshWorkOSSkills({ agents }); // `refreshWorkOSSkills` is truthy as soon as ONE agent lands, so a truthy - // result is not the same as "all of them updated". Reporting it as a clean - // success would leave an agent silently stale, and the offered version above - // is already recorded — deliberately, so a persistently broken refresh can't - // nag after every command — which makes the explicit retry command the only - // remaining path for whatever failed. Say so instead of claiming success. + // result is not the same as "all of them updated" — reporting it as a clean + // success would leave an agent silently stale. const updated = result?.agents.map((a) => a.displayName) ?? []; const failed = agents.map((a) => a.displayName).filter((name) => !updated.includes(name)); if (updated.length === 0) { ui.log.error(`Couldn't update WorkOS skills. Run \`${formatWorkOSCommand('skills install')}\` to retry.`); - } else if (failed.length > 0) { - ui.log.warn( - `Updated WorkOS skills for ${updated.join(', ')}, but ${failed.join(', ')} ${failed.length === 1 ? 'is' : 'are'} still on an older version. Run \`${formatWorkOSCommand('skills install')}\` to finish.`, - ); - } else { + return; + } + + if (failed.length === 0) { ui.log.success(`Updated WorkOS skills for ${updated.join(', ')}.`); + return; } + + // Partial: some agents landed, so the refresh mechanism demonstrably works + // and the miss is worth one more automatic offer. Bounded to one — a + // permanently unwritable skills dir must not re-ask after every command — + // after which the recorded version above stands and only the explicit + // command remains. + if (!alreadyRetried) recordSkillsUpdateRetry(bundled); + ui.log.warn( + `Updated WorkOS skills for ${updated.join(', ')}, but ${failed.join(', ')} ${failed.length === 1 ? 'is' : 'are'} still on an older version. Run \`${formatWorkOSCommand('skills install')}\` to finish.`, + ); } catch (error) { analytics.captureException(error instanceof Error ? error : new Error(String(error)), { 'setup.trigger': 'skills-update', diff --git a/src/lib/preferences.spec.ts b/src/lib/preferences.spec.ts index f4a65bdd..eebc19ec 100644 --- a/src/lib/preferences.spec.ts +++ b/src/lib/preferences.spec.ts @@ -30,6 +30,12 @@ const { getTelemetrySource, getPreferencesPath, clearPreferences, + getSkillsUpdateOfferedVersion, + recordSkillsUpdateOffered, + hasSkillsUpdateRetried, + recordSkillsUpdateRetry, + recordSetupDeclined, + isSetupDeclined, __resetPreferencesCache, } = await import('./preferences.js'); @@ -310,4 +316,34 @@ describe('preferences', () => { expect(getPreferences()).toEqual({}); }); }); + + describe('stale-skills offer state', () => { + it('re-opens the offer when marking a retry, and bounds it to one', () => { + recordSkillsUpdateOffered('2.0.0'); + expect(getSkillsUpdateOfferedVersion()).toBe('2.0.0'); + expect(hasSkillsUpdateRetried('2.0.0')).toBe(false); + + recordSkillsUpdateRetry('2.0.0'); + + // Clearing the answered-version is what makes the next command offer again. + expect(getSkillsUpdateOfferedVersion()).toBeUndefined(); + expect(hasSkillsUpdateRetried('2.0.0')).toBe(true); + // A newer bundled version is a fresh question, not a spent retry. + expect(hasSkillsUpdateRetried('3.0.0')).toBe(false); + + // Second incomplete refresh: the recorded version stands and it goes quiet. + recordSkillsUpdateOffered('2.0.0'); + expect(getSkillsUpdateOfferedVersion()).toBe('2.0.0'); + }); + + it('survives a fresh process and never clobbers a setup sibling', () => { + recordSetupDeclined(); + recordSkillsUpdateRetry('2.0.0'); + __resetPreferencesCache(); // simulate a new process reading from disk + + expect(hasSkillsUpdateRetried('2.0.0')).toBe(true); + expect(getSkillsUpdateOfferedVersion()).toBeUndefined(); + expect(isSetupDeclined()).toBe(true); + }); + }); }); diff --git a/src/lib/preferences.ts b/src/lib/preferences.ts index 5a778b3d..0008c2b3 100644 --- a/src/lib/preferences.ts +++ b/src/lib/preferences.ts @@ -56,6 +56,15 @@ export interface CliPreferences { * answered for. Suppresses re-asking until a newer CLI ships newer skills. */ skillsUpdateOfferedVersion?: string; + /** + * Bundled skills version whose offer was accepted but left at least one + * agent stale. Such a version gets exactly one more offer — the refresh + * demonstrably works for some agents, so the miss is worth one retry — + * after which it lands in `skillsUpdateOfferedVersion` and goes quiet for + * good. Bounding it is the point: a permanently unwritable skills dir must + * not re-ask after every single command. + */ + skillsUpdateRetryVersion?: string; }; } @@ -221,6 +230,20 @@ export function recordSkillsUpdateOffered(version: string): void { savePreferences({ setup: { skillsUpdateOfferedVersion: version } }); } +/** True when `version` has already burned its one post-failure retry offer. */ +export function hasSkillsUpdateRetried(version: string): boolean { + return getPreferences().setup?.skillsUpdateRetryVersion === version; +} + +/** + * Persist that `version` is owed one more offer after an incomplete refresh. + * Clearing the answered-version is what re-opens the offer for the agents that + * are still stale; the retry marker is what bounds it to exactly one more. + */ +export function recordSkillsUpdateRetry(version: string): void { + savePreferences({ setup: { skillsUpdateOfferedVersion: undefined, skillsUpdateRetryVersion: version } }); +} + /** Clear the setup decline (new + legacy) so automatic offers resume. For `workos setup --reset`. */ export function clearSetupDecline(): void { savePreferences({ setup: { declined: false }, mcp: { promptDeclined: false } });