Skip to content

Commit 3506b6d

Browse files
authored
Merge branch 'main' into codex/disable-aws-imds-worker-deployment-test
2 parents b6f3437 + 1a064e5 commit 3506b6d

3 files changed

Lines changed: 53 additions & 6 deletions

File tree

internal/temporalcli/commands.extension.go

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -31,19 +31,23 @@ func tryExecuteExtension(cctx *CommandContext, tcmd *TemporalCommand) (error, bo
3131
// Find the deepest matching built-in command and remaining args.
3232
foundCmd, remainingArgs, findErr := tcmd.Command.Find(cctx.Options.Args)
3333

34-
// Check if remaining args include positional args (not just flags).
35-
// If not, a built-in command fully handles this - no extension needed.
36-
hasPosArgs := slices.ContainsFunc(remainingArgs, isPosArg)
37-
if findErr == nil && !hasPosArgs {
38-
return nil, false
39-
}
34+
// Cobra normally adds --help/-h before parsing, but extension dispatch
35+
// pre-parses flags before Cobra's execution path runs. We Initialize it so that
36+
// help is treated as a normal CLI flag instead of surfacing as pflag.ErrHelp from flag parsing.
37+
foundCmd.InitDefaultHelpFlag()
4038

4139
// Group args into these lists:
4240
// - cliParseArgs: args to validate (subset of cliPassArgs)
4341
// - cliPassArgs: known CLI args to pass to extension
4442
// - extArgs: args to pass to extension and use for extension lookup
4543
cliParseArgs, cliPassArgs, extArgs := groupArgs(foundCmd, remainingArgs)
4644

45+
// Check if remaining args include positional args (not just flags).
46+
// If not, a built-in command fully handles this - no extension needed.
47+
if findErr == nil && !slices.ContainsFunc(extArgs, isPosArg) {
48+
return nil, false
49+
}
50+
4751
// Search for an extension executable.
4852
cmdPrefix := strings.Fields(foundCmd.CommandPath())
4953
extPath, extArgs := lookupExtension(cmdPrefix, extArgs)
@@ -95,6 +99,13 @@ func groupArgs(foundCmd *cobra.Command, args []string) (cliParseArgs, cliPassArg
9599

96100
name, hasInline := parseFlagArg(arg)
97101
if f, takesValue := lookupFlag(foundCmd, name); f != nil {
102+
// Help flag after positional args should go to extArgs so it
103+
// gets forwarded to extensions (e.g. "temporal foo bar --help").
104+
// Before positional args it stays in cliPassArgs for Cobra to handle.
105+
if f.Name == "help" && seenPos {
106+
extArgs = append(extArgs, arg)
107+
continue
108+
}
98109
// Known CLI flag: goes to cliPassArgs.
99110
// Flags in cliArgsToParseForExtension also go to cliParseArgs.
100111
shouldParse := cliArgsToParseForExtension[f.Name]

internal/temporalcli/commands.extension_test.go

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,19 @@ func TestExtension_DoesNotOverrideBuiltinCommand(t *testing.T) {
8888
res := h.Execute("workflow", "list", "--help")
8989
assert.Contains(t, res.Stdout.String(), "List Workflow Executions")
9090
})
91+
92+
t.Run("subcommand with value flags", func(t *testing.T) {
93+
for _, args := range [][]string{
94+
{"workflow", "list", "--address", "123", "--help"},
95+
{"workflow", "list", "--help", "--address", "123"},
96+
{"workflow", "list", "--namespace", "foo", "--help"},
97+
} {
98+
res := h.Execute(args...)
99+
assert.Contains(t, res.Stdout.String(), "List Workflow Executions")
100+
assert.NotContains(t, res.Stderr.String(), "pflag: help requested")
101+
assert.NoError(t, res.Err)
102+
}
103+
})
91104
}
92105

93106
func TestExtension_Flags(t *testing.T) {
@@ -149,6 +162,14 @@ func TestExtension_Flags(t *testing.T) {
149162
{args: "workflow diagram foo --flag value", want: "temporal-workflow-diagram-foo --flag value"}, // nested commands
150163
{args: "workflow diagram --output invalid", want: "temporal-workflow-diagram --output invalid"}, // invalid value passed through
151164

165+
// Help flags are forwarded to extensions, not handled locally.
166+
167+
{args: "foo --help", want: "temporal-foo --help"},
168+
{args: "foo -h", want: "temporal-foo -h"},
169+
{args: "foo bar --help", want: "temporal-foo-bar --help"}, // most specific extension wins
170+
{args: "workflow diagram --help", want: "temporal-workflow-diagram --help"},
171+
{args: "workflow diagram -h", want: "temporal-workflow-diagram -h"},
172+
152173
// Note: Flag aliases are already implicitly tested via other command-specific tests.
153174
}
154175

internal/temporalcli/commands.help_test.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,21 @@ func TestHelp_Subcommand(t *testing.T) {
2929
assert.NoError(t, res.Err)
3030
}
3131

32+
func TestHelp_WithValueFlag(t *testing.T) {
33+
h := NewCommandHarness(t)
34+
35+
for _, args := range [][]string{
36+
{"workflow", "list", "--address", "123", "--help"},
37+
{"workflow", "list", "--help", "--address", "123"},
38+
{"workflow", "list", "--namespace", "foo", "--help"},
39+
} {
40+
res := h.Execute(args...)
41+
assert.Contains(t, res.Stdout.String(), "List Workflow Executions.")
42+
assert.NotContains(t, res.Stderr.String(), "pflag: help requested")
43+
assert.NoError(t, res.Err)
44+
}
45+
}
46+
3247
func TestHelp_HelpShowsAllFlag(t *testing.T) {
3348
h := NewCommandHarness(t)
3449
res := h.Execute("help", "--help")

0 commit comments

Comments
 (0)