From 4e02766dd745033462aaaf809eacdd4786f0582c Mon Sep 17 00:00:00 2001 From: Clay Good Date: Tue, 21 Jul 2026 12:56:24 -0500 Subject: [PATCH] fix(init): only advertise slash commands the profile installs Fixes #1409. The `openspec init` welcome screen and the `openspec update` legacy-upgrade menu hardcoded /opsx:new and /opsx:continue. The default core profile is propose/explore/apply/update/sync/archive, so it never generates them and users were told to run commands that did not exist. getOnboardingCommands() holds the hints in lifecycle order and returns only those whose workflow is installed; both surfaces print its result. In `update` the set is what the newly configured tools actually received, since a legacy upgrade installs an inferred subset for Codex. Stacked on #1404, which decides how each hint is spelled per tool. This commit decides which hints appear; #1404's referenceFor/printStartHints decide the reference form, so Kimi still gets /skill:openspec-*. The welcome screen's quick-start block is width-constrained: it renders beside a 24-column art column and only animates at MIN_WIDTH (60) or wider, and the animation moves the cursor up a fixed count of logical lines. A wrapped line desyncs it, so descriptions are capped at DESCRIPTION_BUDGET and a test asserts no rendered line exceeds 59. Also validates --profile before the welcome screen rather than casting it, so an invalid value fails before the user presses Enter. --- .../profile-aware-onboarding-commands.md | 5 ++ src/core/init.ts | 28 +++++--- src/core/onboarding-commands.ts | 50 ++++++++++++++ src/core/update.ts | 28 +++++--- src/ui/welcome-screen.ts | 25 ++++--- test/core/init.test.ts | 3 + test/core/onboarding-commands.test.ts | 43 ++++++++++++ test/core/update.test.ts | 51 ++++++++++++-- test/ui/welcome-screen.test.ts | 69 ++++++++++++++++++- 9 files changed, 269 insertions(+), 33 deletions(-) create mode 100644 .changeset/profile-aware-onboarding-commands.md create mode 100644 src/core/onboarding-commands.ts create mode 100644 test/core/onboarding-commands.test.ts diff --git a/.changeset/profile-aware-onboarding-commands.md b/.changeset/profile-aware-onboarding-commands.md new file mode 100644 index 0000000000..03f2ef9a73 --- /dev/null +++ b/.changeset/profile-aware-onboarding-commands.md @@ -0,0 +1,5 @@ +--- +"@fission-ai/openspec": patch +--- + +Only advertise onboarding commands that will actually exist. The `openspec init` welcome screen and the `openspec update` "Getting started" summary listed `/opsx:new` and `/opsx:continue`, which the default `core` profile never generates, so users were told to run commands that did not exist. Both surfaces now list the commands for the installed workflows. The `init` and `update` completion hints also name the skill (`/openspec-propose`) instead of a command for tools that receive no command files — Codex, and any tool under skills-only delivery. diff --git a/src/core/init.ts b/src/core/init.ts index 895c2270b0..48774602d7 100644 --- a/src/core/init.ts +++ b/src/core/init.ts @@ -174,17 +174,19 @@ export class InitCommand { migrateIfNeeded(projectPath, detectedTools); } + // Validate profile override early so invalid values fail before tool setup. + // The resolved value is consumed later when generation reads effective config. + // This runs ahead of the welcome screen so an invalid --profile does not make + // the user press Enter before seeing the error. + this.resolveProfileOverride(); + // Show animated welcome screen (interactive mode only) const canPrompt = this.canPromptInteractively(); if (canPrompt) { const { showWelcomeScreen } = await import('../ui/welcome-screen.js'); - await showWelcomeScreen(); + await showWelcomeScreen(this.getActiveWorkflows()); } - // Validate profile override early so invalid values fail before tool setup. - // The resolved value is consumed later when generation reads effective config. - this.resolveProfileOverride(); - // Get tool states before processing const toolStates = getToolStates(projectPath); @@ -248,6 +250,16 @@ export class InitCommand { throw new Error(`Invalid profile "${this.profileOverride}". Available profiles: core, custom`); } + /** + * Resolves the workflows the effective profile installs, so onboarding output + * only mentions commands that will actually exist. + */ + private getActiveWorkflows(): string[] { + const globalCfg = getGlobalConfig(); + const activeProfile: Profile = this.resolveProfileOverride() ?? globalCfg.profile ?? 'core'; + return [...getProfileWorkflows(activeProfile, globalCfg.workflows)]; + } + // ═══════════════════════════════════════════════════════════ // LEGACY CLEANUP // ═══════════════════════════════════════════════════════════ @@ -865,12 +877,10 @@ export class InitCommand { } // Getting started (task 7.6: show propose if in profile) - const globalCfg = getGlobalConfig(); - const activeProfile: Profile = (this.profileOverride as Profile) ?? globalCfg.profile ?? 'core'; - const activeWorkflows = [...getProfileWorkflows(activeProfile, globalCfg.workflows)]; + const activeWorkflows = this.getActiveWorkflows(); // When no tool got /opsx:* commands, point at the skill instead of a // command that does not exist. - const activeDelivery: Delivery = globalCfg.delivery ?? 'both'; + const activeDelivery: Delivery = getGlobalConfig().delivery ?? 'both'; const commandsGenerated = successfulTools.some((tool) => shouldGenerateCommandsForTool(tool.value, activeDelivery)); const skillsGenerated = successfulTools.some((tool) => shouldGenerateSkillsForTool(tool.value, activeDelivery)); // Each hint line must be a usable instruction for the tool it serves. diff --git a/src/core/onboarding-commands.ts b/src/core/onboarding-commands.ts new file mode 100644 index 0000000000..1b18b07797 --- /dev/null +++ b/src/core/onboarding-commands.ts @@ -0,0 +1,50 @@ +/** + * Onboarding command hints. + * + * The commands shown to a user after setup must be limited to the workflows + * their profile actually installs, otherwise we advertise slash commands that + * were correctly never generated. + * + * This module decides WHICH hints to show. How each one is spelled for a given + * tool — command, skill, or a tool-specific skill prefix — is decided by + * src/utils/command-references.ts at the call site. + */ + +import type { WorkflowId } from './profiles.js'; + +export type OnboardingCommand = { + workflow: WorkflowId; + command: string; + description: string; +}; + +/** + * Longest description the welcome screen can render. It shows these beside a + * 24-column art column and only animates at MIN_WIDTH (60) columns or wider; a + * longer line wraps, and the animation's cursor-up count assumes unwrapped + * lines. See src/ui/welcome-screen.ts. + */ +export const DESCRIPTION_BUDGET = 17; + +/** + * Ordered onboarding hints. Each entry is shown only when its workflow is + * installed, so the list follows the change lifecycle: start, then build, + * then implement. + */ +const ONBOARDING_COMMANDS: readonly OnboardingCommand[] = [ + { workflow: 'propose', command: '/opsx:propose', description: 'Start a change' }, + { workflow: 'new', command: '/opsx:new', description: 'Scaffold a change' }, + { workflow: 'continue', command: '/opsx:continue', description: 'Next artifact' }, + { workflow: 'apply', command: '/opsx:apply', description: 'Implement tasks' }, +]; + +/** + * Returns the onboarding hints for the installed workflows, in lifecycle order. + * Returns an empty array when none of the onboarding workflows are installed. + */ +export function getOnboardingCommands( + workflows: readonly string[] +): OnboardingCommand[] { + const installed = new Set(workflows); + return ONBOARDING_COMMANDS.filter((entry) => installed.has(entry.workflow)); +} diff --git a/src/core/update.ts b/src/core/update.ts index bf7122aaa4..e983dd8383 100644 --- a/src/core/update.ts +++ b/src/core/update.ts @@ -41,6 +41,7 @@ import { import { isInteractive } from '../utils/interactive.js'; import { getGlobalConfig, type Delivery, type Profile } from './global-config.js'; import { getProfileWorkflows, ALL_WORKFLOWS, CORE_WORKFLOWS } from './profiles.js'; +import { getOnboardingCommands } from './onboarding-commands.js'; import { getAvailableTools } from './available-tools.js'; import { WORKFLOW_TO_SKILL_DIR, @@ -345,18 +346,27 @@ export class UpdateCommand { ); return forms.size === 1 ? [...forms][0] : neutralForm; }; - const entries: Array<[string, string]> = [ - [referenceFor('/opsx:new'), 'Start a new change'], - [referenceFor('/opsx:continue'), 'Create the next artifact'], - [referenceFor('/opsx:apply'), 'Implement tasks'], + // Only hint at workflows these tools actually received. A legacy upgrade + // can install a narrower set than the profile (inferred Codex prompts). + const installedWorkflows = [ + ...new Set( + newlyConfiguredTools.flatMap( + (toolId) => legacyWorkflowOverrides[toolId] ?? desiredWorkflows + ) + ), ]; - const width = Math.max(...entries.map(([reference]) => reference.length)); + const entries: Array<[string, string]> = getOnboardingCommands(installedWorkflows).map( + ({ command, description }) => [referenceFor(command), description] + ); console.log(); - console.log(chalk.bold('Getting started:')); - for (const [reference, description] of entries) { - console.log(` ${reference.padEnd(width)} ${description}`); + if (entries.length > 0) { + const width = Math.max(...entries.map(([reference]) => reference.length)); + console.log(chalk.bold('Getting started:')); + for (const [reference, description] of entries) { + console.log(` ${reference.padEnd(width)} ${description}`); + } + console.log(); } - console.log(); console.log(`Learn more: ${chalk.cyan('https://github.com/Fission-AI/OpenSpec')}`); } diff --git a/src/ui/welcome-screen.ts b/src/ui/welcome-screen.ts index 4d4c7e7994..efb4eb8889 100644 --- a/src/ui/welcome-screen.ts +++ b/src/ui/welcome-screen.ts @@ -5,6 +5,7 @@ import chalk from 'chalk'; import { WELCOME_ANIMATION } from './ascii-patterns.js'; +import { getOnboardingCommands } from '../core/onboarding-commands.js'; // Minimum terminal width for side-by-side layout const MIN_WIDTH = 60; @@ -15,7 +16,19 @@ const ART_COLUMN_WIDTH = 24; /** * Welcome text content (right column) */ -function getWelcomeText(): string[] { +function getWelcomeText(workflows: readonly string[]): string[] { + const onboardingCommands = getOnboardingCommands(workflows); + const quickStart: string[] = []; + + if (onboardingCommands.length > 0) { + const commandWidth = Math.max(...onboardingCommands.map((c) => c.command.length)); + quickStart.push(chalk.white('Quick start after setup:')); + for (const { command, description } of onboardingCommands) { + quickStart.push(` ${chalk.yellow(command.padEnd(commandWidth + 1))} ${chalk.dim(description)}`); + } + quickStart.push(''); + } + return [ chalk.white.bold('Welcome to OpenSpec'), chalk.dim('A lightweight spec-driven framework'), @@ -24,11 +37,7 @@ function getWelcomeText(): string[] { chalk.dim(' • Agent Skills for AI tools'), chalk.dim(' • /opsx:* slash commands'), '', - chalk.white('Quick start after setup:'), - ` ${chalk.yellow('/opsx:new')} ${chalk.dim('Create a change')}`, - ` ${chalk.yellow('/opsx:continue')} ${chalk.dim('Next artifact')}`, - ` ${chalk.yellow('/opsx:apply')} ${chalk.dim('Implement tasks')}`, - '', + ...quickStart, chalk.cyan('Press Enter to select tools...'), ]; } @@ -107,8 +116,8 @@ async function waitForEnter(): Promise { * Shows the animated welcome screen. * Returns when user presses Enter. */ -export async function showWelcomeScreen(): Promise { - const textLines = getWelcomeText(); +export async function showWelcomeScreen(workflows: readonly string[]): Promise { + const textLines = getWelcomeText(workflows); if (!canAnimate()) { // Fallback: show static welcome diff --git a/test/core/init.test.ts b/test/core/init.test.ts index 8bfddd17ba..839c4a28d5 100644 --- a/test/core/init.test.ts +++ b/test/core/init.test.ts @@ -902,6 +902,9 @@ describe('InitCommand - profile and detection features', () => { await initCommand.execute(testDir); expect(showWelcomeScreenMock).toHaveBeenCalled(); + // The welcome screen must be handed the profile's workflows, otherwise it + // advertises commands this profile never installs. + expect(showWelcomeScreenMock).toHaveBeenCalledWith(['explore', 'new']); expect(confirmMock).not.toHaveBeenCalled(); const exploreSkill = path.join(testDir, '.claude', 'skills', 'openspec-explore', 'SKILL.md'); diff --git a/test/core/onboarding-commands.test.ts b/test/core/onboarding-commands.test.ts new file mode 100644 index 0000000000..84dbf68335 --- /dev/null +++ b/test/core/onboarding-commands.test.ts @@ -0,0 +1,43 @@ +import { describe, expect, it } from 'vitest'; +import { + DESCRIPTION_BUDGET, + getOnboardingCommands, +} from '../../src/core/onboarding-commands.js'; +import { ALL_WORKFLOWS, CORE_WORKFLOWS } from '../../src/core/profiles.js'; + +describe('getOnboardingCommands', () => { + it('omits commands the profile does not install', () => { + const commands = getOnboardingCommands(CORE_WORKFLOWS).map((c) => c.command); + + expect(commands).toEqual(['/opsx:propose', '/opsx:apply']); + expect(commands).not.toContain('/opsx:new'); + expect(commands).not.toContain('/opsx:continue'); + }); + + it('includes expanded commands when a custom profile installs them', () => { + const commands = getOnboardingCommands(['new', 'continue', 'apply']).map((c) => c.command); + + expect(commands).toEqual(['/opsx:new', '/opsx:continue', '/opsx:apply']); + }); + + it('returns lifecycle order regardless of the order workflows are given', () => { + const commands = getOnboardingCommands(['apply', 'continue', 'propose']).map((c) => c.command); + + expect(commands).toEqual(['/opsx:propose', '/opsx:continue', '/opsx:apply']); + }); + + it('returns nothing when no onboarding workflow is installed', () => { + expect(getOnboardingCommands(['archive', 'sync'])).toEqual([]); + expect(getOnboardingCommands([])).toEqual([]); + }); + + it('keeps descriptions within the welcome screen width budget', () => { + // A longer description wraps the welcome screen at 60 columns, which desyncs + // its animation. See the width test in test/ui/welcome-screen.test.ts. + for (const { command, description } of getOnboardingCommands(ALL_WORKFLOWS)) { + expect(description.length, `${command} description is too long`).toBeLessThanOrEqual( + DESCRIPTION_BUDGET + ); + } + }); +}); diff --git a/test/core/update.test.ts b/test/core/update.test.ts index df238b7302..c58065fea9 100644 --- a/test/core/update.test.ts +++ b/test/core/update.test.ts @@ -1130,10 +1130,13 @@ ${OPENSPEC_MARKERS.end} // Legacy managed Codex prompt with codex not yet configured: the // upgrade newly configures codex, whose onboarding menu must not - // advertise /opsx:* commands (codex has no slash surface) + // advertise /opsx:* commands (codex has no slash surface). + // The prompt is opsx-new.md so the inferred workflow ('new') is one the + // onboarding menu actually lists — the menu is now filtered to the + // workflows the upgrade installed. const promptDir = path.join(process.env.CODEX_HOME!, 'prompts'); await fs.mkdir(promptDir, { recursive: true }); - await fs.writeFile(path.join(promptDir, 'opsx-explore.md'), 'legacy explore prompt'); + await fs.writeFile(path.join(promptDir, 'opsx-new.md'), 'legacy new prompt'); const consoleSpy = vi.spyOn(console, 'log'); const forceUpdateCommand = new UpdateCommand({ force: true }); @@ -1143,12 +1146,15 @@ ${OPENSPEC_MARKERS.end} consoleSpy.mockRestore(); expect(logCalls.some((entry) => entry.includes('Getting started'))).toBe(true); - const menuLines = logCalls.filter((entry) => entry.includes('Start a new change')); + const menuLines = logCalls.filter((entry) => entry.includes('Scaffold a change')); expect(menuLines).toHaveLength(1); expect(menuLines[0]).toContain('the openspec-new-change skill'); expect(logCalls.some((entry) => entry.includes('/opsx:new'))).toBe(false); expect(logCalls.some((entry) => entry.includes('/opsx:continue'))).toBe(false); expect(logCalls.some((entry) => entry.includes('/opsx:apply'))).toBe(false); + // Only the inferred workflow is advertised, not the rest of the profile + expect(logCalls.some((entry) => entry.includes('Next artifact'))).toBe(false); + expect(logCalls.some((entry) => entry.includes('Implement tasks'))).toBe(false); }); it('should preserve legacy Codex prompts when a configured Codex tool lacks the replacement workflow', async () => { @@ -1433,13 +1439,19 @@ More user content after markers. expect.stringContaining('Claude Code') ); - // Should show getting started message for newly configured tools + // Should show getting started message for newly configured tools, + // limited to the commands the core profile installs (not new/continue) expect(consoleSpy).toHaveBeenCalledWith( expect.stringContaining('Getting started') ); expect(consoleSpy).toHaveBeenCalledWith( - expect.stringContaining('/opsx:new') + expect.stringContaining('/opsx:propose') ); + const gettingStartedCalls = consoleSpy.mock.calls + .map((call) => call.map((arg) => String(arg)).join(' ')) + .join('\n'); + expect(gettingStartedCalls).not.toContain('/opsx:new'); + expect(gettingStartedCalls).not.toContain('/opsx:continue'); // Skills should be created const skillFile = path.join(testDir, '.claude', 'skills', 'openspec-explore', 'SKILL.md'); @@ -1578,6 +1590,35 @@ More user content after markers. consoleSpy.mockRestore(); }); + it('should list the expanded commands a custom profile installs', async () => { + setMockConfig({ + featureFlags: {}, + profile: 'custom', + delivery: 'both', + workflows: ['new', 'continue', 'apply'], + }); + + const legacyCommandDir = path.join(testDir, '.claude', 'commands', 'openspec'); + await fs.mkdir(legacyCommandDir, { recursive: true }); + await fs.writeFile( + path.join(legacyCommandDir, 'proposal.md'), + 'old command content' + ); + + const consoleSpy = vi.spyOn(console, 'log'); + + await new UpdateCommand({ force: true }).execute(testDir); + + const output = consoleSpy.mock.calls + .map((call) => call.map((arg) => String(arg)).join(' ')) + .join('\n'); + expect(output).toContain('/opsx:new'); + expect(output).toContain('/opsx:continue'); + expect(output).not.toContain('/opsx:propose'); + + consoleSpy.mockRestore(); + }); + it('should not show getting started message when no new tools configured', async () => { // Set up a configured tool (no legacy artifacts) const skillsDir = path.join(testDir, '.claude', 'skills'); diff --git a/test/ui/welcome-screen.test.ts b/test/ui/welcome-screen.test.ts index 4d6b65b49f..c1ff7b2290 100644 --- a/test/ui/welcome-screen.test.ts +++ b/test/ui/welcome-screen.test.ts @@ -1,4 +1,5 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { ALL_WORKFLOWS, CORE_WORKFLOWS } from '../../src/core/profiles.js'; const { useKeypressMock } = vi.hoisted(() => ({ useKeypressMock: vi.fn(), @@ -25,13 +26,22 @@ describe('welcome screen', () => { const originalStdinIsTTY = process.stdin.isTTY; const originalStdoutIsTTY = process.stdout.isTTY; const originalColumns = process.stdout.columns; + let writeSpy: ReturnType>; + + const writtenOutput = () => + writeSpy.mock.calls.map((call) => String(call[0])).join(''); + + // The animated path paints on a timer, so assert against the static fallback. + const renderStatically = () => { + Object.defineProperty(process.stdout, 'isTTY', { value: false, configurable: true }); + }; beforeEach(() => { delete process.env.NO_COLOR; Object.defineProperty(process.stdin, 'isTTY', { value: true, configurable: true }); Object.defineProperty(process.stdout, 'isTTY', { value: true, configurable: true }); Object.defineProperty(process.stdout, 'columns', { value: 100, configurable: true }); - vi.spyOn(process.stdout, 'write').mockImplementation(() => true); + writeSpy = vi.spyOn(process.stdout, 'write').mockImplementation(() => true); useKeypressMock.mockClear(); }); @@ -50,8 +60,63 @@ describe('welcome screen', () => { it('uses an Inquirer prompt to wait for Enter', async () => { const { showWelcomeScreen } = await import('../../src/ui/welcome-screen.js'); - await showWelcomeScreen(); + await showWelcomeScreen(CORE_WORKFLOWS); expect(useKeypressMock).toHaveBeenCalledOnce(); }); + + it('only advertises commands the profile installs', async () => { + const { showWelcomeScreen } = await import('../../src/ui/welcome-screen.js'); + renderStatically(); + + await showWelcomeScreen(CORE_WORKFLOWS); + + const output = writtenOutput(); + + expect(output).toContain('/opsx:propose'); + expect(output).toContain('/opsx:apply'); + expect(output).not.toContain('/opsx:new'); + expect(output).not.toContain('/opsx:continue'); + }); + + it('advertises expanded commands when a custom profile installs them', async () => { + const { showWelcomeScreen } = await import('../../src/ui/welcome-screen.js'); + renderStatically(); + + await showWelcomeScreen(['new', 'continue', 'apply']); + + const output = writtenOutput(); + + expect(output).toContain('/opsx:new'); + expect(output).toContain('/opsx:continue'); + expect(output).not.toContain('/opsx:propose'); + }); + + it('omits the quick start block when no onboarding workflow is installed', async () => { + const { showWelcomeScreen } = await import('../../src/ui/welcome-screen.js'); + renderStatically(); + + await showWelcomeScreen(['archive']); + + const output = writtenOutput(); + + expect(output).toContain('Welcome to OpenSpec'); + expect(output).not.toContain('Quick start after setup:'); + }); + + it('keeps every rendered line inside the animation width budget', async () => { + const { showWelcomeScreen } = await import('../../src/ui/welcome-screen.js'); + renderStatically(); + + // The animated path moves the cursor up a fixed count of logical lines, so a + // line that wraps at the narrowest animating terminal (MIN_WIDTH = 60) makes + // each frame redraw lower than the last. Worst case is every command shown. + await showWelcomeScreen(ALL_WORKFLOWS); + + const rendered = writtenOutput().replace(/\x1b\[[0-9;]*[A-Za-z]/g, ''); + + for (const line of rendered.split('\n')) { + expect(line.length).toBeLessThanOrEqual(59); + } + }); });