fix(cli): make impact [target] optional so --uid resolves alone (U1, #1907)

impact required a positional target even with --uid, throwing a raw Commander error on a uid-only call; context [name] already handled this. Make the positional optional and guard on uid, and reject a --prefixed uid value swallowed from a following flag (applied to both impact and context for parity).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Gergo Magyar 2026-05-30 08:01:59 +00:00
parent c79bbd8ac3
commit 4f14a58f99
3 changed files with 55 additions and 4 deletions

View file

@ -219,7 +219,7 @@ program
.action(createLbugLazyAction(() => import('./tool.js'), 'contextCommand'));
program
.command('impact <target>')
.command('impact [target]')
.description('Blast radius analysis: what breaks if you change a symbol')
.option('-d, --direction <dir>', 'upstream (dependants) or downstream (dependencies)', 'upstream')
.option('-r, --repo <name>', 'Target repository')

View file

@ -94,6 +94,11 @@ export async function contextCommand(
content?: boolean;
},
): Promise<void> {
// Reject a `--`-prefixed uid swallowed from a following flag (see impactCommand).
if (options?.uid?.startsWith('--')) {
cliErrorKey('tool.usage.context');
process.exit(1);
}
if (!name?.trim() && !options?.uid) {
cliErrorKey('tool.usage.context');
process.exit(1);
@ -111,7 +116,7 @@ export async function contextCommand(
}
export async function impactCommand(
target: string,
target?: string,
options?: {
direction?: string;
repo?: string;
@ -125,7 +130,16 @@ export async function impactCommand(
summaryOnly?: boolean;
},
): Promise<void> {
if (!target?.trim()) {
// A `--`-prefixed uid means Commander swallowed a following flag as the uid
// value (e.g. `impact --uid --file x` → uid === '--file'). Reject it rather
// than forwarding a garbage uid that would silently resolve to not-found.
if (options?.uid?.startsWith('--')) {
cliErrorKey('tool.usage.impact');
process.exit(1);
}
// Target is an optional positional: a uid alone is enough to resolve (parity
// with `context [name]`). Only error when neither a target nor a uid is given.
if (!target?.trim() && !options?.uid) {
cliErrorKey('tool.usage.impact');
process.exit(1);
}
@ -137,7 +151,7 @@ export async function impactCommand(
const parsedLimit = Number.isFinite(rawLimit) ? rawLimit : undefined;
const parsedOffset = Number.isFinite(rawOffset) ? rawOffset : undefined;
const result = await backend.callTool('impact', {
target,
target: target || undefined,
target_uid: options?.uid,
file_path: options?.file,
kind: options?.kind,

View file

@ -72,4 +72,41 @@ describe('CLI impact disambiguation flags (#1907)', () => {
expect(params.file_path).toBeUndefined();
expect(params.kind).toBeUndefined();
});
// U1 (#1914 review F1): impact's positional target is now optional, so a uid
// alone resolves — parity with `context [name]`.
it('resolves uid-only with no positional target (parity with context)', async () => {
await impactCommand(undefined, {
direction: 'upstream',
uid: 'Function:src/auth.ts:login',
});
expect(callTool).toHaveBeenCalledTimes(1);
const params = callTool.mock.calls[0][1] as Record<string, unknown>;
expect(params.target_uid).toBe('Function:src/auth.ts:login');
expect(params.target).toBeUndefined();
});
it('errors when neither a target nor a uid is provided', async () => {
const exitSpy = vi.spyOn(process, 'exit').mockImplementation((() => {
throw new Error('process.exit');
}) as never);
await expect(impactCommand(undefined, {})).rejects.toThrow('process.exit');
expect(exitSpy).toHaveBeenCalledWith(1);
expect(callTool).not.toHaveBeenCalled();
exitSpy.mockRestore();
});
it('rejects a --prefixed uid value (a flag swallowed by Commander) without forwarding it', async () => {
const exitSpy = vi.spyOn(process, 'exit').mockImplementation((() => {
throw new Error('process.exit');
}) as never);
await expect(impactCommand(undefined, { uid: '--file' })).rejects.toThrow('process.exit');
expect(callTool).not.toHaveBeenCalled();
exitSpy.mockRestore();
});
});