From 385aad4b68125a42b0986ddbf28d196dc2a34294 Mon Sep 17 00:00:00 2001 From: jdalton Date: Fri, 17 Apr 2026 20:07:08 -0400 Subject: [PATCH 1/6] fix(scan create): detect --default-branch= misuse MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `--default-branch` is a boolean meow flag, so `--default-branch=main` silently becomes `defaultBranch=true` with the `"main"` portion discarded. Users with that (reasonable) intuition ended up with scans that weren't tagged with any branch name and didn't show up in the Main/PR dashboard tabs. Pre-flight check in `run()` scans the raw argv for `--default-branch=`. Values that coerce to boolean (`true` / `false`, any case) are let through; anything else is treated as a misuse and fails with: ✗ "--default-branch=main" looks like you meant the branch name "main". --default-branch is a boolean flag; pass the branch name with --branch instead: socket scan create --branch main --default-branch Exits with code 2 (invalid usage), consistent with other flag validation failures in this command. Added tests: * misuse form with a branch-name value is caught and logged * explicit `--default-branch=true|false|TRUE` all pass through * bare `--default-branch` with paired `--branch main` flows through --- .../cli/src/commands/scan/cmd-scan-create.mts | 43 ++++++++++- .../commands/scan/cmd-scan-create.test.mts | 74 +++++++++++++++++++ 2 files changed, 113 insertions(+), 4 deletions(-) diff --git a/packages/cli/src/commands/scan/cmd-scan-create.mts b/packages/cli/src/commands/scan/cmd-scan-create.mts index c1de899a2..c98dbf4c1 100644 --- a/packages/cli/src/commands/scan/cmd-scan-create.mts +++ b/packages/cli/src/commands/scan/cmd-scan-create.mts @@ -202,10 +202,24 @@ const generalFlags: MeowFlags = { }, } -export const cmdScanCreate = { - description, - hidden, - run, +// Scan argv for `--default-branch=`. The flag is declared +// boolean, so meow coerces `--default-branch=main` to `true` and discards +// "main" — silently leaving the scan without a branch tag. +function findDefaultBranchValueMisuse( + argv: readonly string[], +): string | undefined { + for (const arg of argv) { + if (!arg.startsWith('--default-branch=')) { + continue + } + const value = arg.slice('--default-branch='.length) + const normalized = value.toLowerCase() + if (normalized === 'true' || normalized === 'false' || value === '') { + continue + } + return value + } + return undefined } async function run( @@ -272,6 +286,21 @@ async function run( `, } + // Detect the common `--default-branch=main` misuse before meow parses. + // `--default-branch` is a boolean — meow/yargs-parser treats + // `--default-branch=main` as `defaultBranch=true` and silently drops + // the "main" portion, so the user's scan gets tagged without the + // intended branch name and doesn't appear in the Main/PR dashboard + // tabs. Fail fast with a suggestion toward the correct form. + const defaultBranchMisuse = findDefaultBranchValueMisuse(argv) + if (defaultBranchMisuse) { + logger.fail( + `"--default-branch=${defaultBranchMisuse}" looks like you meant the branch name "${defaultBranchMisuse}".\n--default-branch is a boolean flag; pass the branch name with --branch instead:\n socket scan create --branch ${defaultBranchMisuse} --default-branch`, + ) + process.exitCode = 2 + return + } + const cli = meowOrExit({ argv, config, @@ -680,3 +709,9 @@ async function run( workspace: (workspace && String(workspace)) || '', }) } + +export const cmdScanCreate = { + description, + hidden, + run, +} diff --git a/packages/cli/test/unit/commands/scan/cmd-scan-create.test.mts b/packages/cli/test/unit/commands/scan/cmd-scan-create.test.mts index be57cd47c..09f0a464b 100644 --- a/packages/cli/test/unit/commands/scan/cmd-scan-create.test.mts +++ b/packages/cli/test/unit/commands/scan/cmd-scan-create.test.mts @@ -1366,5 +1366,79 @@ describe('cmd-scan-create', () => { expect(mockHandleCreateNewScan).not.toHaveBeenCalled() }) }) + + describe('--default-branch misuse detection', () => { + // --default-branch is a boolean flag; meow silently discards the + // `=` portion on `--default-branch=`. Catch that + // pattern before meow parses so users get a clear error pointing + // at the right shape (`--branch --default-branch`). + it('fails when --default-branch= is passed with a branch name', async () => { + await cmdScanCreate.run( + ['--org', 'test-org', '--default-branch=main', '.'], + importMeta, + context, + ) + + expect(process.exitCode).toBe(2) + expect(mockHandleCreateNewScan).not.toHaveBeenCalled() + expect(mockLogger.fail).toHaveBeenCalledWith( + expect.stringContaining( + '"--default-branch=main" looks like you meant the branch name "main"', + ), + ) + expect(mockLogger.fail).toHaveBeenCalledWith( + expect.stringContaining('--branch main --default-branch'), + ) + }) + + it.each([ + '--default-branch=true', + '--default-branch=false', + '--default-branch=TRUE', + ])('allows %s (explicit boolean form)', async arg => { + mockHasDefaultApiToken.mockReturnValueOnce(true) + + await cmdScanCreate.run( + [ + '--org', + 'test-org', + '--branch', + 'main', + arg, + '.', + '--no-interactive', + ], + importMeta, + context, + ) + + // meow parses the flag normally and flows through to handleCreateNewScan. + expect(mockLogger.fail).not.toHaveBeenCalledWith( + expect.stringContaining('looks like you meant the branch name'), + ) + }) + + it('allows bare --default-branch (default truthy form)', async () => { + mockHasDefaultApiToken.mockReturnValueOnce(true) + + await cmdScanCreate.run( + [ + '--org', + 'test-org', + '--branch', + 'main', + '--default-branch', + '.', + '--no-interactive', + ], + importMeta, + context, + ) + + expect(mockLogger.fail).not.toHaveBeenCalledWith( + expect.stringContaining('looks like you meant the branch name'), + ) + }) + }) }) }) From d461dc757620a65e02531677879c1ebbac1f0f7f Mon Sep 17 00:00:00 2001 From: jdalton Date: Fri, 17 Apr 2026 22:29:59 -0400 Subject: [PATCH 2/6] fix(scan create): also detect --defaultBranch= camelCase misuse Addresses Cursor bugbot feedback on PR #1230. yargs-parser expands camelCase flag names, so users can type either --default-branch= or --defaultBranch= from the shell. The pre-flight misuse check now tests both prefixes. Added a regression test for the camelCase variant. --- .../cli/src/commands/scan/cmd-scan-create.mts | 14 +++++++++----- .../unit/commands/scan/cmd-scan-create.test.mts | 16 ++++++++++++++++ 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/packages/cli/src/commands/scan/cmd-scan-create.mts b/packages/cli/src/commands/scan/cmd-scan-create.mts index c98dbf4c1..7dda25fff 100644 --- a/packages/cli/src/commands/scan/cmd-scan-create.mts +++ b/packages/cli/src/commands/scan/cmd-scan-create.mts @@ -202,17 +202,21 @@ const generalFlags: MeowFlags = { }, } -// Scan argv for `--default-branch=`. The flag is declared -// boolean, so meow coerces `--default-branch=main` to `true` and discards -// "main" — silently leaving the scan without a branch tag. +// Scan argv for `--default-branch=` (both kebab-case +// and yargs-parser's camelCase expansion). The flag is declared boolean, +// so meow coerces `--default-branch=main` to `true` and discards "main" +// — silently leaving the scan without a branch tag. +const DEFAULT_BRANCH_PREFIXES = ['--default-branch=', '--defaultBranch='] + function findDefaultBranchValueMisuse( argv: readonly string[], ): string | undefined { for (const arg of argv) { - if (!arg.startsWith('--default-branch=')) { + const prefix = DEFAULT_BRANCH_PREFIXES.find(p => arg.startsWith(p)) + if (!prefix) { continue } - const value = arg.slice('--default-branch='.length) + const value = arg.slice(prefix.length) const normalized = value.toLowerCase() if (normalized === 'true' || normalized === 'false' || value === '') { continue diff --git a/packages/cli/test/unit/commands/scan/cmd-scan-create.test.mts b/packages/cli/test/unit/commands/scan/cmd-scan-create.test.mts index 09f0a464b..3b0ba67c4 100644 --- a/packages/cli/test/unit/commands/scan/cmd-scan-create.test.mts +++ b/packages/cli/test/unit/commands/scan/cmd-scan-create.test.mts @@ -1391,6 +1391,22 @@ describe('cmd-scan-create', () => { ) }) + it('also catches the camelCase --defaultBranch= variant', async () => { + // yargs-parser expands camelCase, so users can type either + // form from the shell. See Cursor bugbot feedback on PR #1230. + await cmdScanCreate.run( + ['--org', 'test-org', '--defaultBranch=main', '.'], + importMeta, + context, + ) + + expect(process.exitCode).toBe(2) + expect(mockHandleCreateNewScan).not.toHaveBeenCalled() + expect(mockLogger.fail).toHaveBeenCalledWith( + expect.stringContaining('looks like you meant the branch name "main"'), + ) + }) + it.each([ '--default-branch=true', '--default-branch=false', From 54f92cc8ced9edfce2778d961181894da43c47a6 Mon Sep 17 00:00:00 2001 From: jdalton Date: Fri, 17 Apr 2026 22:57:14 -0400 Subject: [PATCH 3/6] fix(scan create): quote user's actual flag form in misuse error Addresses Cursor bugbot feedback on PR #1230. findDefaultBranchValueMisuse only returned the extracted value, so the error message always quoted '--default-branch=' even when the user typed the camelCase '--defaultBranch=' form. Return the matched prefix alongside the value so the error quotes what the user actually typed. --- packages/cli/src/commands/scan/cmd-scan-create.mts | 10 +++++++--- .../test/unit/commands/scan/cmd-scan-create.test.mts | 5 +++++ 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/packages/cli/src/commands/scan/cmd-scan-create.mts b/packages/cli/src/commands/scan/cmd-scan-create.mts index 7dda25fff..8b02c0556 100644 --- a/packages/cli/src/commands/scan/cmd-scan-create.mts +++ b/packages/cli/src/commands/scan/cmd-scan-create.mts @@ -210,7 +210,7 @@ const DEFAULT_BRANCH_PREFIXES = ['--default-branch=', '--defaultBranch='] function findDefaultBranchValueMisuse( argv: readonly string[], -): string | undefined { +): { prefix: string; value: string } | undefined { for (const arg of argv) { const prefix = DEFAULT_BRANCH_PREFIXES.find(p => arg.startsWith(p)) if (!prefix) { @@ -221,7 +221,7 @@ function findDefaultBranchValueMisuse( if (normalized === 'true' || normalized === 'false' || value === '') { continue } - return value + return { prefix, value } } return undefined } @@ -298,8 +298,12 @@ async function run( // tabs. Fail fast with a suggestion toward the correct form. const defaultBranchMisuse = findDefaultBranchValueMisuse(argv) if (defaultBranchMisuse) { + const { prefix, value } = defaultBranchMisuse + // Strip the trailing `=` from the matched prefix when naming the + // canonical flag in the suggestion — users should always be told + // to use the kebab-case form. logger.fail( - `"--default-branch=${defaultBranchMisuse}" looks like you meant the branch name "${defaultBranchMisuse}".\n--default-branch is a boolean flag; pass the branch name with --branch instead:\n socket scan create --branch ${defaultBranchMisuse} --default-branch`, + `"${prefix}${value}" looks like you meant the branch name "${value}".\n--default-branch is a boolean flag; pass the branch name with --branch instead:\n socket scan create --branch ${value} --default-branch`, ) process.exitCode = 2 return diff --git a/packages/cli/test/unit/commands/scan/cmd-scan-create.test.mts b/packages/cli/test/unit/commands/scan/cmd-scan-create.test.mts index 3b0ba67c4..28a0ae9fc 100644 --- a/packages/cli/test/unit/commands/scan/cmd-scan-create.test.mts +++ b/packages/cli/test/unit/commands/scan/cmd-scan-create.test.mts @@ -1405,6 +1405,11 @@ describe('cmd-scan-create', () => { expect(mockLogger.fail).toHaveBeenCalledWith( expect.stringContaining('looks like you meant the branch name "main"'), ) + // Error quotes the exact form the user typed so there's no + // confusion about whether the error applies to their input. + expect(mockLogger.fail).toHaveBeenCalledWith( + expect.stringContaining('"--defaultBranch=main"'), + ) }) it.each([ From 5de8478264643ca457cee90602eeacbbed7dfcca Mon Sep 17 00:00:00 2001 From: jdalton Date: Mon, 20 Apr 2026 14:11:45 -0400 Subject: [PATCH 4/6] chore(scan create): trim redundant comments and restore export order Match the sibling `cmd-scan-*` convention of placing `export const cmd*` immediately before `async function run`, and drop comments that restated the code rather than explaining non-obvious *why*. The one remaining comment on the misuse check keeps the meow/yargs-parser coercion detail, which isn't derivable from reading the code. --- .../cli/src/commands/scan/cmd-scan-create.mts | 29 +++++++------------ .../commands/scan/cmd-scan-create.test.mts | 9 ------ 2 files changed, 10 insertions(+), 28 deletions(-) diff --git a/packages/cli/src/commands/scan/cmd-scan-create.mts b/packages/cli/src/commands/scan/cmd-scan-create.mts index 8b02c0556..79529e3a0 100644 --- a/packages/cli/src/commands/scan/cmd-scan-create.mts +++ b/packages/cli/src/commands/scan/cmd-scan-create.mts @@ -202,10 +202,6 @@ const generalFlags: MeowFlags = { }, } -// Scan argv for `--default-branch=` (both kebab-case -// and yargs-parser's camelCase expansion). The flag is declared boolean, -// so meow coerces `--default-branch=main` to `true` and discards "main" -// — silently leaving the scan without a branch tag. const DEFAULT_BRANCH_PREFIXES = ['--default-branch=', '--defaultBranch='] function findDefaultBranchValueMisuse( @@ -226,6 +222,12 @@ function findDefaultBranchValueMisuse( return undefined } +export const cmdScanCreate = { + description, + hidden, + run, +} + async function run( argv: string[] | readonly string[], importMeta: ImportMeta, @@ -290,18 +292,13 @@ async function run( `, } - // Detect the common `--default-branch=main` misuse before meow parses. - // `--default-branch` is a boolean — meow/yargs-parser treats - // `--default-branch=main` as `defaultBranch=true` and silently drops - // the "main" portion, so the user's scan gets tagged without the - // intended branch name and doesn't appear in the Main/PR dashboard - // tabs. Fail fast with a suggestion toward the correct form. + // `--default-branch` is declared boolean, so meow/yargs-parser + // coerces `--default-branch=main` to `defaultBranch=true` and drops + // "main" silently — the resulting scan is untagged and invisible in + // the Main/PR dashboard tabs. Catch that shape before meow parses. const defaultBranchMisuse = findDefaultBranchValueMisuse(argv) if (defaultBranchMisuse) { const { prefix, value } = defaultBranchMisuse - // Strip the trailing `=` from the matched prefix when naming the - // canonical flag in the suggestion — users should always be told - // to use the kebab-case form. logger.fail( `"${prefix}${value}" looks like you meant the branch name "${value}".\n--default-branch is a boolean flag; pass the branch name with --branch instead:\n socket scan create --branch ${value} --default-branch`, ) @@ -717,9 +714,3 @@ async function run( workspace: (workspace && String(workspace)) || '', }) } - -export const cmdScanCreate = { - description, - hidden, - run, -} diff --git a/packages/cli/test/unit/commands/scan/cmd-scan-create.test.mts b/packages/cli/test/unit/commands/scan/cmd-scan-create.test.mts index 28a0ae9fc..d70c3c51c 100644 --- a/packages/cli/test/unit/commands/scan/cmd-scan-create.test.mts +++ b/packages/cli/test/unit/commands/scan/cmd-scan-create.test.mts @@ -1368,10 +1368,6 @@ describe('cmd-scan-create', () => { }) describe('--default-branch misuse detection', () => { - // --default-branch is a boolean flag; meow silently discards the - // `=` portion on `--default-branch=`. Catch that - // pattern before meow parses so users get a clear error pointing - // at the right shape (`--branch --default-branch`). it('fails when --default-branch= is passed with a branch name', async () => { await cmdScanCreate.run( ['--org', 'test-org', '--default-branch=main', '.'], @@ -1392,8 +1388,6 @@ describe('cmd-scan-create', () => { }) it('also catches the camelCase --defaultBranch= variant', async () => { - // yargs-parser expands camelCase, so users can type either - // form from the shell. See Cursor bugbot feedback on PR #1230. await cmdScanCreate.run( ['--org', 'test-org', '--defaultBranch=main', '.'], importMeta, @@ -1405,8 +1399,6 @@ describe('cmd-scan-create', () => { expect(mockLogger.fail).toHaveBeenCalledWith( expect.stringContaining('looks like you meant the branch name "main"'), ) - // Error quotes the exact form the user typed so there's no - // confusion about whether the error applies to their input. expect(mockLogger.fail).toHaveBeenCalledWith( expect.stringContaining('"--defaultBranch=main"'), ) @@ -1433,7 +1425,6 @@ describe('cmd-scan-create', () => { context, ) - // meow parses the flag normally and flows through to handleCreateNewScan. expect(mockLogger.fail).not.toHaveBeenCalledWith( expect.stringContaining('looks like you meant the branch name'), ) From 7191f946bf8fc891f9ed5898ed8de7e9e7423628 Mon Sep 17 00:00:00 2001 From: jdalton Date: Mon, 20 Apr 2026 16:14:26 -0400 Subject: [PATCH 5/6] feat(scan create): detect legacy space-separated --default-branch misuse --- .../cli/src/commands/scan/cmd-scan-create.mts | 57 +++++++++++++--- .../commands/scan/cmd-scan-create.test.mts | 66 ++++++++++++++++++- 2 files changed, 113 insertions(+), 10 deletions(-) diff --git a/packages/cli/src/commands/scan/cmd-scan-create.mts b/packages/cli/src/commands/scan/cmd-scan-create.mts index 79529e3a0..c620dfe09 100644 --- a/packages/cli/src/commands/scan/cmd-scan-create.mts +++ b/packages/cli/src/commands/scan/cmd-scan-create.mts @@ -202,11 +202,22 @@ const generalFlags: MeowFlags = { }, } -const DEFAULT_BRANCH_PREFIXES = ['--default-branch=', '--defaultBranch='] +const DEFAULT_BRANCH_FLAGS = ['--default-branch', '--defaultBranch'] +const DEFAULT_BRANCH_PREFIXES = DEFAULT_BRANCH_FLAGS.map(f => `${f}=`) + +function isBareIdentifier(token: string): boolean { + // Accept only tokens that look like a plain branch name. Anything + // with a path separator, dot, or colon is almost certainly a target + // path, URL, or something else the user meant as a positional arg. + return /^[A-Za-z0-9_-]+$/.test(token) +} function findDefaultBranchValueMisuse( argv: readonly string[], -): { prefix: string; value: string } | undefined { +): { form: string; value: string } | undefined { + // `--default-branch=main` — unambiguous: the `=` form attaches a + // value to what meow treats as a boolean flag, so the value is + // silently dropped. for (const arg of argv) { const prefix = DEFAULT_BRANCH_PREFIXES.find(p => arg.startsWith(p)) if (!prefix) { @@ -217,7 +228,32 @@ function findDefaultBranchValueMisuse( if (normalized === 'true' || normalized === 'false' || value === '') { continue } - return { prefix, value } + return { form: `${prefix}${value}`, value } + } + // `--default-branch main` — ambiguous in general (the next token + // could be a positional target path), but if the next token is a + // bare identifier (no `/`, `.`, `:`) AND the user didn't also pass + // `--branch` / `-b`, it's almost certainly a mis-typed branch name. + const hasBranchFlag = argv.some( + arg => + arg === '--branch' || + arg === '-b' || + arg.startsWith('--branch=') || + arg.startsWith('-b='), + ) + if (hasBranchFlag) { + return undefined + } + for (let i = 0; i < argv.length - 1; i += 1) { + const arg = argv[i]! + if (!DEFAULT_BRANCH_FLAGS.includes(arg)) { + continue + } + const next = argv[i + 1]! + if (next.startsWith('-') || !isBareIdentifier(next)) { + continue + } + return { form: `${arg} ${next}`, value: next } } return undefined } @@ -293,14 +329,19 @@ async function run( } // `--default-branch` is declared boolean, so meow/yargs-parser - // coerces `--default-branch=main` to `defaultBranch=true` and drops - // "main" silently — the resulting scan is untagged and invisible in - // the Main/PR dashboard tabs. Catch that shape before meow parses. + // silently drops any value attached to it — the resulting scan is + // untagged and invisible in the Main/PR dashboard tabs. Catch that + // shape before meow parses so the user sees an actionable error + // instead of a mysteriously-mislabelled scan hours later. const defaultBranchMisuse = findDefaultBranchValueMisuse(argv) if (defaultBranchMisuse) { - const { prefix, value } = defaultBranchMisuse + const { form, value } = defaultBranchMisuse logger.fail( - `"${prefix}${value}" looks like you meant the branch name "${value}".\n--default-branch is a boolean flag; pass the branch name with --branch instead:\n socket scan create --branch ${value} --default-branch`, + `"${form}" looks like you meant to name the branch "${value}", but --default-branch is a boolean flag (no value).\n\n` + + `To scan "${value}" as the default branch, use --branch for the name and --default-branch as a flag:\n` + + ` socket scan create --branch ${value} --default-branch\n\n` + + `To scan a non-default branch, drop --default-branch:\n` + + ` socket scan create --branch ${value}`, ) process.exitCode = 2 return diff --git a/packages/cli/test/unit/commands/scan/cmd-scan-create.test.mts b/packages/cli/test/unit/commands/scan/cmd-scan-create.test.mts index d70c3c51c..5a76205f5 100644 --- a/packages/cli/test/unit/commands/scan/cmd-scan-create.test.mts +++ b/packages/cli/test/unit/commands/scan/cmd-scan-create.test.mts @@ -1379,7 +1379,7 @@ describe('cmd-scan-create', () => { expect(mockHandleCreateNewScan).not.toHaveBeenCalled() expect(mockLogger.fail).toHaveBeenCalledWith( expect.stringContaining( - '"--default-branch=main" looks like you meant the branch name "main"', + '"--default-branch=main" looks like you meant to name the branch "main"', ), ) expect(mockLogger.fail).toHaveBeenCalledWith( @@ -1397,13 +1397,75 @@ describe('cmd-scan-create', () => { expect(process.exitCode).toBe(2) expect(mockHandleCreateNewScan).not.toHaveBeenCalled() expect(mockLogger.fail).toHaveBeenCalledWith( - expect.stringContaining('looks like you meant the branch name "main"'), + expect.stringContaining( + 'looks like you meant to name the branch "main"', + ), ) expect(mockLogger.fail).toHaveBeenCalledWith( expect.stringContaining('"--defaultBranch=main"'), ) }) + it('catches the legacy space-separated --default-branch form', async () => { + await cmdScanCreate.run( + ['--org', 'test-org', '--default-branch', 'main', '.'], + importMeta, + context, + ) + + expect(process.exitCode).toBe(2) + expect(mockHandleCreateNewScan).not.toHaveBeenCalled() + expect(mockLogger.fail).toHaveBeenCalledWith( + expect.stringContaining( + '"--default-branch main" looks like you meant to name the branch "main"', + ), + ) + }) + + it('leaves the space-separated form alone when --branch is also passed', async () => { + mockHasDefaultApiToken.mockReturnValueOnce(true) + + await cmdScanCreate.run( + [ + '--org', + 'test-org', + '--branch', + 'main', + '--default-branch', + '.', + '--no-interactive', + ], + importMeta, + context, + ) + + expect(mockLogger.fail).not.toHaveBeenCalledWith( + expect.stringContaining('looks like you meant'), + ) + }) + + it('does not misfire when the next token looks like a target path', async () => { + mockHasDefaultApiToken.mockReturnValueOnce(true) + + // `./some/dir` has path separators, so it is a positional target, + // not a mistyped branch name. + await cmdScanCreate.run( + [ + '--org', + 'test-org', + '--default-branch', + './some/dir', + '--no-interactive', + ], + importMeta, + context, + ) + + expect(mockLogger.fail).not.toHaveBeenCalledWith( + expect.stringContaining('looks like you meant'), + ) + }) + it.each([ '--default-branch=true', '--default-branch=false', From a0d3bb4cef86ddebf15a605c0b1e178b4f0a9268 Mon Sep 17 00:00:00 2001 From: jdalton Date: Mon, 20 Apr 2026 18:11:38 -0400 Subject: [PATCH 6/6] feat(cli): rename --default-branch (scan create) to --make-default-branch Ends the --default-branch overload by aligning each command's flag name with the Socket API field it triggers: - scan create: new --make-default-branch (bool) mirrors `make_default_branch`. Legacy --default-branch / --defaultBranch kept as a deprecated boolean alias (declared as its own flag because meow's `aliases` forwarding was unreliable inside this command's flag set). Deprecation warning fires when the legacy name is used; misuse heuristic still catches --default-branch= and --default-branch on the deprecated alias. - repository create / repository update: flag unchanged (already matches `default_branch`). Added empty-value validation that rejects bare --default-branch and --default-branch= instead of silently persisting a blank default-branch name. Help text in cmd-scan-create.mts rewritten to describe what the flag actually does (reassigns the repo's default-branch pointer). Tests cover: primary flag happy path, primary flag misuse detection, deprecation warning on legacy flag, back-compat wiring of legacy flag, and empty-value rejection on both repository commands. --- CHANGELOG.md | 7 + .../repository/repository-command-factory.mts | 43 ++++++ .../cli/src/commands/scan/cmd-scan-create.mts | 83 ++++++++--- .../repository/cmd-repository-create.test.mts | 60 ++++++++ .../repository/cmd-repository-update.test.mts | 31 ++++ .../commands/scan/cmd-scan-create.test.mts | 141 +++++++++++++++++- 6 files changed, 347 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 187edda2a..42a95a61e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,10 +28,17 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). - Updated to @socketsecurity/socket-patch@1.2.0. - Updated Coana CLI to v14.12.148. +- `socket scan create` now accepts `--make-default-branch` (mirrors the `make_default_branch` API field) instead of `--default-branch`. The old name keeps working but emits a deprecation warning. + +### Deprecated + +- `socket scan create --default-branch` / `--defaultBranch` — use `--make-default-branch` instead. The legacy names still work during the deprecation window but emit a warning. ### Fixed - Prevent heap overflow in large monorepo scans by using streaming-based filtering to avoid accumulating all file paths in memory before filtering. +- `socket scan create` now rejects `--default-branch=` and `--default-branch ` (space-separated) with an actionable error instead of silently dropping the branch name. Scans that used the misuse shape were getting recorded without a branch tag and disappearing from the Main/PR dashboard tabs. +- `socket repository create` / `socket repository update` now reject bare `--default-branch` (no value) and `--default-branch=` (empty value). Previously both persisted a blank default-branch name on the repo record. ## [2.1.0](https://github.com/SocketDev/socket-cli/releases/tag/v2.1.0) - 2025-11-02 diff --git a/packages/cli/src/commands/repository/repository-command-factory.mts b/packages/cli/src/commands/repository/repository-command-factory.mts index 78d48133b..c7f94ecef 100644 --- a/packages/cli/src/commands/repository/repository-command-factory.mts +++ b/packages/cli/src/commands/repository/repository-command-factory.mts @@ -1,3 +1,5 @@ +import { getDefaultLogger } from '@socketsecurity/lib/logger' + import { FLAG_JSON, FLAG_MARKDOWN } from '../../constants/cli.mts' import { V1_MIGRATION_GUIDE_URL } from '../../constants/socket.mts' import { @@ -40,6 +42,29 @@ type RepositoryCommandSpec = { needsRepoName?: boolean } +// If the user wrote `--default-branch` (bare, no value) or +// `--default-branch=`, meow would coerce it to an empty string and +// silently persist a blank default-branch name on the repo record. +// Detect that before meow parses so we can stop with an actionable +// error instead of saving junk data. +function findEmptyDefaultBranch( + argv: readonly string[], +): 'bare' | 'empty-value' | undefined { + for (let i = 0; i < argv.length; i += 1) { + const arg = argv[i]! + if (arg === '--default-branch=' || arg === '--defaultBranch=') { + return 'empty-value' + } + if (arg === '--default-branch' || arg === '--defaultBranch') { + const next = argv[i + 1] + if (next === undefined || next.startsWith('-')) { + return 'bare' + } + } + } + return undefined +} + export function createRepositoryCommand(spec: RepositoryCommandSpec) { return { description: spec.description, @@ -49,6 +74,24 @@ export function createRepositoryCommand(spec: RepositoryCommandSpec) { importMeta: ImportMeta, { parentName }: CliCommandContext, ): Promise { + // Only guard the commands that actually accept `--default-branch` + // as a string (create / update). The list/view/delete commands + // don't, so the check is a no-op for them. + if ( + (spec.commandName === 'create' || spec.commandName === 'update') && + spec.extraFlags?.['defaultBranch'] + ) { + const emptyShape = findEmptyDefaultBranch(argv) + if (emptyShape) { + getDefaultLogger().fail( + emptyShape === 'empty-value' + ? '--default-branch requires a value (e.g. --default-branch=main). Leaving it empty would persist a blank default-branch name on the repo record.' + : '--default-branch requires a value (e.g. --default-branch=main). Bare --default-branch with no value would persist a blank default-branch name on the repo record.', + ) + process.exitCode = 2 + return + } + } const config: CliCommandConfig = { commandName: spec.commandName, description: spec.description, diff --git a/packages/cli/src/commands/scan/cmd-scan-create.mts b/packages/cli/src/commands/scan/cmd-scan-create.mts index c620dfe09..fc33239e1 100644 --- a/packages/cli/src/commands/scan/cmd-scan-create.mts +++ b/packages/cli/src/commands/scan/cmd-scan-create.mts @@ -52,6 +52,7 @@ interface ScanCreateFlags { committers: string cwd: string defaultBranch: boolean + makeDefaultBranch: boolean interactive: boolean json: boolean markdown: boolean @@ -130,11 +131,22 @@ const generalFlags: MeowFlags = { default: '', description: 'working directory, defaults to process.cwd()', }, + makeDefaultBranch: { + type: 'boolean', + default: false, + description: + 'Reassign the repo\'s default-branch pointer at Socket to the branch of this scan. The previous default-branch designation is replaced. Mirrors the `make_default_branch` API field.', + }, + // Deprecated alias for `--make-default-branch`. Declared as its own + // boolean flag (rather than via meow `aliases`) because meow's alias + // forwarding doesn't reliably propagate values in this command's + // large flag set. We merge it onto `makeDefaultBranch` after parsing. defaultBranch: { type: 'boolean', default: false, description: - 'Set the default branch of the repository to the branch of this full-scan. Should only need to be done once, for example for the "main" or "master" branch.', + 'Deprecated alias for --make-default-branch. Kept working for back-compat; emits a deprecation warning on use.', + hidden: true, }, interactive: { type: 'boolean', @@ -202,9 +214,28 @@ const generalFlags: MeowFlags = { }, } -const DEFAULT_BRANCH_FLAGS = ['--default-branch', '--defaultBranch'] +// Legacy flag names kept working via meow aliases on `makeDefaultBranch`. +// Detected here so we can warn on use and keep the misuse heuristic +// working against both the primary and legacy names. +const LEGACY_DEFAULT_BRANCH_FLAGS = ['--default-branch', '--defaultBranch'] +const LEGACY_DEFAULT_BRANCH_PREFIXES = LEGACY_DEFAULT_BRANCH_FLAGS.map( + f => `${f}=`, +) +const DEFAULT_BRANCH_FLAGS = [ + '--make-default-branch', + '--makeDefaultBranch', + ...LEGACY_DEFAULT_BRANCH_FLAGS, +] const DEFAULT_BRANCH_PREFIXES = DEFAULT_BRANCH_FLAGS.map(f => `${f}=`) +function hasLegacyDefaultBranchFlag(argv: readonly string[]): boolean { + return argv.some( + arg => + LEGACY_DEFAULT_BRANCH_FLAGS.includes(arg) || + LEGACY_DEFAULT_BRANCH_PREFIXES.some(p => arg.startsWith(p)), + ) +} + function isBareIdentifier(token: string): boolean { // Accept only tokens that look like a plain branch name. Anything // with a path separator, dot, or colon is almost certainly a target @@ -309,8 +340,10 @@ async function run( The --repo and --branch flags tell Socket to associate this Scan with that repo/branch. The names will show up on your dashboard on the Socket website. - Note: for a first run you probably want to set --default-branch to indicate - the default branch name, like "main" or "master". + Note: on a first scan you probably want to pass --make-default-branch so + Socket records this branch ("main", "master", etc.) as your repo's + default branch. Subsequent scans don't need the flag unless you're + reassigning the default-branch pointer to a different branch. The ${socketDashboardLink('/org/YOURORG/alerts', '"alerts page"')} will show the results from the last scan designated as the "pending head" on the branch @@ -328,25 +361,35 @@ async function run( `, } - // `--default-branch` is declared boolean, so meow/yargs-parser - // silently drops any value attached to it — the resulting scan is - // untagged and invisible in the Main/PR dashboard tabs. Catch that - // shape before meow parses so the user sees an actionable error - // instead of a mysteriously-mislabelled scan hours later. + // `--make-default-branch` (and its deprecated alias `--default-branch`) + // is a boolean flag, so meow/yargs-parser silently drops any value + // attached to it — the resulting scan is untagged and invisible in the + // Main/PR dashboard tabs. Catch that shape before meow parses so the + // user sees an actionable error instead of a mysteriously-mislabelled + // scan hours later. const defaultBranchMisuse = findDefaultBranchValueMisuse(argv) if (defaultBranchMisuse) { const { form, value } = defaultBranchMisuse logger.fail( - `"${form}" looks like you meant to name the branch "${value}", but --default-branch is a boolean flag (no value).\n\n` + - `To scan "${value}" as the default branch, use --branch for the name and --default-branch as a flag:\n` + - ` socket scan create --branch ${value} --default-branch\n\n` + - `To scan a non-default branch, drop --default-branch:\n` + + `"${form}" looks like you meant to name the branch "${value}", but --make-default-branch is a boolean flag (no value).\n\n` + + `To scan "${value}" as the default branch, use --branch for the name and --make-default-branch as a flag:\n` + + ` socket scan create --branch ${value} --make-default-branch\n\n` + + `To scan a non-default branch, drop --make-default-branch:\n` + ` socket scan create --branch ${value}`, ) process.exitCode = 2 return } + // `--default-branch` / `--defaultBranch` is kept working via meow's + // aliases, but nudge callers to migrate so we can eventually retire + // the legacy name. + if (hasLegacyDefaultBranchFlag(argv)) { + logger.warn( + '--default-branch is deprecated on `socket scan create`; use --make-default-branch instead. The old flag still works for now.', + ) + } + const cli = meowOrExit({ argv, config, @@ -359,8 +402,9 @@ async function run( commitMessage, committers, cwd: cwdOverride, - defaultBranch, + defaultBranch: legacyDefaultBranch, interactive = true, + makeDefaultBranch: makeDefaultBranchFlag, json, markdown, org: orgFlag, @@ -386,6 +430,11 @@ async function run( tmp, } = cli.flags as unknown as ScanCreateFlags + // Merge the legacy --default-branch flag into the primary. Both are + // declared as separate boolean flags in the config (see the comment + // on the `defaultBranch` flag definition above). + const makeDefaultBranch = makeDefaultBranchFlag || legacyDefaultBranch + // Validate ecosystem values. const reachEcosystems: PURL_Type[] = [] const reachEcosystemsRaw = cmdFlagValueToArray(cli.flags['reachEcosystems']) @@ -606,8 +655,8 @@ async function run( }, { nook: true, - test: !defaultBranch || !!branchName, - message: 'When --default-branch is set, --branch is mandatory', + test: !makeDefaultBranch || !!branchName, + message: 'When --make-default-branch is set, --branch is mandatory', fail: 'missing branch name', }, { @@ -719,7 +768,7 @@ async function run( commitMessage: (commitMessage && String(commitMessage)) || '', committers: (committers && String(committers)) || '', cwd, - defaultBranch: Boolean(defaultBranch), + defaultBranch: Boolean(makeDefaultBranch), interactive: Boolean(interactive), orgSlug, outputKind, diff --git a/packages/cli/test/unit/commands/repository/cmd-repository-create.test.mts b/packages/cli/test/unit/commands/repository/cmd-repository-create.test.mts index bad892979..2e974a613 100644 --- a/packages/cli/test/unit/commands/repository/cmd-repository-create.test.mts +++ b/packages/cli/test/unit/commands/repository/cmd-repository-create.test.mts @@ -391,6 +391,66 @@ describe('cmd-repository-create', () => { ) }) + describe('--default-branch empty-value detection', () => { + it('fails when --default-branch= is passed with no value', async () => { + mockHasDefaultApiToken.mockReturnValueOnce(true) + + await cmdRepositoryCreate.run( + ['test-repo', '--default-branch=', '--no-interactive'], + importMeta, + context, + ) + + expect(process.exitCode).toBe(2) + expect(mockHandleCreateRepo).not.toHaveBeenCalled() + expect(mockLogger.fail).toHaveBeenCalledWith( + expect.stringContaining('--default-branch requires a value'), + ) + }) + + it('fails when --default-branch is followed by another flag (bare form)', async () => { + mockHasDefaultApiToken.mockReturnValueOnce(true) + + await cmdRepositoryCreate.run( + ['test-repo', '--default-branch', '--no-interactive'], + importMeta, + context, + ) + + expect(process.exitCode).toBe(2) + expect(mockHandleCreateRepo).not.toHaveBeenCalled() + expect(mockLogger.fail).toHaveBeenCalledWith( + expect.stringContaining('--default-branch requires a value'), + ) + }) + + it('fails when --default-branch is the last argv token (bare form)', async () => { + mockHasDefaultApiToken.mockReturnValueOnce(true) + + await cmdRepositoryCreate.run( + ['test-repo', '--default-branch'], + importMeta, + context, + ) + + expect(process.exitCode).toBe(2) + expect(mockHandleCreateRepo).not.toHaveBeenCalled() + }) + + it('also catches the camelCase --defaultBranch variant', async () => { + mockHasDefaultApiToken.mockReturnValueOnce(true) + + await cmdRepositoryCreate.run( + ['test-repo', '--defaultBranch=', '--no-interactive'], + importMeta, + context, + ) + + expect(process.exitCode).toBe(2) + expect(mockHandleCreateRepo).not.toHaveBeenCalled() + }) + }) + it('should handle empty string values for optional flags', async () => { mockHasDefaultApiToken.mockReturnValueOnce(true) diff --git a/packages/cli/test/unit/commands/repository/cmd-repository-update.test.mts b/packages/cli/test/unit/commands/repository/cmd-repository-update.test.mts index 9930bb810..0b49a63e6 100644 --- a/packages/cli/test/unit/commands/repository/cmd-repository-update.test.mts +++ b/packages/cli/test/unit/commands/repository/cmd-repository-update.test.mts @@ -539,5 +539,36 @@ describe('cmd-repository-update', () => { 'text', ) }) + + describe('--default-branch empty-value detection', () => { + it('fails when --default-branch= is passed with no value', async () => { + mockHasDefaultApiToken.mockReturnValueOnce(true) + + await cmdRepositoryUpdate.run( + ['test-repo', '--default-branch=', '--no-interactive'], + importMeta, + context, + ) + + expect(process.exitCode).toBe(2) + expect(mockHandleUpdateRepo).not.toHaveBeenCalled() + expect(mockLogger.fail).toHaveBeenCalledWith( + expect.stringContaining('--default-branch requires a value'), + ) + }) + + it('fails on bare --default-branch followed by another flag', async () => { + mockHasDefaultApiToken.mockReturnValueOnce(true) + + await cmdRepositoryUpdate.run( + ['test-repo', '--default-branch', '--no-interactive'], + importMeta, + context, + ) + + expect(process.exitCode).toBe(2) + expect(mockHandleUpdateRepo).not.toHaveBeenCalled() + }) + }) }) }) diff --git a/packages/cli/test/unit/commands/scan/cmd-scan-create.test.mts b/packages/cli/test/unit/commands/scan/cmd-scan-create.test.mts index 5a76205f5..05b545977 100644 --- a/packages/cli/test/unit/commands/scan/cmd-scan-create.test.mts +++ b/packages/cli/test/unit/commands/scan/cmd-scan-create.test.mts @@ -1383,7 +1383,7 @@ describe('cmd-scan-create', () => { ), ) expect(mockLogger.fail).toHaveBeenCalledWith( - expect.stringContaining('--branch main --default-branch'), + expect.stringContaining('--branch main --make-default-branch'), ) }) @@ -1513,6 +1513,145 @@ describe('cmd-scan-create', () => { expect.stringContaining('looks like you meant the branch name'), ) }) + + it('catches --make-default-branch= misuse on the primary flag', async () => { + await cmdScanCreate.run( + ['--org', 'test-org', '--make-default-branch=main', '.'], + importMeta, + context, + ) + + expect(process.exitCode).toBe(2) + expect(mockHandleCreateNewScan).not.toHaveBeenCalled() + expect(mockLogger.fail).toHaveBeenCalledWith( + expect.stringContaining( + '"--make-default-branch=main" looks like you meant to name the branch "main"', + ), + ) + }) + }) + + describe('--make-default-branch primary flag', () => { + it('passes --make-default-branch through to handleCreateNewScan', async () => { + mockHasDefaultApiToken.mockReturnValueOnce(true) + + await cmdScanCreate.run( + [ + '--org', + 'test-org', + '--branch', + 'main', + '--make-default-branch', + '.', + '--no-interactive', + ], + importMeta, + context, + ) + + expect(mockHandleCreateNewScan).toHaveBeenCalledWith( + expect.objectContaining({ + defaultBranch: true, + branchName: 'main', + }), + ) + }) + + it('does not emit the deprecation warning for the primary name', async () => { + mockHasDefaultApiToken.mockReturnValueOnce(true) + + await cmdScanCreate.run( + [ + '--org', + 'test-org', + '--branch', + 'main', + '--make-default-branch', + '.', + '--no-interactive', + ], + importMeta, + context, + ) + + expect(mockLogger.warn).not.toHaveBeenCalledWith( + expect.stringContaining('--default-branch is deprecated'), + ) + }) + }) + + describe('--default-branch deprecation warning', () => { + it('warns when the legacy --default-branch flag is used', async () => { + mockHasDefaultApiToken.mockReturnValueOnce(true) + + await cmdScanCreate.run( + [ + '--org', + 'test-org', + '--branch', + 'main', + '--default-branch', + '.', + '--no-interactive', + ], + importMeta, + context, + ) + + expect(mockLogger.warn).toHaveBeenCalledWith( + expect.stringContaining('--default-branch is deprecated'), + ) + expect(mockLogger.warn).toHaveBeenCalledWith( + expect.stringContaining('use --make-default-branch'), + ) + }) + + it('warns on the legacy camelCase --defaultBranch name', async () => { + mockHasDefaultApiToken.mockReturnValueOnce(true) + + await cmdScanCreate.run( + [ + '--org', + 'test-org', + '--branch', + 'main', + '--defaultBranch', + '.', + '--no-interactive', + ], + importMeta, + context, + ) + + expect(mockLogger.warn).toHaveBeenCalledWith( + expect.stringContaining('--default-branch is deprecated'), + ) + }) + + it('still wires the legacy flag through to handleCreateNewScan (back-compat)', async () => { + mockHasDefaultApiToken.mockReturnValueOnce(true) + + await cmdScanCreate.run( + [ + '--org', + 'test-org', + '--branch', + 'main', + '--default-branch', + '.', + '--no-interactive', + ], + importMeta, + context, + ) + + expect(mockHandleCreateNewScan).toHaveBeenCalledWith( + expect.objectContaining({ + defaultBranch: true, + branchName: 'main', + }), + ) + }) }) }) })