fixes: Some architectural deviations

.changes/unreleased/BUG FIXES-20260729-100121.yaml

@@ -0,0 +1,3 @@
+kind: BUG FIXES
+body: Using `auth login --dry-run` no longer opens a web browser. --dry-run now appears in argument autocomplete lists.
+time: 2026-07-29T10:01:21.23869-06:00

.changes/unreleased/BUG FIXES-20260729-100210.yaml

@@ -0,0 +1,3 @@
+kind: BUG FIXES
+body: Using `profile display --markdown` no longer produces an error.
+time: 2026-07-29T10:02:10.946104-06:00

.changes/unreleased/BUG FIXES-20260729-101327.yaml

@@ -0,0 +1,3 @@
+kind: BUG FIXES
+body: Removed Token property from json output when using `profile profiles list --json`, preventing accidental exposure.
+time: 2026-07-29T10:13:27.549029-06:00

AGENTS.md

@@ -1,18 +1,110 @@
 # tfctl CLI
 
-A CLI for interacting with HCP Terraform and Terraform Enterprise in several ways, featuring a low-level API helper, high level commands, and interactive commands.
+`tfctl` is a Go CLI for HCP Terraform and Terraform Enterprise. It provides raw API access, high-level workflows, and commands for humans and coding agents.
 
-## Contributing
-**MUST follow these rules:**
+## Planning and Communication
+
+- Use ASD-STE100 Simplified Technical English in plans and documentation.
+- Make the smallest correct change. Follow the established package boundaries and command patterns.
+- Treat the rules in this file as requirements for new and substantially changed code. The known deviations below are technical debt, not examples to copy.
+
+## Development Setup
+
+- Use Go 1.26.4, git, bash, and make.
+- Run `scripts/setup.sh` to install development tools and the `tfctl` binary. The script does not install Go.
+- Run `make bin` to build the binary.
+- Run `make check` for formatting checks, lint, and standard tests. This target does not run the race detector.
+
+## Repository Architecture
+
+- `cmd/tfctl/main.go` is the process entry point. It creates I/O, logging, profiles, telemetry, the shared invocation, and the command tree.
+- `internal/commands/` contains command behavior. The top-level groups are `api`, `get`, `create`, `run`, `auth`, `variable`, `profile`, and `harness`.
+- `internal/pkg/` contains reusable infrastructure. Important packages include `cmd`, `client`, `format`, `iostreams`, `logging`, `telemetry`, `profile`, `openapi`, and `execsession`.
+- `internal/commands/*` can depend on `internal/pkg/*`. Do not add dependencies from infrastructure packages to command packages.
+- `skills/` contains embedded coding-agent skills.
+
+The CLI uses a custom command model in `internal/pkg/cmd` and adapts it to `github.com/hashicorp/cli`. `cmd.Invocation` carries shared I/O, output, profile, shutdown context, and parsed global state to command constructors.
+
+For runnable leaf commands, persistent pre-run applies global flags, configures the context logger, starts a telemetry span, and checks authentication. Group help, flag parse errors, and required-argument errors can return before persistent pre-run.
+
+## Command Design
 
 - Use TDD.
-- Respect the global --dry-run flag - when dry-run is enabled, don't change any data or execute any mutations.
-- For stdout rendering, create a displayer that can render --json, --markdown, and default (pretty) output.
-- Use ColorScheme formatting for stderr rendering.
-- When authoring a new command, pass an XXXOpts type value to a private runXXX function. Don't share the entire command context. Test functions should test the behavior of the runXXX function by varying the options passed to it.
-- Use the command Logger() to produce appropriate debug output.
-
-## Testing instructions
-- Test: `go test ./... -run "<MyTestFunc>"`
-- Lint: `golangci-lint run`
-- Test for regressions: `go test ./... -race`
+- Keep command declaration and flag wiring in `NewCmdXxx`.
+- Put command behavior in a private `runXxx` function. Pass an `XxxOpts` value that contains only the required dependencies and values.
+- Do not pass `*cmd.Invocation` to `runXxx`. Resolve invocation state and construct clients in command wiring, then pass explicit dependencies in the options value.
+- Test `runXxx` directly by varying its options. Add `Command.Run` tests when flag parsing, argument validation, autocomplete, or exit behavior needs coverage.
+- Group commands with no `RunF` do not need an options value or behavior function.
+- Keep shared behavior private unless another command package has a concrete need to call it.
+
+## Global Flags
+
+Command changes must account for these global flags:
+
+- `--dry-run` must prevent remote mutations and command-specific state changes. Report the skipped action to stderr.
+- `--quiet` suppresses `IOStreams.ErrUnessential()` and disables prompts. It does not automatically suppress stdout or `IOStreams.Err()`.
+- `--no-color` disables command-facing color and styling.
+- `--debug` controls the context logger level.
+- `--profile` replaces the active profile for the invocation.
+
+## Mutation Safety
+
+- Check dry-run state before every write, mutation request, browser launch, child process, or other command-specific side effect.
+- In dry-run mode, do not mutate shared in-memory values as a substitute for avoiding a persisted write. Use a copy when validation needs a proposed value.
+- Render dry-run details to `IOStreams.Err()` with `ColorScheme.DryRunLabel()`.
+- Do not rely on `client.Resolver.dryRun` to block creation. The field is not enforced. Callers must set `createIfNotFound` to false or guard the mutation before calling the resolver.
+- Keep destructive API operations behind the existing confirmation and exec-session checks. Harness exec-session permission applies only to selected API deletes and is not a general mutation permission.
+
+## Output and Diagnostics
+
+- Send structured stdout through `format.Outputter` with a `format.Displayer`.
+- A displayer must provide a default format, a payload, and field templates that work with forced JSON and Markdown output. The outputter also supports pretty and table output. `format.Agent` currently renders as JSON.
+- Use direct stdout only for an intentional raw byte stream or child-process pass-through. Document why global format conversion does not apply.
+- Never include credentials or sensitive values in a displayer payload. JSON output serializes the full payload, not only the displayed field templates.
+- Use `IOStreams.Err()` for essential diagnostics that must remain visible with `--quiet`.
+- Use `IOStreams.ErrUnessential()` for progress, guidance, and routine success messages that `--quiet` can suppress.
+- Use `IOStreams.ColorScheme()` for command-facing stderr styling. Logging has separate hclog color handling.
+- A command that must suppress stdout in quiet mode must implement that behavior explicitly.
+
+## Logging and Telemetry
+
+- Get the logger with `logging.FromContext(ctx)`.
+- Add debug logs for useful decisions, fallback behavior, ignored nonfatal errors, and external operations.
+- Do not log tokens, credentials, sensitive variable values, or request bodies that can contain secrets.
+- Pass the command context to API and other blocking calls so cancellation and telemetry propagate.
+- Telemetry command spans exist only for runnable commands that reach persistent pre-run. Do not assume that help and parse-error paths have a command span.
+
+## Arguments, Flags, and Help
+
+- Set `Command.Args.Autocomplete` for positional-argument completion. `PositionalArgument` does not have an `Autocomplete` field.
+- Set `Flag.Autocomplete` for flags that accept values. Use an appropriate `complete.Predictor`.
+- If autocomplete would be incorrect, omit it and add a short comment. `harness exec` is an example because its trailing arguments belong to another executable.
+- Add examples and clear help for user-facing behavior.
+- Run `make gen/screenshot` when root command output changes.
+
+## Profiles and Configuration
+
+- Profiles are HCL files under the `profiles/` configuration directory. The configuration root also contains `active_profile.hcl`, `device_id`, and host caches.
+- `Profile.Predict` completes profile property names. Profile-name completion uses `Loader.ListProfiles`.
+- Hostname helpers default, normalize, and validate hostnames. They do not classify HCP Terraform and Terraform Enterprise.
+- Selected commands can use local Terraform configuration as an organization or workspace fallback.
+- `auth login --token` reads a token from stdin. It does not accept the token as the flag value.
+
+## Testing and Release Checks
+
+- Write a failing test before the implementation change.
+- Run a focused test with `go test ./... -run '<TestFunc>'`.
+- Run lint with `golangci-lint run`.
+- Run regression and race tests with `go test ./... -race`.
+- Use `cmdtest.NewServer` for routed HTTP test servers and `cmdtest.WriteJSONAPI` when a handler needs a JSON:API response.
+- Format tests use inline expected output rather than golden files.
+- Run `changie new` to prepare a changelog entry for a user-visible change.
+
+## Known Architecture Deviations
+
+Do not reproduce these patterns in new code. Fix a deviation when it is in the direct scope of the change.
+
+- `internal/commands/profile/set.go` decodes proposed values directly into the shared profile before the dry-run check.
+- `client.Resolver` stores `dryRun` but does not read it. Creation safety depends on each caller.
+- Some ignored nonfatal errors have no debug log. For example, `auth status` suppresses token-expiration lookup failures.
+- Telemetry shutdown runs after normal CLI dispatch, but early returns such as the root banner path bypass it.

README.md

@@ -201,7 +201,7 @@ The `tfctl` command can manage HCP Terraform runs and variables with the corresp
 
 - `--debug`: Enable debug output.
 
-- `--dry-run`: Creates a preview of the proposed changes.
+- `--dry-run`: Creates a preview of the proposed changes without making them.
 
 - `--json`: Sets the output format to JSON.
 

cmd/tfctl/main.go

@@ -197,6 +197,7 @@ func realMain() int {
 			"--no-color": complete.PredictAnything,
 			"--profile":  profiles.PredictProfiles(false, true),
 			"--quiet":    complete.PredictAnything,
+			"--dry-run":  complete.PredictAnything,
 		},
 	}
 

internal/commands/auth/login.go

@@ -33,13 +33,31 @@ const (
 // non-error exit rather than a failure.
 var errLoginCanceled = errors.New("login canceled")
 
+// LoginOpts defines the options for the `auth login` command.
+type LoginOpts struct {
+	IO           iostreams.IOStreams
+	Hostname     string
+	Output       *format.Outputter
+	NewAPIClient func(token string) (*client.Client, error)
+	Profile      *profile.Profile
+
+	// OpenBrowser opens a URL in the user's default browser. When nil, the
+	// package default openBrowser is used. Tests inject a no-op opener to avoid
+	// launching a real browser and to keep parallel tests free of shared state.
+	OpenBrowser func(url string) error
+
+	Name           string
+	TokenFromStdin bool
+	DryRun         bool
+}
+
 // NewCmdLogin returns the `auth login` command for authenticating.
 func NewCmdLogin(inv *cmd.Invocation) *cmd.Command {
-	opts := &LoginOpts{
+	opts := LoginOpts{
 		IO:          inv.IO,
-		Profile:     inv.Profile,
 		Output:      inv.Output,
 		OpenBrowser: openBrowser,
+		Profile:     inv.Profile,
 	}
 
 	cmd := &cmd.Command{
@@ -72,47 +90,44 @@ func NewCmdLogin(inv *cmd.Invocation) *cmd.Command {
 				{
 					Name:          "token",
 					Description:   "Read the token from standard input instead of prompting.",
-					Value:         flagvalue.Simple(false, &opts.Token),
+					Value:         flagvalue.Simple(false, &opts.TokenFromStdin),
 					IsBooleanFlag: true,
 				},
 			},
 		},
 		NoAuthRequired: true,
 		RunF: func(_ *cmd.Command, _ []string) error {
 			opts.DryRun = inv.IsDryRun()
-			return loginRun(inv.ShutdownCtx, inv, opts)
+			opts.Hostname = inv.Profile.GetHostname()
+			opts.NewAPIClient = func(token string) (*client.Client, error) {
+				return inv.NewAPIClientForHost(inv.Profile.GetHostname(), token)
+			}
+			if opts.DryRun {
+				opts.OpenBrowser = func(url string) error {
+					cs := opts.IO.ColorScheme()
+					fmt.Fprintf(opts.IO.Err(), "%s would open the web browser to URL %s\n",
+						cs.DryRunLabel(), url)
+					return fmt.Errorf("can't open web browser when --dry-run is enabled")
+				}
+			}
+
+			return loginRun(inv.ShutdownCtx, opts)
 		},
 	}
 
 	return cmd
 }
 
-// LoginOpts defines the options for the `auth login` command.
-type LoginOpts struct {
-	IO      iostreams.IOStreams
-	Profile *profile.Profile
-	Output  *format.Outputter
-
-	// OpenBrowser opens a URL in the user's default browser. When nil, the
-	// package default openBrowser is used. Tests inject a no-op opener to avoid
-	// launching a real browser and to keep parallel tests free of shared state.
-	OpenBrowser func(url string) error
-
-	Name   string
-	Token  bool
-	DryRun bool
-}
-
-func loginRun(ctx context.Context, inv *cmd.Invocation, opts *LoginOpts) error {
-	hostname := opts.Profile.GetHostname()
+func loginRun(ctx context.Context, opts LoginOpts) error {
+	hostname := opts.Hostname
 	logger := logging.FromContext(ctx)
 
-	logger.Debug("starting login process", "hostname", hostname, "token_from_stdin", opts.Token)
+	logger.Debug("Starting login process", "hostname", hostname, "token_from_stdin", opts.TokenFromStdin)
 
 	// Read the token.
 	var token string
 	var err error
-	if opts.Token {
+	if opts.TokenFromStdin {
 		token, err = readTokenFromStdin(opts)
 	} else {
 		token, err = readTokenInteractive(opts, hostname)
@@ -126,18 +141,15 @@ func loginRun(ctx context.Context, inv *cmd.Invocation, opts *LoginOpts) error {
 	}
 
 	// Set the token on the profile and create a client to verify it.
-	opts.Profile.Token = token
-	logger.Debug("verifying token", "hostname", hostname)
-	apiClient, err := inv.NewAPIClient()
+	err = saveToken(ctx, opts, hostname, token)
 	if err != nil {
-		return fmt.Errorf("failed to create API client: %w", err)
+		return err
 	}
-
-	return saveToken(ctx, opts, apiClient, hostname, token)
+	return nil
 }
 
 // readTokenFromStdin reads a token from stdin.
-func readTokenFromStdin(opts *LoginOpts) (string, error) {
+func readTokenFromStdin(opts LoginOpts) (string, error) {
 	scanner := bufio.NewScanner(opts.IO.In())
 	if !scanner.Scan() {
 		if err := scanner.Err(); err != nil {
@@ -157,7 +169,7 @@ func readTokenFromStdin(opts *LoginOpts) (string, error) {
 // readTokenInteractive explains the flow, asks the user to confirm, opens the
 // browser to the token page, and prompts for the generated token. The user must
 // confirm before the browser is opened; declining is a clean exit.
-func readTokenInteractive(opts *LoginOpts, hostname string) (string, error) {
+func readTokenInteractive(opts LoginOpts, hostname string) (string, error) {
 	if !opts.IO.CanPrompt() {
 		return "", fmt.Errorf("interactive login requires a terminal; use --token to read from stdin")
 	}
@@ -190,9 +202,13 @@ func readTokenInteractive(opts *LoginOpts, hostname string) (string, error) {
 		openURL = openBrowser
 	}
 	if err := openURL(tokenURL); err != nil {
+		reason := ""
+		if opts.DryRun {
+			reason = " when --dry-run is enabled"
+		}
 		fmt.Fprintf(opts.IO.Err(),
-			"%s Could not open the browser automatically. Open the URL above manually.\n\n",
-			cs.WarningLabel())
+			"%s Could not open the browser automatically%s. Visit the URL above to continue.\n\n",
+			cs.WarningLabel(), reason)
 	}
 
 	// Prompt for the token and read it without echoing.
@@ -215,22 +231,31 @@ func readTokenInteractive(opts *LoginOpts, hostname string) (string, error) {
 }
 
 // saveToken verifies the token via the API and persists it to the profile.
-func saveToken(ctx context.Context, opts *LoginOpts, apiClient *client.Client, hostname, token string) error {
-	cs := opts.IO.ColorScheme()
+func saveToken(ctx context.Context, opts LoginOpts, hostname, token string) error {
+	logger := logging.FromContext(ctx)
+
+	apiClient, err := opts.NewAPIClient(token)
+	if err != nil {
+		return fmt.Errorf("failed to create API client: %w", err)
+	}
 
+	logger.Debug("Verifying token", "hostname", hostname)
 	user, err := verifyToken(ctx, apiClient)
 	if err != nil {
 		return fmt.Errorf("failed to verify token: %w", err)
 	}
 
+	cs := opts.IO.ColorScheme()
 	if opts.DryRun {
 		fmt.Fprintf(opts.IO.Err(), "%s would save token to profile %q for host %s (user: %s)\n",
 			cs.DryRunLabel(), opts.Profile.Name, hostname, user)
 		return nil
 	}
 
-	opts.Profile.Token = token
-	if err := opts.Profile.Write(); err != nil {
+	profile := opts.Profile
+
+	profile.Token = token
+	if err := profile.Write(); err != nil {
 		return fmt.Errorf("failed to save token to profile: %w", err)
 	}