From 2643c2c7ca38950c7e02699182bac409f74f5e9e Mon Sep 17 00:00:00 2001 From: jdalton Date: Tue, 21 Apr 2026 18:37:12 -0400 Subject: [PATCH] fix(cli): return org slug, not display name, from org resolution MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Orgs whose display name differs from the slug (e.g. a name with spaces like "Example Org Ltd" vs slug "example-org-ltd") produced broken API URLs like `/v0/orgs/Example%20Org%20Ltd/full-scans` that 404'd. The CLI returned `.name` in two paths where it should have returned `.slug`. Fixed both call sites: * commands/scan/suggest-org-slug.mts — interactive prompt now uses the display name for the visible label/description but sets the choice `value` to the slug. User sees the friendly name, API calls route by slug. * commands/ci/fetch-default-org-slug.mts — CI auto-resolve no longer reads `.name` from the first org. Uses `.slug`. Also simplified iteration: fetchOrganization already returns an array via Object.values, so the previous Object.keys(organizations)[0] indirection was meaningless. Tests: * Added regression coverage in both test files for the name-differs-from-slug case using the dummy "Example Org Ltd" fixture. * Fixed the pre-existing "fetches from API" test in fetch-default-org-slug.test.mts — it mocked organizations as a dict but the real shape is an array, and asserted the buggy display-name return value. Now asserts slug. * Renamed "returns error when organization has no name" to "returns error when organization has no slug" with a matching mock payload — the new defensive check guards against missing slug, not missing name. --- .../commands/ci/fetch-default-org-slug.mts | 10 ++-- .../src/commands/scan/suggest-org-slug.mts | 12 ++--- .../ci/fetch-default-org-slug.test.mts | 47 ++++++++++++++----- .../commands/scan/suggest-org-slug.test.mts | 27 +++++++++++ 4 files changed, 71 insertions(+), 25 deletions(-) diff --git a/packages/cli/src/commands/ci/fetch-default-org-slug.mts b/packages/cli/src/commands/ci/fetch-default-org-slug.mts index ea144771c..e286cd9c8 100644 --- a/packages/cli/src/commands/ci/fetch-default-org-slug.mts +++ b/packages/cli/src/commands/ci/fetch-default-org-slug.mts @@ -29,8 +29,7 @@ export async function getDefaultOrgSlug(): Promise> { } const { organizations } = orgsCResult.data - const keys = Object.keys(organizations) - if (!keys.length) { + if (!organizations.length) { return { ok: false, message: 'Failed to establish identity', @@ -38,10 +37,9 @@ export async function getDefaultOrgSlug(): Promise> { } } - const [firstKey] = keys - const slug = firstKey - ? ((organizations as any)[firstKey]?.name ?? undefined) - : undefined + // Use `.slug` (URL-safe) — `.name` is the display label and may + // contain spaces ("Example Org Ltd") that break API URLs. + const slug = organizations[0]?.slug if (!slug) { return { ok: false, diff --git a/packages/cli/src/commands/scan/suggest-org-slug.mts b/packages/cli/src/commands/scan/suggest-org-slug.mts index 55b09dea6..b7dd8a459 100644 --- a/packages/cli/src/commands/scan/suggest-org-slug.mts +++ b/packages/cli/src/commands/scan/suggest-org-slug.mts @@ -13,19 +13,19 @@ export async function suggestOrgSlug(): Promise { return undefined } - // Ignore a failed request here. It was not the primary goal of - // running this command and reporting it only leads to end-user confusion. const { organizations } = orgsCResult.data const proceed = await select({ message: 'Missing org name; do you want to use any of these orgs for this scan?', choices: [ ...organizations.map(o => { - const name = o.name ?? o.slug + // Display the human-readable name but route with the slug — + // display names may contain spaces that break API URLs. + const display = o.name ?? o.slug return { - name: `Yes [${name}]`, - value: name, - description: `Use "${name}" as the organization`, + name: `Yes [${display}]`, + value: o.slug, + description: `Use "${display}" as the organization`, } }), { diff --git a/packages/cli/test/unit/commands/ci/fetch-default-org-slug.test.mts b/packages/cli/test/unit/commands/ci/fetch-default-org-slug.test.mts index 2075e64a3..79236be7e 100644 --- a/packages/cli/test/unit/commands/ci/fetch-default-org-slug.test.mts +++ b/packages/cli/test/unit/commands/ci/fetch-default-org-slug.test.mts @@ -97,13 +97,15 @@ describe('getDefaultOrgSlug', () => { mockFetchFn.mockResolvedValue({ ok: true, data: { - organizations: { - 'org-1': { + // fetchOrganization converts the SDK dict into an array of org + // objects before returning, so mock the array shape directly. + organizations: [ + { id: 'org-1', name: 'Test Organization', slug: 'test-org', }, - }, + ], }, }) @@ -112,7 +114,31 @@ describe('getDefaultOrgSlug', () => { expect(result).toEqual({ ok: true, message: 'Retrieved default org from server', - data: 'Test Organization', + data: 'test-org', + }) + }) + + it('returns slug (not display name) for orgs with spaces', async () => { + // Regression guard: orgs whose display name has spaces produce + // URLs like `/v0/orgs/Example%20Org%20Ltd/...` that 404. + mockFn.mockReturnValue(undefined) + mockOrgSlug.value = undefined + + mockFetchFn.mockResolvedValue({ + ok: true, + data: { + organizations: [ + { id: 'org-1', name: 'Example Org Ltd', slug: 'example-org-ltd' }, + ], + }, + }) + + const result = await getDefaultOrgSlug() + + expect(result).toEqual({ + ok: true, + message: 'Retrieved default org from server', + data: 'example-org-ltd', }) }) @@ -139,7 +165,7 @@ describe('getDefaultOrgSlug', () => { mockFetchFn.mockResolvedValue({ ok: true, data: { - organizations: {}, + organizations: [], }, }) @@ -152,20 +178,15 @@ describe('getDefaultOrgSlug', () => { }) }) - it('returns error when organization has no name', async () => { + it('returns error when organization has no slug', async () => { mockFn.mockReturnValue(undefined) mockOrgSlug.value = undefined mockFetchFn.mockResolvedValue({ ok: true, data: { - organizations: { - 'org-1': { - id: 'org-1', - slug: 'org-slug', - // Missing name field. - }, - }, + // Missing slug — defensive check in case the API ever omits it. + organizations: [{ id: 'org-1', name: 'Test Org' }], }, }) diff --git a/packages/cli/test/unit/commands/scan/suggest-org-slug.test.mts b/packages/cli/test/unit/commands/scan/suggest-org-slug.test.mts index e184c62a1..60169dfa4 100644 --- a/packages/cli/test/unit/commands/scan/suggest-org-slug.test.mts +++ b/packages/cli/test/unit/commands/scan/suggest-org-slug.test.mts @@ -136,5 +136,32 @@ describe('suggest-org-slug', () => { expect(noChoice).toBeDefined() expect(noChoice!.value).toBe('') }) + + it('returns the slug (not display name) for orgs where they differ', async () => { + // Regression guard: passing the display name through to the API + // produced 404s for orgs with spaces, e.g. + // `/v0/orgs/Example%20Org%20Ltd/...` instead of + // `/v0/orgs/example-org-ltd/...`. + mockFetchOrganization.mockResolvedValue({ + ok: true, + data: { + organizations: [ + { name: 'Example Org Ltd', slug: 'example-org-ltd' }, + ], + }, + }) + mockSelect.mockResolvedValue('example-org-ltd') + + await suggestOrgSlug() + + const callArg = mockSelect.mock.calls[0]![0] as { + choices: Array<{ name: string; value: string; description: string }> + } + // The choice value must be the slug. The visible label/description + // still use the friendlier display name. + expect(callArg.choices[0]!.value).toBe('example-org-ltd') + expect(callArg.choices[0]!.name).toContain('Example Org Ltd') + expect(callArg.choices[0]!.description).toContain('Example Org Ltd') + }) }) })