From 1c666ebf90dfae4506cdd516b9a0cc972c19863b Mon Sep 17 00:00:00 2001 From: Nick Josevski Date: Wed, 16 Sep 2026 16:12:05 +1000 Subject: [PATCH] Allow a space id to be used wherever a space name is accepted resolveSpaceId queried /spaces?partialName= and then required an exact Name match, so a space id such as "Spaces-1795" always failed with "No spaces exist with name 'Spaces-1795'". This is surprising given push-package-action documents its space input as "The name or ID of a space", and a caller in this repo already passes spaceName: "Spaces-1". Name matching is tried first and the id lookup only runs when the value is shaped like a space id, so existing behaviour is unchanged and a space that happens to be named like an id still resolves by name. Mirrors the CLI, which prefers name then falls back to id (OctopusDeploy/cli#30). Co-Authored-By: Claude Opus 5 (1M context) --- src/spaceResolver.test.ts | 71 ++++++++++++++++++++++++++++ src/spaceResolver.ts | 99 +++++++++++++++++++++++---------------- 2 files changed, 130 insertions(+), 40 deletions(-) create mode 100644 src/spaceResolver.test.ts diff --git a/src/spaceResolver.test.ts b/src/spaceResolver.test.ts new file mode 100644 index 0000000..4c10da3 --- /dev/null +++ b/src/spaceResolver.test.ts @@ -0,0 +1,71 @@ +import type { Client } from "./client"; +import { Space } from "./features/spaces/space"; +import { resolveSpaceId } from "./spaceResolver"; + +// resolveSpaceId memoises results in module scope, so each test uses a distinct +// name/id to keep the cases independent. +function space(id: string, name: string): Space { + return { + Id: id, + Name: name, + Slug: name.toLowerCase().replace(/\s/g, "-"), + IsDefault: false, + SpaceManagersTeamMembers: [], + SpaceManagersTeams: [], + TaskQueueStopped: false, + }; +} + +function clientReturning(handlers: { byName?: Space[]; byId?: Space }): { client: Client; paths: string[] } { + const paths: string[] = []; + const client = { + debug: () => undefined, + error: () => undefined, + get: (path: string) => { + paths.push(path); + if (path.includes("partialName=")) { + const items = handlers.byName ?? []; + return Promise.resolve({ Items: items, TotalResults: items.length }); + } + if (handlers.byId) return Promise.resolve(handlers.byId); + return Promise.reject(new Error("404 not found")); + }, + } as unknown as Client; + return { client, paths }; +} + +describe("resolveSpaceId", () => { + test("resolves an exact name match", async () => { + const target = space("Spaces-11", "By Name"); + const { client } = clientReturning({ byName: [target] }); + await expect(resolveSpaceId(client, "By Name")).resolves.toBe("Spaces-11"); + }); + + test("ignores a partial name match that is not exact", async () => { + const { client } = clientReturning({ byName: [space("Spaces-12", "Partial Match Longer")] }); + await expect(resolveSpaceId(client, "Partial Match")).rejects.toThrow("Unable to resolve space name or id 'Partial Match'"); + }); + + test("resolves a space id when no name matches", async () => { + const { client } = clientReturning({ byName: [], byId: space("Spaces-1795", "Swordfish") }); + await expect(resolveSpaceId(client, "Spaces-1795")).resolves.toBe("Spaces-1795"); + }); + + test("prefers a name match over an id match", async () => { + const named = space("Spaces-13", "Spaces-9999"); + const { client, paths } = clientReturning({ byName: [named], byId: space("Spaces-9999", "Other") }); + await expect(resolveSpaceId(client, "Spaces-9999")).resolves.toBe("Spaces-13"); + expect(paths.filter((p) => !p.includes("partialName="))).toHaveLength(0); + }); + + test("does not attempt an id lookup for a value that is not id-shaped", async () => { + const { client, paths } = clientReturning({ byName: [] }); + await expect(resolveSpaceId(client, "Not An Id")).rejects.toThrow("No spaces exist with name or id 'Not An Id'"); + expect(paths.filter((p) => !p.includes("partialName="))).toHaveLength(0); + }); + + test("throws when an id-shaped value matches no space", async () => { + const { client } = clientReturning({ byName: [] }); + await expect(resolveSpaceId(client, "Spaces-4242")).rejects.toThrow("No spaces exist with name or id 'Spaces-4242'"); + }); +}); diff --git a/src/spaceResolver.ts b/src/spaceResolver.ts index 12283cd..79a7270 100644 --- a/src/spaceResolver.ts +++ b/src/spaceResolver.ts @@ -1,40 +1,59 @@ -import type { Client } from "./client"; -import { apiLocation } from "./apiLocation"; -import { Space } from "./features/spaces/space"; -import { ResourceCollection } from "./resourceCollection"; -import { BasicRepository } from "./features"; - -const knownSpaces: Record = {}; - -export async function resolveSpaceId(client: Client, spaceName: string): Promise { - if (knownSpaces[spaceName]) { - return knownSpaces[spaceName]; - } - - client.debug(`Resolving space from name '${spaceName}'`); - - const spaces = await client.get>( - `${apiLocation}/spaces?partialName=${encodeURIComponent(spaceName)}&skip=0&take=${BasicRepository.TakeAll}` - ); - let spaceId = ""; - - if (spaces.TotalResults === 0) { - client.error(`No spaces exist with name '${spaceName}'`); - throw new Error(`No spaces exist with name '${spaceName}'`); - } - - spaces.Items.forEach((space) => { - if (space.Name == spaceName) { - spaceId = space.Id; - knownSpaces[spaceName] = spaceId; - client.debug(`Resolved space name '${spaceName}' to Id ${spaceId}`); - } - }); - - if (spaceId === "") { - client.error(`Unable to resolve space name '${spaceName}'`); - throw new Error(`Unable to resolve space name '${spaceName}'`); - } - - return spaceId; -} +import type { Client } from "./client"; +import { apiLocation } from "./apiLocation"; +import { Space } from "./features/spaces/space"; +import { ResourceCollection } from "./resourceCollection"; +import { BasicRepository } from "./features"; + +const knownSpaces: Record = {}; + +// Server-assigned space ids take the form "Spaces-123". +const spaceIdFormat = /^Spaces-\d+$/; + +export async function resolveSpaceId(client: Client, spaceNameOrId: string): Promise { + if (knownSpaces[spaceNameOrId]) { + return knownSpaces[spaceNameOrId]; + } + + client.debug(`Resolving space from name or id '${spaceNameOrId}'`); + + // We prefer to match on name and only then fall back to id, matching the + // behaviour of the CLI (https://github.com/OctopusDeploy/cli/issues/30). The + // server has no single filter covering both, so the name lookup runs first + // and the id lookup only when the value is shaped like a space id. + const spaces = await client.get>( + `${apiLocation}/spaces?partialName=${encodeURIComponent(spaceNameOrId)}&skip=0&take=${BasicRepository.TakeAll}` + ); + + const spaceMatchedByName = spaces.Items.find((space) => space.Name === spaceNameOrId); + if (spaceMatchedByName) { + knownSpaces[spaceNameOrId] = spaceMatchedByName.Id; + client.debug(`Resolved space name '${spaceNameOrId}' to Id ${spaceMatchedByName.Id}`); + return spaceMatchedByName.Id; + } + + if (spaceIdFormat.test(spaceNameOrId)) { + const spaceMatchedById = await tryGetSpaceById(client, spaceNameOrId); + if (spaceMatchedById) { + knownSpaces[spaceNameOrId] = spaceMatchedById.Id; + client.debug(`Resolved space id '${spaceNameOrId}' to space '${spaceMatchedById.Name}'`); + return spaceMatchedById.Id; + } + } + + if (spaces.TotalResults === 0) { + client.error(`No spaces exist with name or id '${spaceNameOrId}'`); + throw new Error(`No spaces exist with name or id '${spaceNameOrId}'`); + } + + client.error(`Unable to resolve space name or id '${spaceNameOrId}'`); + throw new Error(`Unable to resolve space name or id '${spaceNameOrId}'`); +} + +async function tryGetSpaceById(client: Client, spaceId: string): Promise { + try { + return await client.get(`${apiLocation}/spaces/${encodeURIComponent(spaceId)}`); + } catch (error) { + client.debug(`No space exists with id '${spaceId}'`); + return undefined; + } +}