fix: --variable silently ignored on Create release steps that do not deploy - #220
Merged
Merged
Conversation
NickJosevski
marked this pull request as draft
August 12, 2026 03:40
NickJosevski
force-pushed
the
fix/204-prompted-variables-on-create-release
branch
2 times, most recently
from
August 19, 2026 02:17
2ffa13c to
2fffffb
Compare
NickJosevski
marked this pull request as ready for review
August 19, 2026 02:18
NickJosevski
force-pushed
the
fix/204-prompted-variables-on-create-release
branch
from
August 19, 2026 06:18
2fffffb to
82b0b05
Compare
| if (!VARIABLE_ARGUMENT.matcher(additionalArguments).find()) { | ||
| return; | ||
| } | ||
| if (StringUtil.isEmptyOrSpaces(properties.get(constants.getDeployToKey()))) { |
There was a problem hiding this comment.
Isn't it possible that someone might have manually supplied the --deployTo argument hit this error message now? I guess this is only relevant for the old octo CLI though so I'm unsure what the fix should be as it looks like both CLIs ares funnelled through this path
NickJosevski
added a commit
that referenced
this pull request
Aug 20, 2026
…argument The legacy octo CLI takes the deploy target on create-release, so a step that supplies it through the additional arguments rather than the "Deploy to" field does deploy, and --variable does take effect. Verified against the octo 9.1.7 bundled in this repo: create-release --deployTo Development --variable ImageTag:7.7.7 creates the release, deploys it, and the prompted variable reaches the script. Reported by sathvikkumar-octo on #220. The check now fires only on a step that deploys via neither route. octo's option parser is case insensitive and accepts -, -- and / prefixes, so the pattern matches all of those forms. Which CLI will run is an agent-side decision, so this cannot be narrowed further server-side. On the new CLI --deployTo is not a "release create" argument at all and the step fails with "unknown flag", which is loud enough to diagnose without help from this check. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
NickJosevski
added a commit
that referenced
this pull request
Aug 20, 2026
…argument The legacy octo CLI takes the deploy target on create-release, so a step that supplies it through the additional arguments rather than the "Deploy to" field does deploy, and --variable does take effect. Verified against the octo 9.1.7 bundled in this repo: create-release --deployTo Development --variable ImageTag:7.7.7 creates the release, deploys it, and the prompted variable reaches the script. Reported by sathvikkumar-octo on #220. The check now fires only on a step that deploys via neither route. octo's option parser is case insensitive and accepts -, -- and / prefixes, so the pattern matches all of those forms. Which CLI will run is an agent-side decision, so this cannot be narrowed further server-side. On the new CLI --deployTo is not a "release create" argument at all and the step fails with "unknown flag", which is loud enough to diagnose without help from this check. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
NickJosevski
force-pushed
the
fix/204-prompted-variables-on-create-release
branch
from
August 20, 2026 05:22
4ac1597 to
ffbfe1e
Compare
NickJosevski
added a commit
that referenced
this pull request
Aug 20, 2026
…argument The legacy octo CLI takes the deploy target on create-release, so a step that supplies it through the additional arguments rather than the "Deploy to" field does deploy, and --variable does take effect. Verified against the octo 9.1.7 bundled in this repo: create-release --deployTo Development --variable ImageTag:7.7.7 creates the release, deploys it, and the prompted variable reaches the script. Reported by sathvikkumar-octo on #220. The check now fires only on a step that deploys via neither route. octo's option parser is case insensitive and accepts -, -- and / prefixes, so the pattern matches all of those forms. Which CLI will run is an agent-side decision, so this cannot be narrowed further server-side. On the new CLI --deployTo is not a "release create" argument at all and the step fails with "unknown flag", which is loud enough to diagnose without help from this check. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
NickJosevski
force-pushed
the
fix/204-prompted-variables-on-create-release
branch
from
August 20, 2026 05:29
ffbfe1e to
30214fe
Compare
NickJosevski
enabled auto-merge (rebase)
August 20, 2026 05:29
The "Additional command line arguments" field is shared by every command a step runs, so each command strips the arguments belonging to the other. Two bugs in that filtering silently discarded valid arguments: - Switches that take no value (--update-variables, --guided-failure, --ignore-existing, ...) consumed the token that followed them. So "--ignore-existing --variable ImageTag:1.5.2" left the deploy command with a bare "ImageTag:1.5.2" and no --variable at all. - Matching used String.contains against a comma-joined list, so --var and --e were stripped as substrings of --variable and --environment. Match argument names exactly against a Set, and only consume the following token as a value when it does not itself start with "-". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…deploy Prompted variables are supplied when a release is deployed. The Octopus CLI accepts --variable on a create-only command and then ignores it, so putting it in "Additional command line arguments" on a Create release step appears to do nothing, and the deployment later fails with "Please provide a variable for the prompted value". Reject that combination when the step is saved, pointing at either the "Deploy to" field on this step or a separate Deploy release step, and say the same thing in the field's hint. Fixes #204 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…argument The legacy octo CLI takes the deploy target on create-release, so a step that supplies it through the additional arguments rather than the "Deploy to" field does deploy, and --variable does take effect. Verified against the octo 9.1.7 bundled in this repo: create-release --deployTo Development --variable ImageTag:7.7.7 creates the release, deploys it, and the prompted variable reaches the script. Reported by sathvikkumar-octo on #220. The check now fires only on a step that deploys via neither route. octo's option parser is case insensitive and accepts -, -- and / prefixes, so the pattern matches all of those forms. Which CLI will run is an agent-side decision, so this cannot be narrowed further server-side. On the new CLI --deployTo is not a "release create" argument at all and the step fails with "unknown flag", which is loud enough to diagnose without help from this check. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
-v is --variable on "release deploy" but --version on "release create". It was missing from deployReleaseAdditionalArgumentsToBeIgnored and only got stripped by accident, as a substring of --variable under the old String.contains matching. Exact Set matching drops that coverage, so -v reached release create and the CLI rejected the release number: The release number 'ImageTag:1.5.2' does not appear to be a valid version number. Verified against a live Octopus by generating the commands from CommandHelper itself: with -v stripped, release create succeeds and the deploy command keeps -v ImageTag:1.5.2, and the prompted variable reaches the script. The release number has its own field, so losing -v as a create-side alias for --version is the cheaper side of the ambiguity. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
NickJosevski
force-pushed
the
fix/204-prompted-variables-on-create-release
branch
from
August 20, 2026 05:31
30214fe to
27b1c6f
Compare
sathvikkumar-octo
approved these changes
Aug 20, 2026
NickJosevski
added a commit
that referenced
this pull request
Aug 20, 2026
…argument The legacy octo CLI takes the deploy target on create-release, so a step that supplies it through the additional arguments rather than the "Deploy to" field does deploy, and --variable does take effect. Verified against the octo 9.1.7 bundled in this repo: create-release --deployTo Development --variable ImageTag:7.7.7 creates the release, deploys it, and the prompted variable reaches the script. Reported by sathvikkumar-octo on #220. The check now fires only on a step that deploys via neither route. octo's option parser is case insensitive and accepts -, -- and / prefixes, so the pattern matches all of those forms. Which CLI will run is an agent-side decision, so this cannot be narrowed further server-side. On the new CLI --deployTo is not a "release create" argument at all and the step fails with "unknown flag", which is loud enough to diagnose without help from this check. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #204.
What #204 actually is
The reported symptom is not an argument-mangling bug. The legacy path (
OctopusCreateReleaseBuildProcess, which is whatocto9.1.7 uses) appends the additional arguments verbatim, and the splitter handles--variable ImageTag:1.5.2,--variable=ImageTag:1.5.2and quoted forms correctly — matching the command line in the ticket, which shows the flag was passed to the CLI.Prompted variables are supplied when a release is deployed.
octo create-release --variableon a create-only invocation is accepted and then ignored, which is why it "seems to be ignored" and why the deployment later fails withPlease provide a variable for the prompted value ImageTag. That matches Clare's conclusion on the issue.So from the plugin's side the defect is that it fails silently. This PR makes it fail loudly, at configuration time.
Changes
1. Reject
--variableon a Create release step with no "Deploy to" (PropertiesValidator.checkPromptedVariablesOnlyWhenDeploying, wired intoOctopusCreateReleaseRunType):Matching is anchored on whitespace boundaries so
--update-variablesdoes not trip it. Being server-side, it covers both the legacy and new CLI paths.2. Fix two real bugs in
CommandHelper.sanitizeCommandArgs. Separate from the reported symptom, but the same bug class — this code silently drops valid arguments on the new CLI path:--update-variables,--guided-failure,--ignore-existing, …) consumed the token after them.--ignore-existing --variable ImageTag:1.5.2left the deploy command with a bareImageTag:1.5.2and no--variable— --variable=VALUE argument seems to be ignored in the CreateRelease TeamCity Step #204's exact symptom, for real, in code.String.containsagainst a comma-joined string, so--varand--ewere stripped as substrings of--variableand--environment.Now matches argument names exactly against a
Set, and only consumes the following token as a value when it does not itself start with-. The documented trade-off: a value that legitimately starts with-survives as a stray positional rather than causing a real argument to be dropped.3. Field hint on the Create release form noting that
--variableonly takes effect when Deploy to is set.Tests
Five new cases in
CommandHelperTest(including the--ignore-existing --variableregression and exact-name matching) and five inOctopusCreateReleaseRunTypeValidationTest../gradlew check distZipis green.Verified locally
Built and loaded into TeamCity 2025.03.3; confirmed the Create release form renders with the new hint and the
error_octopus_additionalcommandlineargumentsspan — the failure mode unit tests cannot catch. The config-time error itself is only reachable through the UI (the REST API bypassesPropertiesProcessorentirely), so that one still wants a click-through on review.Still needed outside this repo
The docs change Clare asks for on the issue: create-release should say
--variablerequires--deployto.🤖 Generated with Claude Code