Skip to content

Comments

Invoke CLI extension#889

Merged
stephanos merged 7 commits intotemporalio:mainfrom
stephanos:call-cliext
Dec 18, 2025
Merged

Invoke CLI extension#889
stephanos merged 7 commits intotemporalio:mainfrom
stephanos:call-cliext

Conversation

@stephanos
Copy link
Contributor

@stephanos stephanos commented Dec 15, 2025

What was changed

CLI now invokes extensions in accordance with CLI Extensions proposal.

Note that the help command integration part is going to be in a follow-up PR.

Why?

Allow Temporal CLI Extensions.

Checklist

  1. Closes

  2. How was this tested:

  1. Any docs updates needed?

@stephanos stephanos force-pushed the call-cliext branch 4 times, most recently from 0d1e799 to a55cee8 Compare December 15, 2025 22:01
return nil, false
}

cmd := exec.CommandContext(ctx, extPath, extArgs...)
Copy link
Contributor Author

@stephanos stephanos Dec 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interrupt signal handling was called out in the CLI extension proposal as an area of active search; here we decide to go with the option of aborting the sub-command on interrupt to unblock this work. This will be re-evaluated after gathering more data.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may be good enough. On nix-based platforms, I think we should consider tunnelling signals (e.g. sigint) and on context timeout, sending sigterm in this case instead of sigkill, but on Windows hard kill like this is acceptable. We will need to test I think.

@stephanos stephanos force-pushed the call-cliext branch 5 times, most recently from e4f973d to bddc00b Compare December 15, 2025 22:36
//
// If temporal-foo-bar_baz is found, returns (path, ["--flag=value"]).
// If temporal-foo is found, returns (path, ["--flag=value", "bar-baz"]).
func lookupExtension(args []string) (path string, extArgs []string) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The CLI extension proposal describes the question of "whether flags of parent commands can still be placed before the extension command" as an area of active research. For now we require them to be placed after the extension command to disambiguate. This will be re-evaluated after gathering more data.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you should find the most applicable built-in sub-command, remove all common flags on that subcommand, then do your extension resolution (and pass along all of those flags to the subcommand). And you should fail if there are unrecognized parent/common flags before the subcommand. For instance, I would expect:

  • temporal --output json my-command --some-bool-arg to run temporal-something --output json --some-bool-arg
  • temporal --no-json-shorthand-payloads my-command to run temporal-my-command --no-json-shorthand-payloads
  • temporal --address 1.2.3.4:5678 workflow --tls diagram to run temporal-workflow-diagram --address 1.2.3.4:5678 --tls
  • temporal workflow --some-bool-arg diagram to fail with "unrecognized flag --some-bool-arg"
  • temporal --some-arg some-value workflow diagram to fail with "unrecognized flag --some-arg"

I have not investigated how to do this yet or how much of it is possible, but you cannot comfortably know which flags accept values and which are standalone to do some kind of arbitrary parsing. May have to put a ArbitraryArgs on every command. I can help with a PoC of this if needed.

@stephanos stephanos changed the title invoke CLI extension Invoke CLI extension Dec 15, 2025
@stephanos stephanos marked this pull request as ready for review December 15, 2025 23:35
@stephanos stephanos requested review from a team as code owners December 15, 2025 23:35
@stephanos stephanos force-pushed the call-cliext branch 2 times, most recently from 18e1453 to 2ff2d54 Compare December 16, 2025 23:29
@stephanos stephanos force-pushed the call-cliext branch 7 times, most recently from 1edb099 to 4f129e8 Compare December 17, 2025 22:10
@stephanos stephanos force-pushed the call-cliext branch 3 times, most recently from 635b21c to 165ca33 Compare December 17, 2025 22:25
Copy link
Member

@cretz cretz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great. I appreciate the test building an executable. I assume there are no backwards incompatibilities for users today.

@stephanos stephanos force-pushed the call-cliext branch 5 times, most recently from 1041e26 to 10a279d Compare December 18, 2025 18:27
@stephanos stephanos merged commit 168728b into temporalio:main Dec 18, 2025
8 checks passed
@stephanos stephanos deleted the call-cliext branch December 18, 2025 18:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants