feat(cli-module-actions): improve CLI formatting, help output, and UX (#33517)

* fix(cli-module-actions): show schema flags in execute --help

When an action ID is provided with --help, fetch the action schema and
display action-specific flags. Falls back to generic help if auth fails.

Signed-off-by: benjdlambert <ben@blam.sh>

* fix(cli-module-actions): show schema flags in execute --help and fix build errors (#33518)

* feat(cli-module-actions): improve CLI output formatting and UX

- Pretty grouped list output with colored headers and action titles
- Custom help rendering for execute --help with markdown descriptions
- Support complex schema types (object, array, union) as JSON flags
- Show server error details instead of generic status codes
- Accept multiple plugin IDs in sources add/remove

Signed-off-by: benjdlambert <ben@blam.sh>

* fix(cli-module-auth): preserve instance metadata on re-login

Signed-off-by: benjdlambert <ben@blam.sh>

* fix: address code review feedback

- Extract triplicated cli() config into showGenericHelp helper
- Strip ANSI escape sequences before rendering server markdown
- Configure marked-terminal extension once via lazy singleton
- Parallelize listGrouped HTTP requests with Promise.all
- Log actual error message in execute help catch block
- Fix marked version in declarations.d.ts comment
- Add tests for sourcesAdd/sourcesRemove batch operations
- Add test for execute JSON parse error path
- Add tests for login metadata preservation on re-auth

Signed-off-by: benjdlambert <ben@blam.sh>

* fix: use RegExp constructor to satisfy no-control-regex lint rule

Signed-off-by: benjdlambert <ben@blam.sh>

* fix: improve ANSI stripping, default info.usage, add renderMarkdown comment

- Extend stripAnsiEscapes to cover OSC, DCS, APC, PM sequences
- Default info.usage to avoid undefined in help output
- Document why marked.use() is called per invocation

Signed-off-by: benjdlambert <ben@blam.sh>

* fix: use strip-ansi, fresh Marked instance, add allOf support

- Replace hand-rolled ANSI stripping with strip-ansi package
- Use fresh Marked instance per call instead of mutating global singleton
- Add allOf to complex type detection alongside anyOf/oneOf
- Add happy-path test for valid JSON complex flag parsing
- Bump changeset to minor for new user-facing capabilities

Signed-off-by: benjdlambert <ben@blam.sh>

* refactor: collapse listGrouped into list on ActionsClient

Signed-off-by: benjdlambert <ben@blam.sh>

* refactor: clean up cli-module-actions structure

- Extract shared pluginSourcesSchema into lib/pluginSources.ts
- Merge schemaToFlags and getComplexKeys into single return value
- Move CleyeFlag-to-FlagInfo conversion into format.ts
- Extract parseArgs and showActionHelp from execute command body

Signed-off-by: benjdlambert <ben@blam.sh>

---------

Signed-off-by: benjdlambert <ben@blam.sh>
Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
This commit is contained in:
Ben Lambert
2026-04-09 13:53:44 +02:00
committed by GitHub
parent ee0c95d971
commit c705d44e4b
24 changed files with 1535 additions and 123 deletions
@@ -0,0 +1,182 @@
/*
* Copyright 2025 The Backstage Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import type { CliCommandContext } from '@backstage/cli-node';
const mockUpsertInstance = jest.fn();
const mockGetInstanceByName = jest.fn();
const mockWithMetadataLock = jest
.fn()
.mockImplementation((fn: Function) => fn());
const mockSecretStoreSet = jest.fn();
jest.mock('../lib/storage', () => ({
upsertInstance: (...args: any[]) => mockUpsertInstance(...args),
getInstanceByName: (...args: any[]) => mockGetInstanceByName(...args),
withMetadataLock: (...args: any[]) => mockWithMetadataLock(...args),
getAllInstances: jest
.fn()
.mockResolvedValue({ instances: [], selected: undefined }),
}));
jest.mock('@internal/cli', () => ({
getSecretStore: jest.fn().mockResolvedValue({
set: (...args: any[]) => mockSecretStoreSet(...args),
}),
getAuthInstanceService: jest.fn().mockReturnValue('test-service'),
}));
jest.mock('cleye', () => ({
cli: jest.fn().mockReturnValue({
flags: {
backendUrl: 'https://backstage.example.com',
noBrowser: true,
instance: 'test-instance',
},
}),
}));
const mockWaitForCode = jest.fn().mockResolvedValue({
code: 'test-code',
state: 'test-state',
});
const mockClose = jest.fn();
jest.mock('../lib/localServer', () => ({
startCallbackServer: jest.fn().mockResolvedValue({
url: 'http://localhost:9999/callback',
waitForCode: () => mockWaitForCode(),
close: () => mockClose(),
}),
}));
jest.mock('../lib/pkce', () => ({
generateVerifier: jest.fn().mockReturnValue('test-verifier'),
challengeFromVerifier: jest.fn().mockReturnValue('test-challenge'),
}));
jest.mock('../lib/http', () => ({
httpJson: jest.fn().mockResolvedValue({
access_token: 'new-access-token',
token_type: 'bearer',
expires_in: 3600,
refresh_token: 'new-refresh-token',
}),
}));
jest.mock('node:crypto', () => ({
randomBytes: jest.fn().mockReturnValue({
toString: () => 'test-state',
}),
}));
jest.mock('node:child_process', () => ({ spawn: jest.fn() }));
jest.mock('fs-extra', () => ({ readFile: jest.fn() }));
jest.mock('glob', () => ({ sync: jest.fn().mockReturnValue([]) }));
jest.mock('yaml', () => ({ parse: jest.fn() }));
jest.mock('inquirer', () => ({ prompt: jest.fn() }));
import loginCommand from './login';
// Mock global fetch for the metadata endpoint check
const originalFetch = global.fetch;
beforeAll(() => {
global.fetch = jest.fn().mockResolvedValue({
ok: true,
json: async () => ({}),
}) as any;
});
afterAll(() => {
global.fetch = originalFetch;
});
const baseContext: CliCommandContext = {
args: [],
info: { name: 'login', description: 'Log in to Backstage' },
} as unknown as CliCommandContext;
describe('login command - metadata preservation', () => {
let stdoutSpy: jest.SpiedFunction<typeof process.stdout.write>;
let stderrSpy: jest.SpiedFunction<typeof process.stderr.write>;
beforeEach(() => {
jest.clearAllMocks();
mockWithMetadataLock.mockImplementation((fn: Function) => fn());
stdoutSpy = jest
.spyOn(process.stdout, 'write')
.mockImplementation(() => true);
stderrSpy = jest
.spyOn(process.stderr, 'write')
.mockImplementation(() => true);
});
afterEach(() => {
stdoutSpy.mockRestore();
stderrSpy.mockRestore();
});
it('preserves metadata and selected flag when re-logging into an existing instance', async () => {
mockGetInstanceByName.mockResolvedValue({
name: 'test-instance',
baseUrl: 'https://backstage.example.com',
clientId: 'old-client',
issuedAt: 1000,
accessTokenExpiresAt: 2000,
selected: true,
metadata: { pluginSources: ['catalog', 'scaffolder'] },
});
await loginCommand({
...baseContext,
args: [
'--backendUrl',
'https://backstage.example.com',
'--instance',
'test-instance',
'--noBrowser',
],
});
expect(mockUpsertInstance).toHaveBeenCalledWith(
expect.objectContaining({
name: 'test-instance',
selected: true,
metadata: { pluginSources: ['catalog', 'scaffolder'] },
}),
);
});
it('sets metadata and selected to undefined for a new instance', async () => {
mockGetInstanceByName.mockRejectedValue(new Error('Not found'));
await loginCommand({
...baseContext,
args: [
'--backendUrl',
'https://backstage.example.com',
'--instance',
'new-instance',
'--noBrowser',
],
});
expect(mockUpsertInstance).toHaveBeenCalledWith(
expect.objectContaining({
selected: undefined,
metadata: undefined,
}),
);
});
});
@@ -343,6 +343,7 @@ async function persistInstance(options: {
issuedAt: Date.now(),
accessTokenExpiresAt: Date.now() + token.expires_in * 1000,
selected: existing?.selected,
metadata: existing?.metadata,
});
});
}