From 99c79de8983d49a54cfb0d17242ac6cd587cd6e1 Mon Sep 17 00:00:00 2001 From: Dan Davison Date: Mon, 23 Feb 2026 22:32:37 -0500 Subject: [PATCH 01/10] Test script --- .task/repro.sh | 132 ++++++++++++++++++++++ internal/temporalcli/commands.env_test.go | 51 +++++++++ internal/temporalcli/commands_test.go | 25 ++++ 3 files changed, 208 insertions(+) create mode 100755 .task/repro.sh diff --git a/.task/repro.sh b/.task/repro.sh new file mode 100755 index 000000000..b6fc3f834 --- /dev/null +++ b/.task/repro.sh @@ -0,0 +1,132 @@ +#!/usr/bin/env bash +# Run: bash .task/repro.sh +# +# Builds the CLI from the currently checked-out commit and runs scenarios +# that demonstrate whether error reporting and user-facing warnings are +# coupled to the structured logger. Run on main to see the bug; run on +# the fix branch to see correct behavior. +set -euo pipefail + +REPO_ROOT="$(cd "$(dirname "$0")/.." && pwd)" +cd "$REPO_ROOT" + +BINARY=/tmp/temporal-repro +ENV_FILE=$(mktemp) +STDERR_FILE=$(mktemp) +trap 'rm -f "$ENV_FILE" "$STDERR_FILE" "$BINARY"' EXIT + +echo "Building temporal from $(git rev-parse --short HEAD) ($(git branch --show-current))..." >&2 +go build -o "$BINARY" ./cmd/temporal + +COMMIT="$(git rev-parse --short HEAD)" +BRANCH="$(git branch --show-current)" + +run() { + local stdout rc + stdout=$("$BINARY" "$@" 2>"$STDERR_FILE") && rc=$? || rc=$? + local stderr + stderr=$(cat "$STDERR_FILE") + echo '```bash' + echo "$ temporal $*" + echo '```' + echo "" + if [ -n "$stdout" ]; then + echo "**stdout:**" + echo '```' + echo "$stdout" + echo '```' + fi + echo "**stderr** (exit $rc):" + if [ -z "$stderr" ]; then + echo '```' + echo "(empty)" + echo '```' + else + echo '```' + echo "$stderr" + echo '```' + fi + echo "" +} + +cat </dev/null +\`\`\` + +EOF + +# Seed the env file +"$BINARY" env set --env-file "$ENV_FILE" --env myenv -k foo -v bar 2>/dev/null + +cat <<'EOF' +## Scenario 1: error reporting vs log level + +Attempting `workflow list` against a closed port should produce a clear +error message on stderr regardless of `--log-level`. Expected: an +`Error: ...` message appears at every log level. + +### Default log level +EOF +run workflow list --address 127.0.0.1:1 + +cat <<'EOF' +### `--log-level never` +EOF +run workflow list --address 127.0.0.1:1 --log-level never + +cat <<'EOF' +--- + +## Scenario 2: deprecation warning vs log level + +Using the deprecated positional-argument syntax for `env get` should +produce a plain-text warning on stderr regardless of `--log-level`. +Expected: a `Warning: ...` line appears at every log level, and it is +plain text (not a structured log message with `time=`/`level=` prefixes). + +### Default log level +EOF +run env get --env-file "$ENV_FILE" myenv + +cat <<'EOF' +### `--log-level never` +EOF +run env get --env-file "$ENV_FILE" --log-level never myenv + +cat <<'EOF' +--- + +## Scenario 3: default log level noise + +Running `env set` at the default log level should not dump structured +log lines to stderr. Expected: stderr is empty. + +### Default log level +EOF +rm -f "$ENV_FILE" && touch "$ENV_FILE" +run env set --env-file "$ENV_FILE" --env myenv -k foo -v bar + +cat <<'EOF' +### `--log-level info` (opt-in) +EOF +rm -f "$ENV_FILE" && touch "$ENV_FILE" +run env set --env-file "$ENV_FILE" --env myenv -k foo -v bar --log-level info + +cat <<'EOF' +--- + +## Cleanup + +```bash +rm -f /tmp/temporal-repro $ENV_FILE +``` +EOF diff --git a/internal/temporalcli/commands.env_test.go b/internal/temporalcli/commands.env_test.go index cdd1157a9..1a2a0a45a 100644 --- a/internal/temporalcli/commands.env_test.go +++ b/internal/temporalcli/commands.env_test.go @@ -2,6 +2,7 @@ package temporalcli_test import ( "os" + "strings" "testing" "gopkg.in/yaml.v3" @@ -104,3 +105,53 @@ func TestEnv_InputValidation(t *testing.T) { res = h.Execute("env", "set", "myenv1.foo") h.ErrorContains(res.Err, `no value provided`) } + +func TestEnv_DeprecationWarningBypassesLogger(t *testing.T) { + h := NewCommandHarness(t) + defer h.Close() + + tmpFile, err := os.CreateTemp("", "") + h.NoError(err) + h.Options.DeprecatedEnvConfig.EnvConfigFile = tmpFile.Name() + defer os.Remove(h.Options.DeprecatedEnvConfig.EnvConfigFile) + res := h.Execute("env", "set", "--env", "myenv1", "-k", "foo", "-v", "bar") + h.NoError(res.Err) + + // Using deprecated argument syntax should produce a warning on stderr. + // The warning must bypass the logger so it appears regardless of log level. + for _, logLevel := range []string{"never", "error", "info"} { + t.Run("log-level="+logLevel, func(t *testing.T) { + res = h.Execute("env", "get", "--log-level", logLevel, "myenv1") + h.NoError(res.Err) + + stderr := res.Stderr.String() + h.Contains(stderr, "Warning:") + h.Contains(stderr, "deprecated") + + // Must be a plain-text warning, not a structured log message + h.False(strings.Contains(stderr, "time="), "warning should not be a structured log message") + h.False(strings.Contains(stderr, "level="), "warning should not be a structured log message") + }) + } +} + +func TestEnv_DefaultLogLevelProducesNoLogOutput(t *testing.T) { + h := NewCommandHarness(t) + defer h.Close() + + tmpFile, err := os.CreateTemp("", "") + h.NoError(err) + h.Options.DeprecatedEnvConfig.EnvConfigFile = tmpFile.Name() + defer os.Remove(h.Options.DeprecatedEnvConfig.EnvConfigFile) + + // env set logs "Setting env property" via cctx.Logger.Info(). With the + // default log level ("never"), this should not appear on stderr. + res := h.Execute("env", "set", "--env", "myenv1", "-k", "foo", "-v", "bar") + h.NoError(res.Err) + h.Empty(res.Stderr.String(), "default log level should produce no log output") + + // With --log-level info, the logger output should appear. + res = h.Execute("env", "set", "--env", "myenv1", "-k", "baz", "-v", "qux", "--log-level", "info") + h.NoError(res.Err) + h.Contains(res.Stderr.String(), "Setting env property") +} diff --git a/internal/temporalcli/commands_test.go b/internal/temporalcli/commands_test.go index 2312e442e..749f2c3e6 100644 --- a/internal/temporalcli/commands_test.go +++ b/internal/temporalcli/commands_test.go @@ -6,6 +6,7 @@ import ( "fmt" "io" "log/slog" + "net" "regexp" "slices" "strings" @@ -625,6 +626,30 @@ func TestUnknownCommandExitsNonzero(t *testing.T) { assert.Contains(t, res.Err.Error(), "unknown command") } +func TestErrorReporting_IndependentOfLogLevel(t *testing.T) { + // Errors must be reported through Fail regardless of --log-level, and + // must not produce structured log output on stderr. + for _, logLevel := range []string{"never", "error", "info"} { + t.Run("log-level="+logLevel, func(t *testing.T) { + h := NewCommandHarness(t) + ln, err := net.Listen("tcp", "127.0.0.1:0") + require.NoError(t, err) + ln.Close() // close immediately so connection is refused + + res := h.Execute( + "workflow", "list", + "--address", ln.Addr().String(), + "--log-level", logLevel, + ) + require.Error(t, res.Err) + + stderr := res.Stderr.String() + assert.NotContains(t, stderr, "level=ERROR", + "errors should not appear as structured log messages on stderr") + }) + } +} + func (s *SharedServerSuite) TestHiddenAliasLogFormat() { _ = s.waitActivityStarted().GetID() res := s.Execute("workflow", "list", "--log-format", "pretty", "--address", s.Address()) From 785cadcce16d60c14cf7c7a042a7c66c5ac8babe Mon Sep 17 00:00:00 2001 From: Dan Davison Date: Mon, 23 Feb 2026 23:02:02 -0500 Subject: [PATCH 02/10] Decouple error reporting from logging The Fail callback was routing errors through the slog logger, which meant: - Errors appeared as structured log messages (with timestamp and level) - --log-level never silently swallowed errors - --log-format json wrapped errors in JSON Errors are now always printed directly to stderr as "Error: ", independent of log configuration. Default log level changed from "info" to "never"; server start-dev continues to default to "warn" via its existing ChangedFromDefault override. Users can still opt in with --log-level on any command. Two deprecation warnings (namespace arg, env command args) converted from logger calls to direct stderr writes so they remain visible regardless of log level. --- cliext/flags.gen.go | 4 ++-- cliext/option-sets.yaml | 4 ++-- internal/temporalcli/commands.env.go | 2 +- internal/temporalcli/commands.go | 6 +----- internal/temporalcli/commands.operator_namespace.go | 2 +- internal/temporalcli/commands.workflow_exec.go | 2 +- internal/temporalcli/commands.workflow_view.go | 2 +- 7 files changed, 9 insertions(+), 13 deletions(-) diff --git a/cliext/flags.gen.go b/cliext/flags.gen.go index 28668a3ed..c607e4c13 100644 --- a/cliext/flags.gen.go +++ b/cliext/flags.gen.go @@ -38,8 +38,8 @@ func (v *CommonOptions) BuildFlags(f *pflag.FlagSet) { f.StringVar(&v.Profile, "profile", "", "Profile to use for config file. EXPERIMENTAL.") f.BoolVar(&v.DisableConfigFile, "disable-config-file", false, "If set, disables loading environment config from config file. EXPERIMENTAL.") f.BoolVar(&v.DisableConfigEnv, "disable-config-env", false, "If set, disables loading environment config from environment variables. EXPERIMENTAL.") - v.LogLevel = NewFlagStringEnum([]string{"debug", "info", "warn", "error", "never"}, "info") - f.Var(&v.LogLevel, "log-level", "Log level. Default is \"info\" for most commands and \"warn\" for \"server start-dev\". Accepted values: debug, info, warn, error, never.") + v.LogLevel = NewFlagStringEnum([]string{"debug", "info", "warn", "error", "never"}, "never") + f.Var(&v.LogLevel, "log-level", "Log level. Default is \"never\" for most commands and \"warn\" for \"server start-dev\". Accepted values: debug, info, warn, error, never.") v.LogFormat = NewFlagStringEnum([]string{"text", "json", "pretty"}, "text") f.Var(&v.LogFormat, "log-format", "Log format. Accepted values: text, json.") v.Output = NewFlagStringEnum([]string{"text", "json", "jsonl", "none"}, "text") diff --git a/cliext/option-sets.yaml b/cliext/option-sets.yaml index c8472fd03..e7a7ff8cf 100644 --- a/cliext/option-sets.yaml +++ b/cliext/option-sets.yaml @@ -51,8 +51,8 @@ option-sets: - never description: | Log level. - Default is "info" for most commands and "warn" for "server start-dev". - default: info + Default is "never" for most commands and "warn" for "server start-dev". + default: never - name: log-format type: string-enum description: Log format. diff --git a/internal/temporalcli/commands.env.go b/internal/temporalcli/commands.env.go index d98c06f04..5d43843d7 100644 --- a/internal/temporalcli/commands.env.go +++ b/internal/temporalcli/commands.env.go @@ -13,7 +13,7 @@ import ( func (c *TemporalEnvCommand) envNameAndKey(cctx *CommandContext, args []string, keyFlag string) (string, string, error) { if len(args) > 0 { - cctx.Logger.Warn("Arguments to env commands are deprecated; please use --env and --key (or -k) instead") + fmt.Fprintln(cctx.Options.Stderr, "Warning: Arguments to env commands are deprecated; please use --env and --key (or -k) instead") if c.Parent.Env != "default" || keyFlag != "" { return "", "", fmt.Errorf("cannot specify both an argument and flags; please use flags instead") diff --git a/internal/temporalcli/commands.go b/internal/temporalcli/commands.go index a60307b83..1983ebdfa 100644 --- a/internal/temporalcli/commands.go +++ b/internal/temporalcli/commands.go @@ -143,11 +143,7 @@ func (c *CommandContext) preprocessOptions() error { if c.Err() != nil { err = fmt.Errorf("program interrupted") } - if c.Logger != nil { - c.Logger.Error(err.Error()) - } else { - fmt.Fprintln(os.Stderr, err) - } + fmt.Fprintf(c.Options.Stderr, "Error: %v\n", err) os.Exit(1) } } diff --git a/internal/temporalcli/commands.operator_namespace.go b/internal/temporalcli/commands.operator_namespace.go index ebcc96cab..ca4b6c6d5 100644 --- a/internal/temporalcli/commands.operator_namespace.go +++ b/internal/temporalcli/commands.operator_namespace.go @@ -19,7 +19,7 @@ func (c *TemporalOperatorCommand) getNSFromFlagOrArg0(cctx *CommandContext, args } if len(args) > 0 { - cctx.Logger.Warn("Passing the namespace as an argument is now deprecated; please switch to using -n instead") + fmt.Fprintln(cctx.Options.Stderr, "Warning: Passing the namespace as an argument is now deprecated; please switch to using -n instead") return args[0], nil } return c.Namespace, nil diff --git a/internal/temporalcli/commands.workflow_exec.go b/internal/temporalcli/commands.workflow_exec.go index d5ba34899..935504b96 100644 --- a/internal/temporalcli/commands.workflow_exec.go +++ b/internal/temporalcli/commands.workflow_exec.go @@ -94,7 +94,7 @@ func (c *TemporalWorkflowExecuteCommand) run(cctx *CommandContext, args []string // Log print failure and return workflow failure if workflow failed if closeEvent.EventType != enums.EVENT_TYPE_WORKFLOW_EXECUTION_COMPLETED { if err != nil { - cctx.Logger.Error("Workflow failed, and printing the output also failed", "error", err) + fmt.Fprintf(cctx.Options.Stderr, "Warning: printing workflow output failed: %v\n", err) } err = fmt.Errorf("workflow failed") } diff --git a/internal/temporalcli/commands.workflow_view.go b/internal/temporalcli/commands.workflow_view.go index 38daf936a..278fcad35 100644 --- a/internal/temporalcli/commands.workflow_view.go +++ b/internal/temporalcli/commands.workflow_view.go @@ -564,7 +564,7 @@ func (c *TemporalWorkflowResultCommand) run(cctx *CommandContext, _ []string) er // Log print failure and return workflow failure if workflow failed if closeEvent.EventType != enums.EVENT_TYPE_WORKFLOW_EXECUTION_COMPLETED { if err != nil { - cctx.Logger.Error("Workflow failed, and printing the output also failed", "error", err) + fmt.Fprintf(cctx.Options.Stderr, "Warning: printing workflow output failed: %v\n", err) } err = fmt.Errorf("workflow failed") } From e1e34fadba5dd7921e41973736f9c13d68eea4a9 Mon Sep 17 00:00:00 2001 From: Dan Davison Date: Sun, 1 Mar 2026 13:33:13 -0500 Subject: [PATCH 03/10] rm repro script --- .task/repro.sh | 132 ------------------------------------------------- 1 file changed, 132 deletions(-) delete mode 100755 .task/repro.sh diff --git a/.task/repro.sh b/.task/repro.sh deleted file mode 100755 index b6fc3f834..000000000 --- a/.task/repro.sh +++ /dev/null @@ -1,132 +0,0 @@ -#!/usr/bin/env bash -# Run: bash .task/repro.sh -# -# Builds the CLI from the currently checked-out commit and runs scenarios -# that demonstrate whether error reporting and user-facing warnings are -# coupled to the structured logger. Run on main to see the bug; run on -# the fix branch to see correct behavior. -set -euo pipefail - -REPO_ROOT="$(cd "$(dirname "$0")/.." && pwd)" -cd "$REPO_ROOT" - -BINARY=/tmp/temporal-repro -ENV_FILE=$(mktemp) -STDERR_FILE=$(mktemp) -trap 'rm -f "$ENV_FILE" "$STDERR_FILE" "$BINARY"' EXIT - -echo "Building temporal from $(git rev-parse --short HEAD) ($(git branch --show-current))..." >&2 -go build -o "$BINARY" ./cmd/temporal - -COMMIT="$(git rev-parse --short HEAD)" -BRANCH="$(git branch --show-current)" - -run() { - local stdout rc - stdout=$("$BINARY" "$@" 2>"$STDERR_FILE") && rc=$? || rc=$? - local stderr - stderr=$(cat "$STDERR_FILE") - echo '```bash' - echo "$ temporal $*" - echo '```' - echo "" - if [ -n "$stdout" ]; then - echo "**stdout:**" - echo '```' - echo "$stdout" - echo '```' - fi - echo "**stderr** (exit $rc):" - if [ -z "$stderr" ]; then - echo '```' - echo "(empty)" - echo '```' - else - echo '```' - echo "$stderr" - echo '```' - fi - echo "" -} - -cat </dev/null -\`\`\` - -EOF - -# Seed the env file -"$BINARY" env set --env-file "$ENV_FILE" --env myenv -k foo -v bar 2>/dev/null - -cat <<'EOF' -## Scenario 1: error reporting vs log level - -Attempting `workflow list` against a closed port should produce a clear -error message on stderr regardless of `--log-level`. Expected: an -`Error: ...` message appears at every log level. - -### Default log level -EOF -run workflow list --address 127.0.0.1:1 - -cat <<'EOF' -### `--log-level never` -EOF -run workflow list --address 127.0.0.1:1 --log-level never - -cat <<'EOF' ---- - -## Scenario 2: deprecation warning vs log level - -Using the deprecated positional-argument syntax for `env get` should -produce a plain-text warning on stderr regardless of `--log-level`. -Expected: a `Warning: ...` line appears at every log level, and it is -plain text (not a structured log message with `time=`/`level=` prefixes). - -### Default log level -EOF -run env get --env-file "$ENV_FILE" myenv - -cat <<'EOF' -### `--log-level never` -EOF -run env get --env-file "$ENV_FILE" --log-level never myenv - -cat <<'EOF' ---- - -## Scenario 3: default log level noise - -Running `env set` at the default log level should not dump structured -log lines to stderr. Expected: stderr is empty. - -### Default log level -EOF -rm -f "$ENV_FILE" && touch "$ENV_FILE" -run env set --env-file "$ENV_FILE" --env myenv -k foo -v bar - -cat <<'EOF' -### `--log-level info` (opt-in) -EOF -rm -f "$ENV_FILE" && touch "$ENV_FILE" -run env set --env-file "$ENV_FILE" --env myenv -k foo -v bar --log-level info - -cat <<'EOF' ---- - -## Cleanup - -```bash -rm -f /tmp/temporal-repro $ENV_FILE -``` -EOF From 07880a94f36a921f06c480abba9fe67b5a9990d9 Mon Sep 17 00:00:00 2001 From: Dan Davison Date: Thu, 19 Mar 2026 22:44:45 -0400 Subject: [PATCH 04/10] Add --verbose flag; replace remaining Logger.Info with plain text Convert CLI diagnostic messages (env/config file operations, env-var flag overrides) from structured slog output to plain-text stderr gated on --verbose. The structured logger now exists solely for the SDK client and dev server, which conventionally use it. --- cliext/flags.gen.go | 2 ++ cliext/option-sets.yaml | 3 ++ internal/temporalcli/commands.config.go | 2 +- internal/temporalcli/commands.env.go | 8 +++--- internal/temporalcli/commands.env_test.go | 13 ++++----- internal/temporalcli/commands.gen.go | 2 +- internal/temporalcli/commands.go | 34 ++++++++++++++--------- internal/temporalcli/commands.server.go | 1 - internal/temporalcli/commands.yaml | 2 +- 9 files changed, 39 insertions(+), 28 deletions(-) diff --git a/cliext/flags.gen.go b/cliext/flags.gen.go index c607e4c13..fbca6583c 100644 --- a/cliext/flags.gen.go +++ b/cliext/flags.gen.go @@ -19,6 +19,7 @@ type CommonOptions struct { Profile string DisableConfigFile bool DisableConfigEnv bool + Verbose bool LogLevel FlagStringEnum LogFormat FlagStringEnum Output FlagStringEnum @@ -38,6 +39,7 @@ func (v *CommonOptions) BuildFlags(f *pflag.FlagSet) { f.StringVar(&v.Profile, "profile", "", "Profile to use for config file. EXPERIMENTAL.") f.BoolVar(&v.DisableConfigFile, "disable-config-file", false, "If set, disables loading environment config from config file. EXPERIMENTAL.") f.BoolVar(&v.DisableConfigEnv, "disable-config-env", false, "If set, disables loading environment config from environment variables. EXPERIMENTAL.") + f.BoolVar(&v.Verbose, "verbose", false, "Enable verbose output.") v.LogLevel = NewFlagStringEnum([]string{"debug", "info", "warn", "error", "never"}, "never") f.Var(&v.LogLevel, "log-level", "Log level. Default is \"never\" for most commands and \"warn\" for \"server start-dev\". Accepted values: debug, info, warn, error, never.") v.LogFormat = NewFlagStringEnum([]string{"text", "json", "pretty"}, "text") diff --git a/cliext/option-sets.yaml b/cliext/option-sets.yaml index e7a7ff8cf..f70424461 100644 --- a/cliext/option-sets.yaml +++ b/cliext/option-sets.yaml @@ -41,6 +41,9 @@ option-sets: If set, disables loading environment config from environment variables. experimental: true + - name: verbose + type: bool + description: Enable verbose output. - name: log-level type: string-enum enum-values: diff --git a/internal/temporalcli/commands.config.go b/internal/temporalcli/commands.config.go index 79467328d..24e12aec3 100644 --- a/internal/temporalcli/commands.config.go +++ b/internal/temporalcli/commands.config.go @@ -325,7 +325,7 @@ func writeEnvConfigFile(cctx *CommandContext, conf *envconfig.ClientConfig) erro } // Write to file, making dirs as needed - cctx.Logger.Info("Writing config file", "file", configFile) + cctx.printVerbose(fmt.Sprintf("Writing config file %s", configFile)) if err := os.MkdirAll(filepath.Dir(configFile), 0700); err != nil { return fmt.Errorf("failed making config file parent dirs: %w", err) } else if err := os.WriteFile(configFile, b, 0600); err != nil { diff --git a/internal/temporalcli/commands.env.go b/internal/temporalcli/commands.env.go index 5d43843d7..2c1d57a42 100644 --- a/internal/temporalcli/commands.env.go +++ b/internal/temporalcli/commands.env.go @@ -49,10 +49,10 @@ func (c *TemporalEnvDeleteCommand) run(cctx *CommandContext, args []string) erro env, _ := cctx.DeprecatedEnvConfigValues[envName] // User can remove single flag or all in env if key != "" { - cctx.Logger.Info("Deleting env property", "env", envName, "property", key) + cctx.printVerbose(fmt.Sprintf("Deleting env property %s/%s", envName, key)) delete(env, key) } else { - cctx.Logger.Info("Deleting env", "env", env) + cctx.printVerbose(fmt.Sprintf("Deleting env %s", envName)) delete(cctx.DeprecatedEnvConfigValues, envName) } return writeDeprecatedEnvConfigToFile(cctx) @@ -129,7 +129,7 @@ func (c *TemporalEnvSetCommand) run(cctx *CommandContext, args []string) error { if cctx.DeprecatedEnvConfigValues[envName] == nil { cctx.DeprecatedEnvConfigValues[envName] = map[string]string{} } - cctx.Logger.Info("Setting env property", "env", envName, "property", key, "value", value) + cctx.printVerbose(fmt.Sprintf("Setting env property %s/%s=%s", envName, key, value)) cctx.DeprecatedEnvConfigValues[envName][key] = value return writeDeprecatedEnvConfigToFile(cctx) } @@ -138,7 +138,7 @@ func writeDeprecatedEnvConfigToFile(cctx *CommandContext) error { if cctx.Options.DeprecatedEnvConfig.EnvConfigFile == "" { return fmt.Errorf("unable to find place for env file (unknown HOME dir)") } - cctx.Logger.Info("Writing env file", "file", cctx.Options.DeprecatedEnvConfig.EnvConfigFile) + cctx.printVerbose(fmt.Sprintf("Writing env file %s", cctx.Options.DeprecatedEnvConfig.EnvConfigFile)) return writeDeprecatedEnvConfigFile(cctx.Options.DeprecatedEnvConfig.EnvConfigFile, cctx.DeprecatedEnvConfigValues) } diff --git a/internal/temporalcli/commands.env_test.go b/internal/temporalcli/commands.env_test.go index 1a2a0a45a..59be2a3aa 100644 --- a/internal/temporalcli/commands.env_test.go +++ b/internal/temporalcli/commands.env_test.go @@ -135,7 +135,7 @@ func TestEnv_DeprecationWarningBypassesLogger(t *testing.T) { } } -func TestEnv_DefaultLogLevelProducesNoLogOutput(t *testing.T) { +func TestEnv_VerboseFlag(t *testing.T) { h := NewCommandHarness(t) defer h.Close() @@ -144,14 +144,13 @@ func TestEnv_DefaultLogLevelProducesNoLogOutput(t *testing.T) { h.Options.DeprecatedEnvConfig.EnvConfigFile = tmpFile.Name() defer os.Remove(h.Options.DeprecatedEnvConfig.EnvConfigFile) - // env set logs "Setting env property" via cctx.Logger.Info(). With the - // default log level ("never"), this should not appear on stderr. + // Without --verbose, no output on stderr. res := h.Execute("env", "set", "--env", "myenv1", "-k", "foo", "-v", "bar") h.NoError(res.Err) - h.Empty(res.Stderr.String(), "default log level should produce no log output") + h.Empty(res.Stderr.String()) - // With --log-level info, the logger output should appear. - res = h.Execute("env", "set", "--env", "myenv1", "-k", "baz", "-v", "qux", "--log-level", "info") + // With --verbose, diagnostic messages appear as plain text. + res = h.Execute("env", "set", "--env", "myenv1", "-k", "baz", "-v", "qux", "--verbose") h.NoError(res.Err) - h.Contains(res.Stderr.String(), "Setting env property") + h.Contains(res.Stderr.String(), "Setting env property myenv1/baz=qux") } diff --git a/internal/temporalcli/commands.gen.go b/internal/temporalcli/commands.gen.go index 1b1f31295..2455cf233 100644 --- a/internal/temporalcli/commands.gen.go +++ b/internal/temporalcli/commands.gen.go @@ -2117,7 +2117,7 @@ func NewTemporalServerStartDevCommand(cctx *CommandContext, parent *TemporalServ s.Command.Flags().StringVar(&s.UiCodecEndpoint, "ui-codec-endpoint", "", "UI remote codec HTTP endpoint.") s.Command.Flags().StringArrayVar(&s.SqlitePragma, "sqlite-pragma", nil, "SQLite pragma statements in \"PRAGMA=VALUE\" format.") s.Command.Flags().StringArrayVar(&s.DynamicConfigValue, "dynamic-config-value", nil, "Dynamic configuration value using `KEY=VALUE` pairs. Keys must be identifiers, and values must be JSON values. For example: `YourKey=\"YourString\"` Can be passed multiple times.") - s.Command.Flags().BoolVar(&s.LogConfig, "log-config", false, "Log the server config to stderr.") + s.Command.Flags().BoolVar(&s.LogConfig, "log-config", false, "Print the server config to stderr.") s.Command.Flags().StringArrayVar(&s.SearchAttribute, "search-attribute", nil, "Search attributes to register using `KEY=VALUE` pairs. Keys must be identifiers, and values must be the search attribute type, which is one of the following: Text, Keyword, Int, Double, Bool, Datetime, KeywordList.") s.Command.Run = func(c *cobra.Command, args []string) { if err := s.run(cctx, args); err != nil { diff --git a/internal/temporalcli/commands.go b/internal/temporalcli/commands.go index 1983ebdfa..ecf8d58c0 100644 --- a/internal/temporalcli/commands.go +++ b/internal/temporalcli/commands.go @@ -50,6 +50,7 @@ type CommandContext struct { // These values may not be available until after pre-run of main command Printer *printer.Printer Logger *slog.Logger + Verbose bool JSONOutput bool JSONShorthandPayloads bool @@ -256,13 +257,13 @@ func UnmarshalProtoJSONWithOptions(b []byte, m proto.Message, jsonShorthandPaylo return opts.Unmarshal(b, m) } -// Set flag values from environment file & variables. Returns a callback to log anything interesting -// since logging will not yet be initialized when this runs. -func (c *CommandContext) populateFlagsFromEnv(flags *pflag.FlagSet) (func(*slog.Logger), error) { +// Set flag values from environment file & variables. Returns a callback to +// print verbose messages (deferred because --verbose is not yet parsed). +func (c *CommandContext) populateFlagsFromEnv(flags *pflag.FlagSet) (func(), error) { if flags == nil { - return func(logger *slog.Logger) {}, nil + return func() {}, nil } - var logCalls []func(*slog.Logger) + var verboseMessages []string var flagErr error flags.VisitAll(func(flag *pflag.Flag) { // If the flag was already changed by the user, we don't overwrite @@ -285,20 +286,25 @@ func (c *CommandContext) populateFlagsFromEnv(flags *pflag.FlagSet) (func(*slog. return } if flag.Changed { - logCalls = append(logCalls, func(l *slog.Logger) { - l.Info("Env var overrode --env setting", "env_var", anns[0], "flag", flag.Name) - }) + verboseMessages = append(verboseMessages, + fmt.Sprintf("Env var %s overrode --env setting for flag --%s", anns[0], flag.Name)) } flag.Changed = true } } }) - logFn := func(logger *slog.Logger) { - for _, call := range logCalls { - call(logger) + printFn := func() { + for _, msg := range verboseMessages { + c.printVerbose(msg) } } - return logFn, flagErr + return printFn, flagErr +} + +func (c *CommandContext) printVerbose(msg string) { + if c.Verbose { + fmt.Fprintln(c.Options.Stderr, msg) + } } // Returns error if JSON output enabled @@ -468,7 +474,7 @@ func (c *TemporalCommand) initCommand(cctx *CommandContext) { res := c.preRun(cctx) - logCalls(cctx.Logger) + logCalls() // Always disable color if JSON output is on (must be run after preRun so JSONOutput is set) if cctx.JSONOutput { @@ -518,6 +524,8 @@ func (c *TemporalCommand) preRun(cctx *CommandContext) error { } } + cctx.Verbose = c.Verbose + // Configure printer if not already on context cctx.JSONOutput = c.Output.Value == "json" || c.Output.Value == "jsonl" // Only indent JSON if not jsonl diff --git a/internal/temporalcli/commands.server.go b/internal/temporalcli/commands.server.go index c484344b5..ac216423f 100644 --- a/internal/temporalcli/commands.server.go +++ b/internal/temporalcli/commands.server.go @@ -134,7 +134,6 @@ func (t *TemporalServerStartDevCommand) run(cctx *CommandContext, args []string) // Log config if requested if t.LogConfig { opts.LogConfig = func(b []byte) { - cctx.Logger.Info("Logging config") _, _ = cctx.Options.Stderr.Write(b) } } diff --git a/internal/temporalcli/commands.yaml b/internal/temporalcli/commands.yaml index d293b75f8..607b6b499 100644 --- a/internal/temporalcli/commands.yaml +++ b/internal/temporalcli/commands.yaml @@ -2428,7 +2428,7 @@ commands: Can be passed multiple times. - name: log-config type: bool - description: Log the server config to stderr. + description: Print the server config to stderr. - name: search-attribute type: string[] description: | From f17c358ba377a2522be6bbbf6c70caab16a8ad38 Mon Sep 17 00:00:00 2001 From: Dan Davison Date: Thu, 19 Mar 2026 22:53:47 -0400 Subject: [PATCH 05/10] Remove verbose messages that parrot the command back to the user --verbose should reveal non-obvious behavior (e.g. env var overriding a flag), not echo what the user just asked to happen. --- internal/temporalcli/commands.config.go | 1 - internal/temporalcli/commands.env.go | 4 ---- 2 files changed, 5 deletions(-) diff --git a/internal/temporalcli/commands.config.go b/internal/temporalcli/commands.config.go index 24e12aec3..e8a423dd0 100644 --- a/internal/temporalcli/commands.config.go +++ b/internal/temporalcli/commands.config.go @@ -325,7 +325,6 @@ func writeEnvConfigFile(cctx *CommandContext, conf *envconfig.ClientConfig) erro } // Write to file, making dirs as needed - cctx.printVerbose(fmt.Sprintf("Writing config file %s", configFile)) if err := os.MkdirAll(filepath.Dir(configFile), 0700); err != nil { return fmt.Errorf("failed making config file parent dirs: %w", err) } else if err := os.WriteFile(configFile, b, 0600); err != nil { diff --git a/internal/temporalcli/commands.env.go b/internal/temporalcli/commands.env.go index 2c1d57a42..7ec952327 100644 --- a/internal/temporalcli/commands.env.go +++ b/internal/temporalcli/commands.env.go @@ -49,10 +49,8 @@ func (c *TemporalEnvDeleteCommand) run(cctx *CommandContext, args []string) erro env, _ := cctx.DeprecatedEnvConfigValues[envName] // User can remove single flag or all in env if key != "" { - cctx.printVerbose(fmt.Sprintf("Deleting env property %s/%s", envName, key)) delete(env, key) } else { - cctx.printVerbose(fmt.Sprintf("Deleting env %s", envName)) delete(cctx.DeprecatedEnvConfigValues, envName) } return writeDeprecatedEnvConfigToFile(cctx) @@ -129,7 +127,6 @@ func (c *TemporalEnvSetCommand) run(cctx *CommandContext, args []string) error { if cctx.DeprecatedEnvConfigValues[envName] == nil { cctx.DeprecatedEnvConfigValues[envName] = map[string]string{} } - cctx.printVerbose(fmt.Sprintf("Setting env property %s/%s=%s", envName, key, value)) cctx.DeprecatedEnvConfigValues[envName][key] = value return writeDeprecatedEnvConfigToFile(cctx) } @@ -138,7 +135,6 @@ func writeDeprecatedEnvConfigToFile(cctx *CommandContext) error { if cctx.Options.DeprecatedEnvConfig.EnvConfigFile == "" { return fmt.Errorf("unable to find place for env file (unknown HOME dir)") } - cctx.printVerbose(fmt.Sprintf("Writing env file %s", cctx.Options.DeprecatedEnvConfig.EnvConfigFile)) return writeDeprecatedEnvConfigFile(cctx.Options.DeprecatedEnvConfig.EnvConfigFile, cctx.DeprecatedEnvConfigValues) } From a648b782f918dfcb3815bfadc0433fb4f31127aa Mon Sep 17 00:00:00 2001 From: Dan Davison Date: Thu, 19 Mar 2026 23:10:44 -0400 Subject: [PATCH 06/10] Simplify populateFlagsFromEnv: print verbose messages directly Set cctx.Verbose before populateFlagsFromEnv so it can call printVerbose immediately, eliminating the deferred callback. --- internal/temporalcli/commands.go | 25 +++++++------------------ 1 file changed, 7 insertions(+), 18 deletions(-) diff --git a/internal/temporalcli/commands.go b/internal/temporalcli/commands.go index ecf8d58c0..661a9f674 100644 --- a/internal/temporalcli/commands.go +++ b/internal/temporalcli/commands.go @@ -257,13 +257,11 @@ func UnmarshalProtoJSONWithOptions(b []byte, m proto.Message, jsonShorthandPaylo return opts.Unmarshal(b, m) } -// Set flag values from environment file & variables. Returns a callback to -// print verbose messages (deferred because --verbose is not yet parsed). -func (c *CommandContext) populateFlagsFromEnv(flags *pflag.FlagSet) (func(), error) { +// Set flag values from environment file & variables. +func (c *CommandContext) populateFlagsFromEnv(flags *pflag.FlagSet) error { if flags == nil { - return func() {}, nil + return nil } - var verboseMessages []string var flagErr error flags.VisitAll(func(flag *pflag.Flag) { // If the flag was already changed by the user, we don't overwrite @@ -286,19 +284,14 @@ func (c *CommandContext) populateFlagsFromEnv(flags *pflag.FlagSet) (func(), err return } if flag.Changed { - verboseMessages = append(verboseMessages, + c.printVerbose( fmt.Sprintf("Env var %s overrode --env setting for flag --%s", anns[0], flag.Name)) } flag.Changed = true } } }) - printFn := func() { - for _, msg := range verboseMessages { - c.printVerbose(msg) - } - } - return printFn, flagErr + return flagErr } func (c *CommandContext) printVerbose(msg string) { @@ -459,10 +452,10 @@ func (c *TemporalCommand) initCommand(cctx *CommandContext) { c.Command.PersistentPreRunE = func(cmd *cobra.Command, args []string) error { // Set command cctx.CurrentCommand = cmd + cctx.Verbose = c.Verbose // Populate environ. We will make the error return here which will cause // usage to be printed. - logCalls, err := cctx.populateFlagsFromEnv(cmd.Flags()) - if err != nil { + if err := cctx.populateFlagsFromEnv(cmd.Flags()); err != nil { return err } @@ -474,8 +467,6 @@ func (c *TemporalCommand) initCommand(cctx *CommandContext) { res := c.preRun(cctx) - logCalls() - // Always disable color if JSON output is on (must be run after preRun so JSONOutput is set) if cctx.JSONOutput { color.NoColor = true @@ -524,8 +515,6 @@ func (c *TemporalCommand) preRun(cctx *CommandContext) error { } } - cctx.Verbose = c.Verbose - // Configure printer if not already on context cctx.JSONOutput = c.Output.Value == "json" || c.Output.Value == "jsonl" // Only indent JSON if not jsonl From 8e43c74457b0196ae0805482f4c8f03c8fc449df Mon Sep 17 00:00:00 2001 From: Dan Davison Date: Thu, 19 Mar 2026 23:22:03 -0400 Subject: [PATCH 07/10] Delete test --- internal/temporalcli/commands.env_test.go | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/internal/temporalcli/commands.env_test.go b/internal/temporalcli/commands.env_test.go index 59be2a3aa..f92506114 100644 --- a/internal/temporalcli/commands.env_test.go +++ b/internal/temporalcli/commands.env_test.go @@ -134,23 +134,3 @@ func TestEnv_DeprecationWarningBypassesLogger(t *testing.T) { }) } } - -func TestEnv_VerboseFlag(t *testing.T) { - h := NewCommandHarness(t) - defer h.Close() - - tmpFile, err := os.CreateTemp("", "") - h.NoError(err) - h.Options.DeprecatedEnvConfig.EnvConfigFile = tmpFile.Name() - defer os.Remove(h.Options.DeprecatedEnvConfig.EnvConfigFile) - - // Without --verbose, no output on stderr. - res := h.Execute("env", "set", "--env", "myenv1", "-k", "foo", "-v", "bar") - h.NoError(res.Err) - h.Empty(res.Stderr.String()) - - // With --verbose, diagnostic messages appear as plain text. - res = h.Execute("env", "set", "--env", "myenv1", "-k", "baz", "-v", "qux", "--verbose") - h.NoError(res.Err) - h.Contains(res.Stderr.String(), "Setting env property myenv1/baz=qux") -} From f08ca489f7c381644059b78b775695a8a3860c59 Mon Sep 17 00:00:00 2001 From: Dan Davison Date: Thu, 19 Mar 2026 23:36:17 -0400 Subject: [PATCH 08/10] Remove --verbose flag; print env-var override warning unconditionally The only printVerbose call site was unreachable (the sole flag with an env var annotation is --env, which is consumed before populateFlagsFromEnv runs). Replace with an unconditional warning to stderr so it works if BindFlagEnvVar is used for other flags in future. --- cliext/flags.gen.go | 2 -- cliext/option-sets.yaml | 3 --- internal/temporalcli/commands.go | 12 ++---------- 3 files changed, 2 insertions(+), 15 deletions(-) diff --git a/cliext/flags.gen.go b/cliext/flags.gen.go index fbca6583c..c607e4c13 100644 --- a/cliext/flags.gen.go +++ b/cliext/flags.gen.go @@ -19,7 +19,6 @@ type CommonOptions struct { Profile string DisableConfigFile bool DisableConfigEnv bool - Verbose bool LogLevel FlagStringEnum LogFormat FlagStringEnum Output FlagStringEnum @@ -39,7 +38,6 @@ func (v *CommonOptions) BuildFlags(f *pflag.FlagSet) { f.StringVar(&v.Profile, "profile", "", "Profile to use for config file. EXPERIMENTAL.") f.BoolVar(&v.DisableConfigFile, "disable-config-file", false, "If set, disables loading environment config from config file. EXPERIMENTAL.") f.BoolVar(&v.DisableConfigEnv, "disable-config-env", false, "If set, disables loading environment config from environment variables. EXPERIMENTAL.") - f.BoolVar(&v.Verbose, "verbose", false, "Enable verbose output.") v.LogLevel = NewFlagStringEnum([]string{"debug", "info", "warn", "error", "never"}, "never") f.Var(&v.LogLevel, "log-level", "Log level. Default is \"never\" for most commands and \"warn\" for \"server start-dev\". Accepted values: debug, info, warn, error, never.") v.LogFormat = NewFlagStringEnum([]string{"text", "json", "pretty"}, "text") diff --git a/cliext/option-sets.yaml b/cliext/option-sets.yaml index f70424461..e7a7ff8cf 100644 --- a/cliext/option-sets.yaml +++ b/cliext/option-sets.yaml @@ -41,9 +41,6 @@ option-sets: If set, disables loading environment config from environment variables. experimental: true - - name: verbose - type: bool - description: Enable verbose output. - name: log-level type: string-enum enum-values: diff --git a/internal/temporalcli/commands.go b/internal/temporalcli/commands.go index 661a9f674..1ecbd1597 100644 --- a/internal/temporalcli/commands.go +++ b/internal/temporalcli/commands.go @@ -50,7 +50,6 @@ type CommandContext struct { // These values may not be available until after pre-run of main command Printer *printer.Printer Logger *slog.Logger - Verbose bool JSONOutput bool JSONShorthandPayloads bool @@ -284,8 +283,8 @@ func (c *CommandContext) populateFlagsFromEnv(flags *pflag.FlagSet) error { return } if flag.Changed { - c.printVerbose( - fmt.Sprintf("Env var %s overrode --env setting for flag --%s", anns[0], flag.Name)) + fmt.Fprintf(c.Options.Stderr, + "Warning: env var %s overrode --env setting for flag --%s\n", anns[0], flag.Name) } flag.Changed = true } @@ -294,12 +293,6 @@ func (c *CommandContext) populateFlagsFromEnv(flags *pflag.FlagSet) error { return flagErr } -func (c *CommandContext) printVerbose(msg string) { - if c.Verbose { - fmt.Fprintln(c.Options.Stderr, msg) - } -} - // Returns error if JSON output enabled func (c *CommandContext) promptYes(message string, autoConfirm bool) (bool, error) { if c.JSONOutput && !autoConfirm { @@ -452,7 +445,6 @@ func (c *TemporalCommand) initCommand(cctx *CommandContext) { c.Command.PersistentPreRunE = func(cmd *cobra.Command, args []string) error { // Set command cctx.CurrentCommand = cmd - cctx.Verbose = c.Verbose // Populate environ. We will make the error return here which will cause // usage to be printed. if err := cctx.populateFlagsFromEnv(cmd.Flags()); err != nil { From 90f17ef431d531605ca94d85b0ca2c6b03f24eb4 Mon Sep 17 00:00:00 2001 From: Dan Davison Date: Thu, 19 Mar 2026 23:41:11 -0400 Subject: [PATCH 09/10] Add comment documenting no-logging discipline --- internal/temporalcli/commands.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/internal/temporalcli/commands.go b/internal/temporalcli/commands.go index 1ecbd1597..fb76546e2 100644 --- a/internal/temporalcli/commands.go +++ b/internal/temporalcli/commands.go @@ -48,7 +48,9 @@ type CommandContext struct { FlagsWithEnvVars []*pflag.Flag // These values may not be available until after pre-run of main command - Printer *printer.Printer + Printer *printer.Printer + // The logger is for the SDK and server. The CLI itself should print warnings or errors to + // stderr but should not use logging. Logger *slog.Logger JSONOutput bool JSONShorthandPayloads bool From 960d5d382f34acc4799cdc17fdbc1f22ff27bc3f Mon Sep 17 00:00:00 2001 From: Dan Davison Date: Thu, 19 Mar 2026 23:55:16 -0400 Subject: [PATCH 10/10] Simplify test: use invalid host instead of socket dance --- internal/temporalcli/commands_test.go | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/internal/temporalcli/commands_test.go b/internal/temporalcli/commands_test.go index 749f2c3e6..d37c890b9 100644 --- a/internal/temporalcli/commands_test.go +++ b/internal/temporalcli/commands_test.go @@ -6,7 +6,6 @@ import ( "fmt" "io" "log/slog" - "net" "regexp" "slices" "strings" @@ -626,25 +625,21 @@ func TestUnknownCommandExitsNonzero(t *testing.T) { assert.Contains(t, res.Err.Error(), "unknown command") } -func TestErrorReporting_IndependentOfLogLevel(t *testing.T) { +func TestErrorsAreNotLogMessages(t *testing.T) { // Errors must be reported through Fail regardless of --log-level, and // must not produce structured log output on stderr. for _, logLevel := range []string{"never", "error", "info"} { t.Run("log-level="+logLevel, func(t *testing.T) { h := NewCommandHarness(t) - ln, err := net.Listen("tcp", "127.0.0.1:0") - require.NoError(t, err) - ln.Close() // close immediately so connection is refused - res := h.Execute( "workflow", "list", - "--address", ln.Addr().String(), + "--address", "not-a-valid-host:1", "--log-level", logLevel, ) require.Error(t, res.Err) stderr := res.Stderr.String() - assert.NotContains(t, stderr, "level=ERROR", + assert.NotContains(t, stderr, "level=", "errors should not appear as structured log messages on stderr") }) }