[NickJosevski]

Why

Renaming a space in Octopus silently breaks every TeamCity configuration that names it. We just hit
this: renaming one space needed PRs across five repos, and the TeamCity side alone was five
octopus_space_name params across four build types — found only by grepping the config repo,
because nothing surfaces a stale name until a build fails.

The plugin has only ever offered a free-text "Space name" field, on the connection and on all six
step forms, so every one of those is a hand-typed string that goes stale on rename.

What this does

Adds a space picker: Load spaces lists the spaces on the Octopus server and stores the chosen
space's id (Spaces-1795), which survives a rename. The name is stored alongside it purely so
the UI stays readable.

The free-text field is deliberately kept rather than replaced - see the constraint below.

The storage half is free

Both CLIs the agent drives already resolve a space by name or id, so the stored id needs no CLI
change and no wire-format change:

• Legacy octo: ApiCommand.cs declares --space as "The name or ID of a space" and calls
  Spaces.FindByNameOrIdOrFail(spaceNameOrId).
• Go CLI: client_factory.go's SpaceNameOrID, name first then id (cli#30).

That also makes this backwards compatible by construction: existing configurations store names, and
names keep resolving exactly as before.

The constraint that shaped the design

A dropdown needs credentials at edit time, and two of the three API key sources cannot provide
them:

API key source	Available at edit time?
key	Yes - but see below
parameter	No. Resolved in OctopusConnectionBuildStartProcessor, at build start
oidc	No. The token is minted per-build by the OIDC plugin

Even for key: it is stored as secure:octopus_apikey, so when editing a saved connection
TeamCity renders a placeholder and the browser does not hold the real key. The lookup therefore
sends a connection id and the server reads the stored credentials itself, falling back to posted
form values only for a connection that has not been saved yet.

So the picker cannot replace the text field. For parameter and oidc it reports why, in words,
and typing a name remains the supported path. Replacing the input with a plain <select> would have
broken exactly the OIDC setups this repo already supports.

Shape

• SpaceSelection (octopus-common) - one place that answers "which space does this step act on",
  preferring id over name. It replaces 13 near-duplicate blocks in CommandHelper and the six
  legacy *BuildProcess classes. It deliberately exposes both resolve (empty when unset, so the
  legacy path keeps omitting --space) and resolveOrDefault (substitutes "Default", as the Go
  CLI path always has) rather than flattening that difference.
• OctopusSpacesFetcher / OctopusSpacesController (octopus-server) - the lookup. Hand-rolled
  HttpURLConnection + gson following CommitHistoryFetcher, rather than pulling in
  com.octopus:octopus-sdk, which is test-scope only today and would drag a gson upgrade into the
  shipped plugin (the repo pins gson 2.2.2; the e2e module already had to override it).
• spacePicker.jsp - one shared include used by the connection form and all six step forms, so the
  markup and JS exist once. Follows the existing conventions: HTML-escaped data- attributes rather
  than server-interpolated JS string literals, textContent for admin-supplied names, $j, no
  framework.

Security

The endpoint makes an outbound request to a URL the caller influences, so it is deliberately narrow:

• caller must be logged in and hold EDIT_PROJECT on the project whose form they are editing -
  the same permission needed to configure a connection at all, so it grants no reach they did not
  already have. Without this, any logged-in user could aim the server at a URL of their choosing and
  test credentials through it.
• a named saved connection's stored credentials win over anything posted.
• the API key is never echoed back, in an error or otherwise.
• Cache-Control: no-store.

All four are covered by tests.

Precedence subtlety

A step that identified a space - by id or by name - keeps its own selection, and the id and name
are inherited from the connection together. Inheriting the connection's id alongside a name the
step set deliberately would let the id win in SpaceSelection and silently retarget the step at the
wrong space. There is a test for each direction.

Verification

• ./gradlew build green. spotlessCheck clean.

• New unit tests: 8 SpaceSelectionTest, 10 OctopusSpacesFetcherTest, 11
  OctopusSpacesControllerTest, plus 3 precedence and 2 connection-description tests. 269 tests
  across the three modules, 0 failures; all pre-existing tests pass untouched.

• New e2e OctopusSpacePickerUiTest drives the real form in a browser against the real Octopus
  container: Load spaces returns the actual space, selecting it stores the real id in
  octopus_space_id, and a parameter-sourced connection shows the explanatory message. It
  compiles but has not been executed - the Docker daemon on the machine this was written on stopped
  responding (docker ps and docker version both hang against a live OrbStack socket), so the
  container stack never came up. It needs a run before merge:

  ./gradlew :e2e:playwrightInstall
  ./gradlew :e2e:e2eTest --tests "*OctopusSpacePickerUiTest*" --tests "*OctopusConnectionUiTest*"

  OctopusConnectionUiTest is worth including in that run: it asserts the step's space field is
  hidden when the connection defines a space, and this change replaced that row with the shared
  include. The picker test also covers the one assumption not provable by unit tests - that
  props:hiddenProperty renders a field named octopus_space_id that round-trips through
  TeamCity's properties bean.

• checkLicenses fails on this branch, but it fails identically on main -
  teamcity-rest-client (no license found) and retrofit1-okhttp3-client. Pre-existing and
  unrelated; the deps added here (gson, commons-io, both already used elsewhere in the repo)
  introduce no new license findings.

Follow-ups, not in here

• create-release-action's action.yml documents space as name-only while
  push-package-action says "name or ID"; the JS api-client is genuinely name-only, fixed
  separately in Allow a space id to be used wherever a space name is accepted api-client.ts#293.
• TeamCity project ids (TeamModernDeployments_*), the Slack channel in project descriptions, and
  modern-deployment-calamari-jwt-token all still carry old team naming. Out of scope here.

[NickJosevski]

[NickJosevski]

[NickJosevski]

[matt-richardson]

Claude here (on Matt's behalf): merged main into this branch, which had gone conflicting after the release-link work and #249 (refactor: let each CLI command read its own response) landed. Two conflicts, both straightforward, plus one thing that needs your eye.

octopus-common/build.gradle wanted both sides — this branch's common-api test dependency and the JUnit/AssertJ ones #246 added.

CommandHelper took main's structure, since #249 moved several of these helpers to returning the argument array directly rather than an OctopusCommandBuilder. Your space handling was then reapplied on top, so all seven sites resolve through SpaceSelection instead of reading the space name and defaulting it locally. One of those is spaceView, which arrived on main after you branched and so still had the old shape — it's now consistent with the rest.

The part worth checking: CreateReleaseBuildProcess.createCommand decides whether it can skip the space view lookup by asking whether the step already names a space id. It was reading the space-name parameter directly — the last place in the agent still doing so after your change — so a space id picked from the new dropdown would not have been seen there, and the step would have spent a CLI call working out an id it had already been given. Not a breakage, since the lookup still resolves correctly, but it made this branch's whole point a no-op on that path. It now goes through SpaceSelection.resolve, which prefers the id and returns empty when neither is set. That's a line beyond conflict resolution, in a file this branch doesn't otherwise touch, so revert it if you'd rather own that decision yourself.

Verified: build test with --rerun-tasks (311 unit tests, 0 failures) and the release-link plus space e2e tests against the container stack. I diffed the file lists to confirm nothing of yours was dropped and nothing leaked in from main beyond that one deliberate file.

[matt-richardson]

Claude here: superseded by the space-picker work now on main: #256, #257, #258, #261, #262, #263, #264, #265, #267 and #268. The space field is now an autocomplete that lists spaces for stored, parameter-held and OIDC API keys, and stores the chosen space's id.

Not carried over: SpacePickerManualTest (from #240) and its section in docs/e2e-tests.md. The driver targets this PR's <select> markup and would need rewriting for the autocomplete field.