Skip to content

Commit 5295a3f

Browse files
designcodeclaude
andauthored
chore: remove dead code and improve unknown command errors (#64)
* chore: remove dead code in auth modules Remove isAuthenticated() from provider.ts and clearCredentials() from storage.ts — both exported but never imported anywhere. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: improve error message for unknown subcommands Replace confusing "too many arguments" error with a clear "Unknown command" message that lists available subcommands. Covers all levels: top-level, subcommand groups, nested groups, and commands with a default subcommand. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: sweep stale test buckets in integration test setup Previous failed/crashed CI runs left behind tigris-cli-test-* buckets with no cleanup mechanism. Add a best-effort sweeper in beforeAll that deletes test buckets older than 2 hours. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: reduce stale bucket sweep threshold to 30 minutes Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: use recursive rm for stale bucket sweep Wildcard `rm t3://bucket/* -f` doesn't match folder markers like `folder/`, leaving the bucket non-empty and undeletable. Use `rm t3://bucket -r -f` instead to remove everything including folders. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: use -r -f on bucket contents before deleting bucket rm -r -f can't be applied directly on a bucket. First recursively remove all contents (including folder markers) with bucket/*, then delete the empty bucket. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: pass force flag to removeBucket in delete and rm commands The force option was not being forwarded to removeBucket, preventing force-deletion of non-empty buckets. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: only treat implemented commands as known in subcommand check knownNames included all spec commands regardless of whether they had implementations. An unimplemented command name would bypass the "Unknown command" error and silently fall through. Now knownNames is built from the same implemented-commands filter as the available list. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent f226f33 commit 5295a3f

6 files changed

Lines changed: 106 additions & 28 deletions

File tree

src/auth/provider.ts

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -319,18 +319,6 @@ export async function getStorageConfig(options?: {
319319
}
320320
}
321321

322-
/**
323-
* Check if user is authenticated (either method)
324-
*/
325-
export async function isAuthenticated(): Promise<boolean> {
326-
return (
327-
hasAwsProfile() ||
328-
getEnvCredentials() !== null ||
329-
(await getLoginMethod()) !== null ||
330-
getStoredCredentials() !== null
331-
);
332-
}
333-
334322
/**
335323
* Get storage config with organization overlay from selected org.
336324
*/

src/auth/storage.ts

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -391,17 +391,6 @@ export async function storeTemporaryCredentials(
391391
await writeConfig(config);
392392
}
393393

394-
/**
395-
* Clear saved credentials (from configure)
396-
*/
397-
export async function clearCredentials(): Promise<void> {
398-
const config = readConfig();
399-
if (config.credentials) {
400-
delete config.credentials.saved;
401-
}
402-
await writeConfig(config);
403-
}
404-
405394
/**
406395
* Store the login method used by the user
407396
*/

src/cli-core.ts

Lines changed: 80 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,60 @@ import { Command as CommanderCommand } from 'commander';
88

99
import type { Argument, CommandSpec, Specs } from './types.js';
1010

11+
/**
12+
* Check if the first positional arg is an unrecognized subcommand.
13+
* If so, print a helpful error and exit. Returns the positional args
14+
* for the caller to use if no error is found.
15+
*/
16+
function checkUnknownSubcommand(
17+
actionArgs: unknown[],
18+
spec: { commands?: CommandSpec[]; name?: string },
19+
currentPath: string[],
20+
specs: Specs,
21+
hasImplementation: ImplementationChecker
22+
): string[] {
23+
// Commander passes the Command object as the last arg; its .args has positionals
24+
const last = actionArgs[actionArgs.length - 1];
25+
const positional =
26+
typeof last === 'object' && last !== null && 'args' in last
27+
? ((last as { args: string[] }).args as string[])
28+
: (actionArgs.filter((a) => typeof a === 'string') as string[]);
29+
30+
if (positional.length === 0) return positional;
31+
32+
const first = positional[0];
33+
const subcommands = spec.commands ?? [];
34+
const implemented = subcommands.filter((c) =>
35+
commandHasAnyImplementation(c, [...currentPath, c.name], hasImplementation)
36+
);
37+
const knownNames = new Set(
38+
implemented.flatMap((c) => [
39+
c.name,
40+
...(Array.isArray(c.alias) ? c.alias : c.alias ? [c.alias] : []),
41+
])
42+
);
43+
44+
if (!knownNames.has(first)) {
45+
const available = implemented.map((c) => c.name);
46+
const pathLabel =
47+
currentPath.length > 0
48+
? `'${first}' for '${currentPath.join(' ')}'`
49+
: `'${first}'`;
50+
console.error(`Unknown command ${pathLabel}.`);
51+
if (available.length > 0) {
52+
console.error(`Available commands: ${available.join(', ')}`);
53+
}
54+
const helpCmd =
55+
currentPath.length > 0
56+
? `${specs.name} ${currentPath.join(' ')} help`
57+
: `${specs.name} help`;
58+
console.error(`\nRun "${helpCmd}" for usage.`);
59+
process.exit(1);
60+
}
61+
62+
return positional;
63+
}
64+
1165
export interface ModuleLoader {
1266
(commandPath: string[]): Promise<{
1367
module: Record<string, unknown> | null;
@@ -458,10 +512,17 @@ export function registerCommands(
458512
...(defaultCmd.arguments || []),
459513
]);
460514
addArgumentsToCommand(cmd, allArguments);
515+
cmd.allowExcessArguments(true);
461516

462517
cmd.action(async (...args) => {
463518
const options = args.pop();
464-
const positionalArgs = args;
519+
const positionalArgs = checkUnknownSubcommand(
520+
[options],
521+
spec,
522+
currentPath,
523+
specs,
524+
hasImplementation
525+
);
465526

466527
if (
467528
allArguments.length > 0 &&
@@ -486,7 +547,15 @@ export function registerCommands(
486547
});
487548
}
488549
} else {
489-
cmd.action(() => {
550+
cmd.allowExcessArguments(true);
551+
cmd.action((...args) => {
552+
checkUnknownSubcommand(
553+
args,
554+
spec,
555+
currentPath,
556+
specs,
557+
hasImplementation
558+
);
490559
showCommandHelp(specs, spec, currentPath, hasImplementation);
491560
});
492561
}
@@ -559,7 +628,15 @@ export function createProgram(config: CLIConfig): CommanderCommand {
559628
console.log(version);
560629
});
561630

562-
program.action(() => {
631+
program.allowExcessArguments(true);
632+
program.action((...args) => {
633+
checkUnknownSubcommand(
634+
args,
635+
{ commands: specs.commands },
636+
[],
637+
specs,
638+
hasImplementation
639+
);
563640
showMainHelp(specs, version, hasImplementation);
564641
});
565642

src/lib/buckets/delete.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ export default async function deleteBucket(options: Record<string, unknown>) {
4444
const deleted: string[] = [];
4545
const errors: { name: string; error: string }[] = [];
4646
for (const name of bucketNames) {
47-
const { error } = await removeBucket(name, { config });
47+
const { error } = await removeBucket(name, { config, force });
4848

4949
if (error) {
5050
printFailure(context, error.message, { name });

src/lib/rm.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ export default async function rm(options: Record<string, unknown>) {
5151
}
5252
}
5353

54-
const { error } = await removeBucket(bucket, { config });
54+
const { error } = await removeBucket(bucket, { config, force });
5555

5656
if (error) {
5757
exitWithError(error);

test/cli.test.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -270,6 +270,30 @@ describe.skipIf(skipTests)('CLI Integration Tests', () => {
270270
console.warn('Failed to setup credentials, tests may fail');
271271
}
272272

273+
// Sweep stale test buckets from previous failed runs
274+
const staleThresholdMs = 30 * 60 * 1000; // 30 minutes
275+
const listResult = runCli('buckets list --format json');
276+
if (listResult.exitCode === 0 && listResult.stdout.trim()) {
277+
try {
278+
const buckets = JSON.parse(listResult.stdout.trim()) as Array<{
279+
name: string;
280+
created: string;
281+
}>;
282+
const now = Date.now();
283+
for (const bucket of buckets) {
284+
if (!bucket.name.startsWith('tigris-cli-test-')) continue;
285+
const age = now - new Date(bucket.created).getTime();
286+
if (age > staleThresholdMs) {
287+
console.log(`Sweeping stale test bucket: ${bucket.name}`);
288+
runCli(`rm ${t3(bucket.name)}/* -r -f`);
289+
runCli(`rm ${t3(bucket.name)} -f`);
290+
}
291+
}
292+
} catch {
293+
// Best-effort cleanup — don't block tests
294+
}
295+
}
296+
273297
console.log(`Test prefix: ${testPrefix}`);
274298
console.log(`Creating test bucket: ${testBucket}`);
275299
// Use mk command instead of buckets create to avoid interactive prompts

0 commit comments

Comments
 (0)