From 4b17e5fe9410db1b5aa1c139d14381c1b3c7cb26 Mon Sep 17 00:00:00 2001 From: A Ibrahim Date: Mon, 11 May 2026 16:54:41 +0200 Subject: [PATCH] refactor: make `objects set-access` access a positional arg MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `--access` flag was declared `required: true` on a spec option, which routes through `validateRequiredWhen` in `cli-core.ts`. That helper prints the error to stderr but doesn't `process.exit(1)`, leaving Node to exit 0. Other commands in the codebase that exit 1 on a missing required arg do so because the required arg is a positional — Commander's own missing-positional path exits 1. Make `access` a third positional, matching the codebase's pattern and `objects put`'s three-positional resolution shape: - `tigris objects set-access ` - `tigris objects set-access t3://my-bucket/my-file.txt public` Resolution mirrors `objects put`: when the first positional is a full `t3://bucket/key` path, the second positional becomes the access value and the third is omitted. The impl runtime-checks both for missing access and invalid value. Tests updated: - Old `--access public/private` invocations switched to positional - New case asserting t3:// path + positional access - New case asserting invalid-value path exits 1 Co-Authored-By: Claude Opus 4.7 (1M context) --- src/lib/objects/set-access.ts | 16 +++++++++++++--- src/specs.yaml | 9 ++++----- test/cli.test.ts | 26 +++++++++++++++++++++----- 3 files changed, 38 insertions(+), 13 deletions(-) diff --git a/src/lib/objects/set-access.ts b/src/lib/objects/set-access.ts index 02fb62c..675d3c7 100644 --- a/src/lib/objects/set-access.ts +++ b/src/lib/objects/set-access.ts @@ -14,20 +14,30 @@ export default async function setAccess(options: Record) { const bucketArg = getOption(options, ['bucket']); const keyArg = getOption(options, ['key']); - const access = getOption(options, ['access', 'a', 'A']); + const accessArg = getOption(options, ['access']); if (!bucketArg) { failWithError(context, 'Bucket name or path is required'); } - const { bucket, key } = resolveObjectArgs(bucketArg, keyArg); + // When the user passes a full t3://bucket/key path as the first + // positional, the second positional slot is the access value and + // there is no third. Mirrors the resolution shape in objects put. + const combined = resolveObjectArgs(bucketArg); + const bucket = combined.bucket; + const key = combined.key || keyArg; + const access = combined.key ? keyArg : accessArg; if (!key) { failWithError(context, 'Object key is required'); } + if (!access) { + failWithError(context, 'Access level is required (public or private)'); + } + if (access !== 'public' && access !== 'private') { - failWithError(context, '--access must be either "public" or "private"'); + failWithError(context, 'Access level must be either "public" or "private"'); } const config = await getStorageConfig(); diff --git a/src/specs.yaml b/src/specs.yaml index 19ca7d8..88e7ccd 100644 --- a/src/specs.yaml +++ b/src/specs.yaml @@ -1395,8 +1395,8 @@ commands: description: Set the access level (public or private) on an existing object alias: sa examples: - - "tigris objects set-access my-bucket my-file.txt --access public" - - "tigris objects set-access t3://my-bucket/my-file.txt --access private" + - "tigris objects set-access my-bucket my-file.txt public" + - "tigris objects set-access t3://my-bucket/my-file.txt private" messages: onStart: 'Updating object access...' onSuccess: "Access for '{{key}}' updated to {{access}}" @@ -1410,10 +1410,9 @@ commands: description: Key of the object (omit if bucket contains the full path) type: positional - name: access - description: Access level - alias: a + description: Access level (public or private) + type: positional options: *access_options - required: true - name: format description: Output format options: [json, table, xml] diff --git a/test/cli.test.ts b/test/cli.test.ts index 4d2a8fb..f7cf4d2 100644 --- a/test/cli.test.ts +++ b/test/cli.test.ts @@ -1756,23 +1756,39 @@ describe.skipIf(skipTests)('CLI Integration Tests', () => { runCli(`rm ${t3(testBucket)}/${testFile} -f`); }); - it('should set --access public', () => { + it('should set access to public via positional', () => { const result = runCli( - `objects set-access ${testBucket} ${testFile} --access public` + `objects set-access ${testBucket} ${testFile} public` ); expect(result.exitCode).toBe(0); }); - it('should set --access private', () => { + it('should set access to private via positional', () => { + const result = runCli( + `objects set-access ${testBucket} ${testFile} private` + ); + expect(result.exitCode).toBe(0); + }); + + it('should accept t3:// path with access as second positional', () => { const result = runCli( - `objects set-access ${testBucket} ${testFile} --access private` + `objects set-access ${t3(testBucket)}/${testFile} public` ); expect(result.exitCode).toBe(0); }); - it('should error on missing --access', () => { + it('should error when the access positional is missing', () => { const result = runCli(`objects set-access ${testBucket} ${testFile}`); expect(result.exitCode).toBe(1); + expect(result.stderr).toContain('Access level is required'); + }); + + it('should error on invalid access value', () => { + const result = runCli( + `objects set-access ${testBucket} ${testFile} maybe` + ); + expect(result.exitCode).toBe(1); + expect(result.stderr).toContain('Access level must be either'); }); });