From 8a98d0c3521fd69fc21a9c4356a8f2a34042f331 Mon Sep 17 00:00:00 2001 From: Rian Stockbower Date: Wed, 24 Jun 2026 17:43:13 -0400 Subject: [PATCH 1/4] Migrate config diagnostics to presenters Closes #436 --- .../271-436-cfl-diagnostics-advisories.md | 209 ++++++++++++++++++ tools/cfl/internal/cmd/configcmd/clear.go | 53 +---- .../cfl/internal/cmd/configcmd/clear_test.go | 22 +- tools/cfl/internal/cmd/configcmd/test.go | 43 +--- tools/cfl/internal/cmd/configcmd/test_test.go | 31 ++- tools/cfl/internal/present/config.go | 121 ++++++++++ tools/cfl/internal/present/config_test.go | 112 ++++++++++ 7 files changed, 500 insertions(+), 91 deletions(-) create mode 100644 docs/proofs/271-436-cfl-diagnostics-advisories.md create mode 100644 tools/cfl/internal/present/config.go create mode 100644 tools/cfl/internal/present/config_test.go diff --git a/docs/proofs/271-436-cfl-diagnostics-advisories.md b/docs/proofs/271-436-cfl-diagnostics-advisories.md new file mode 100644 index 00000000..5bff8268 --- /dev/null +++ b/docs/proofs/271-436-cfl-diagnostics-advisories.md @@ -0,0 +1,209 @@ +# Proof: #436 `cfl` Diagnostics and Advisory Presenter Migration + +## Scope + +This proof covers presenter-owned diagnostic/advisory/status output for: + +- `config test` +- `config clear` + +The migrated commands now: + +- orchestrate config, API, keyring, and prompt control flow only +- emit presenter-owned diagnostic/status models through + `tools/cfl/internal/present/config.go` +- keep one-shot confirmation prompt text as an explicit direct `stderr` + exception +- keep `cfl init` wizard output outside this ticket + +Pagination advisories and the `page edit --legacy` warning were already moved +to presenters in earlier child tickets. `#435` contains the exact command and +live proof for the legacy-editor advisory. + +## Verification Commands + +Executed: + +```bash +rtk go test ./tools/cfl/internal/cmd/configcmd ./tools/cfl/internal/present +``` + +Result: + +```text +Go test: 54 passed in 2 packages +``` + +Executed: + +```bash +rtk go test ./tools/cfl/internal/present ./tools/cfl/internal/cmd/configcmd ./tools/cfl/internal/cmd/page ./tools/cfl/internal/cmd/root ./shared/present +``` + +Result: + +```text +Go test: 283 passed in 5 packages +``` + +Executed: + +```bash +rtk golangci-lint run ./tools/cfl/internal/present ./tools/cfl/internal/cmd/configcmd ./tools/cfl/internal/cmd/page ./tools/cfl/internal/cmd/root +``` + +Result: + +```text +golangci-lint: No issues found +``` + +Executed: + +```bash +rtk go test ./tools/cfl/... ./shared/... +``` + +Result: + +```text +Go test: 1633 passed in 34 packages +``` + +Executed: + +```bash +rtk proxy git diff --check +``` + +Result: + +```text + +``` + +## Grep Gates + +Executed: + +```bash +rtk rg -n 'fmt\.Fprint|fmt\.Fprintf|fmt\.Fprintln' tools/cfl/internal/cmd/configcmd/test.go tools/cfl/internal/cmd/configcmd/clear.go +``` + +Result: + +```text +tools/cfl/internal/cmd/configcmd/clear.go:87: _, _ = fmt.Fprint(opts.Stderr, promptText+" [y/N]: ") +``` + +This is the documented prompt exception. + +Executed: + +```bash +rtk rg -n 'Testing connection|Troubleshooting|Authenticated as|No stored API token|Cancelled\. Nothing was cleared|Removed key|Removed the shared keyring bundle|Warning: this is the SHARED token|Note: .*environment' tools/cfl/internal/cmd/configcmd/test.go tools/cfl/internal/cmd/configcmd/clear.go +``` + +Result: + +```text +tools/cfl/internal/cmd/configcmd/clear.go:43:Note: CFL_API_TOKEN / ATLASSIAN_API_TOKEN environment variables still +``` + +This remaining match is command help text, not runtime diagnostic/status +output. + +## Test Evidence + +Presenter tests in `tools/cfl/internal/present/config_test.go` assert exact +`OutputModel` messages and stderr routing for: + +- config-test success with user details +- config-test fallback success when user lookup fails after connectivity works +- config-test failure/troubleshooting +- clear planned default action +- clear planned `--all` action, including plaintext cleanup and keyring + unavailable notes +- clear cancelled +- clear success +- clear `--all` success +- no stored token +- environment override notes + +Command tests now assert exact stdout/stderr for: + +- `config test` success +- `config test` fallback success +- `config test` connection failure +- `config clear` no-op +- `config clear` no-op with an environment override note, without exposing the + token value +- `config clear --force` default deletion +- interactive `config clear` confirm/cancel prompt behavior +- `config clear --non-interactive` without `--force`, proving no preview or + warning leaks before `prompt.ErrConfirmationRequired` +- `config clear --non-interactive --force` +- `config clear --all` + +## Live CLI Transcript + +The binary was rebuilt: + +```bash +rtk go build -o ./bin/cfl ./tools/cfl/cmd/cfl +``` + +Transcript directory: + +```text +/tmp/cfl-436-proof.u3byyu +``` + +Executed: + +```bash +./bin/cfl --no-color config test +``` + +Result: + +```text +exit status: 0 +stdout bytes: 0 +stderr bytes: 169 +``` + +Observed stderr, with identity fields redacted: + +```text +Testing connection... success! + +Authentication successful +API access verified + +Authenticated as: () +Account ID: +``` + +Executed: + +```bash +./bin/cfl --no-color --non-interactive config clear +``` + +Result: + +```text +exit status: 1 +stdout bytes: 0 +stderr bytes: 80 +``` + +Observed stderr: + +```text +Error: --non-interactive: confirmation required; re-run with --force to proceed +``` + +This proves the safe non-destructive case fails loudly before prompt preview, +shared-token warning, keyring deletion, or any token value exposure. diff --git a/tools/cfl/internal/cmd/configcmd/clear.go b/tools/cfl/internal/cmd/configcmd/clear.go index 01f63639..b38dacb0 100644 --- a/tools/cfl/internal/cmd/configcmd/clear.go +++ b/tools/cfl/internal/cmd/configcmd/clear.go @@ -4,7 +4,6 @@ import ( "fmt" "io" "os" - "strings" "github.com/spf13/cobra" @@ -13,6 +12,7 @@ import ( promptpkg "github.com/open-cli-collective/atlassian-go/prompt" "github.com/open-cli-collective/confluence-cli/internal/cmd/root" + cflpresent "github.com/open-cli-collective/confluence-cli/internal/present" ) type clearOptions struct { @@ -89,40 +89,14 @@ func runClear(opts *clearOptions) error { return promptpkg.ConfirmOrFail(opts.force, opts.NonInteractive, opts.stdin) } - envNote := func() { - if len(plan.EnvActive) > 0 { - _, _ = fmt.Fprintf(opts.Stderr, - "Note: %s still set in the environment and will continue to override at runtime (not cleared).\n", - strings.Join(plan.EnvActive, ", ")) - } - } - if opts.all { - _, _ = fmt.Fprintf(opts.Stderr, "This will remove the ENTIRE shared keyring bundle %s", plan.Ref) - if len(plan.ExistingKeys) > 0 { - _, _ = fmt.Fprintf(opts.Stderr, " (keys: %s)", strings.Join(plan.ExistingKeys, ", ")) - } - _, _ = fmt.Fprintln(opts.Stderr, ".") - if plan.SharedConfigPath != "" { - _, _ = fmt.Fprintf(opts.Stderr, "It will also delete the shared config file: %s\n", plan.SharedConfigPath) - } - if plan.OldSharedConfigPath != "" { - _, _ = fmt.Fprintf(opts.Stderr, "It will also delete the prior shared config file: %s\n", plan.OldSharedConfigPath) - } - for _, lp := range plan.LegacyPaths { - _, _ = fmt.Fprintf(opts.Stderr, "It will scrub the legacy plaintext file: %s\n", lp) - } - if err != nil { - _, _ = fmt.Fprintf(opts.Stderr, - "Note: the keyring could not be opened (%v); plaintext artifacts will still be cleaned, but the keyring bundle will be left intact.\n", err) - } + _ = cflpresent.Emit(opts.Options, cflpresent.ConfigPresenter{}.PresentClearAllPlan(plan, err)) ok, cerr := confirm("Proceed?") if cerr != nil { return cerr } if !ok { - _, _ = fmt.Fprintln(opts.Stderr, "Cancelled. Nothing was cleared.") - return nil + return cflpresent.Emit(opts.Options, cflpresent.ConfigPresenter{}.PresentClearCancelled()) } cleared, aerr := keyring.ClearAll(store) if aerr != nil { @@ -133,34 +107,23 @@ func runClear(opts *clearOptions) error { "plaintext artifacts were cleaned, but the keyring bundle %s was NOT cleared because the keyring is unavailable (%w); fix the keyring and re-run `cfl config clear --all`", plan.Ref, err) } - _, _ = fmt.Fprintln(opts.Stderr, "Removed the shared keyring bundle and config file.") - envNote() - return nil + return cflpresent.Emit(opts.Options, cflpresent.ConfigPresenter{}.PresentClearAllSuccess(plan)) } if plan.ToolKey == "" { - _, _ = fmt.Fprintf(opts.Stderr, "No stored API token in keyring %s for cfl; nothing to clear.\n", plan.Ref) - envNote() - return nil + return cflpresent.Emit(opts.Options, cflpresent.ConfigPresenter{}.PresentClearNoStoredToken(plan)) } - _, _ = fmt.Fprintf(opts.Stderr, "This will delete key %q from keyring %s.\n", plan.ToolKey, plan.Ref) - // One key per logical credential (§1.11.10): the only deletable key - // is the shared api_token, so clearing it always deauths the sibling. - _, _ = fmt.Fprintln(opts.Stderr, - "Warning: this is the SHARED token (api_token). jtk will also lose access (cfl and jtk resolve the same key).") + _ = cflpresent.Emit(opts.Options, cflpresent.ConfigPresenter{}.PresentClearDefaultPlan(plan)) ok, cerr := confirm("Proceed?") if cerr != nil { return cerr } if !ok { - _, _ = fmt.Fprintln(opts.Stderr, "Cancelled. Nothing was cleared.") - return nil + return cflpresent.Emit(opts.Options, cflpresent.ConfigPresenter{}.PresentClearCancelled()) } if err := store.DeleteToken(plan.ToolKey); err != nil { return err } - _, _ = fmt.Fprintf(opts.Stderr, "Removed key %q from keyring %s.\n", plan.ToolKey, plan.Ref) - envNote() - return nil + return cflpresent.Emit(opts.Options, cflpresent.ConfigPresenter{}.PresentClearDefaultSuccess(plan)) } diff --git a/tools/cfl/internal/cmd/configcmd/clear_test.go b/tools/cfl/internal/cmd/configcmd/clear_test.go index 4f4c37a0..6143cf61 100644 --- a/tools/cfl/internal/cmd/configcmd/clear_test.go +++ b/tools/cfl/internal/cmd/configcmd/clear_test.go @@ -3,6 +3,7 @@ package configcmd import ( "bytes" "errors" + "fmt" "os" "strings" "testing" @@ -38,7 +39,16 @@ func TestRunClear_NothingToClear(t *testing.T) { credtest.Hermetic(t) opts, _, errBuf := newClearOpts(true, "") testutil.RequireNoError(t, runClear(opts)) - testutil.Contains(t, errBuf.String(), "nothing to clear") + testutil.Equal(t, fmt.Sprintf("No stored API token in keyring %s for cfl; nothing to clear.\n", keyring.Ref), errBuf.String()) +} + +func TestRunClear_NothingToClear_WithEnvOverrideNote(t *testing.T) { + credtest.Hermetic(t) + t.Setenv("CFL_API_TOKEN", "env-token") + opts, _, errBuf := newClearOpts(true, "") + testutil.RequireNoError(t, runClear(opts)) + testutil.Equal(t, fmt.Sprintf("No stored API token in keyring %s for cfl; nothing to clear.\nNote: CFL_API_TOKEN still set in the environment and will continue to override at runtime (not cleared).\n", keyring.Ref), errBuf.String()) + testutil.NotContains(t, errBuf.String(), "env-token") } func TestRunClear_DeletesSharedKey_WithForce(t *testing.T) { @@ -49,8 +59,7 @@ func TestRunClear_DeletesSharedKey_WithForce(t *testing.T) { testutil.RequireNoError(t, runClear(opts)) testutil.False(t, tokenPresent(t, keyring.KeyAPIToken)) - // Shared-default deletion must warn that the sibling loses access. - testutil.Contains(t, errBuf.String(), "jtk will also lose access") + testutil.Equal(t, fmt.Sprintf("This will delete key %q from keyring %s.\nWarning: this is the SHARED token (api_token). jtk will also lose access (cfl and jtk resolve the same key).\nRemoved key %q from keyring %s.\n", keyring.KeyAPIToken, keyring.Ref, keyring.KeyAPIToken, keyring.Ref), errBuf.String()) } func TestRunClear_DeletesSharedKey_Confirmed(t *testing.T) { @@ -63,7 +72,7 @@ func TestRunClear_DeletesSharedKey_Confirmed(t *testing.T) { // One key per logical credential (§1.11.10): a confirmed clear removes // the single shared api_token and warns the sibling loses access. testutil.False(t, tokenPresent(t, keyring.KeyAPIToken)) - testutil.Contains(t, errBuf.String(), "jtk will also lose access") + testutil.Equal(t, fmt.Sprintf("This will delete key %q from keyring %s.\nWarning: this is the SHARED token (api_token). jtk will also lose access (cfl and jtk resolve the same key).\nProceed? [y/N]: Removed key %q from keyring %s.\n", keyring.KeyAPIToken, keyring.Ref, keyring.KeyAPIToken, keyring.Ref), errBuf.String()) // Removed per-tool override keys must never be advised again. testutil.NotContains(t, errBuf.String(), "cfl_api_token") testutil.NotContains(t, errBuf.String(), "override") @@ -76,10 +85,11 @@ func TestRunClear_Cancelled(t *testing.T) { credtest.Hermetic(t) credtest.SeedToken(t, "shared-secret") - opts, _, _ := newClearOpts(false, "n\n") + opts, _, errBuf := newClearOpts(false, "n\n") testutil.RequireNoError(t, runClear(opts)) testutil.True(t, tokenPresent(t, keyring.KeyAPIToken)) + testutil.Equal(t, fmt.Sprintf("This will delete key %q from keyring %s.\nWarning: this is the SHARED token (api_token). jtk will also lose access (cfl and jtk resolve the same key).\nProceed? [y/N]: Cancelled. Nothing was cleared.\n", keyring.KeyAPIToken, keyring.Ref), errBuf.String()) } // TestRunClear_NonInteractive_WithoutForce_ShortCircuits — §3.4 contract @@ -122,6 +132,7 @@ func TestRunClear_NonInteractive_WithForce_Proceeds(t *testing.T) { testutil.RequireNoError(t, runClear(opts)) testutil.False(t, tokenPresent(t, keyring.KeyAPIToken)) + testutil.Equal(t, fmt.Sprintf("This will delete key %q from keyring %s.\nWarning: this is the SHARED token (api_token). jtk will also lose access (cfl and jtk resolve the same key).\nRemoved key %q from keyring %s.\n", keyring.KeyAPIToken, keyring.Ref, keyring.KeyAPIToken, keyring.Ref), opts.Stderr.(*bytes.Buffer).String()) } func TestRunClear_All(t *testing.T) { @@ -138,4 +149,5 @@ func TestRunClear_All(t *testing.T) { testutil.False(t, tokenPresent(t, keyring.KeyAPIToken)) _, statErr := os.Stat(sharedPath) testutil.True(t, os.IsNotExist(statErr)) + testutil.Equal(t, fmt.Sprintf("This will remove the ENTIRE shared keyring bundle %s (keys: %s).\nIt will also delete the shared config file: %s\nRemoved the shared keyring bundle and config file.\n", keyring.Ref, keyring.KeyAPIToken, sharedPath), opts.Stderr.(*bytes.Buffer).String()) } diff --git a/tools/cfl/internal/cmd/configcmd/test.go b/tools/cfl/internal/cmd/configcmd/test.go index e31b44d9..e0110d9f 100644 --- a/tools/cfl/internal/cmd/configcmd/test.go +++ b/tools/cfl/internal/cmd/configcmd/test.go @@ -7,6 +7,7 @@ import ( "github.com/spf13/cobra" "github.com/open-cli-collective/confluence-cli/internal/cmd/root" + cflpresent "github.com/open-cli-collective/confluence-cli/internal/present" ) func newTestCmd(opts *root.Options) *cobra.Command { @@ -34,55 +35,19 @@ func runTest(ctx context.Context, opts *root.Options) error { return fmt.Errorf("configuration error: %w", err) } - _, _ = fmt.Fprint(opts.Stderr, "Testing connection... ") - // Try to list spaces (limit 1) to verify connectivity _, err = client.ListSpaces(ctx, nil) if err != nil { - _, _ = fmt.Fprintln(opts.Stderr, "failed!") - _, _ = fmt.Fprintln(opts.Stderr) - _, _ = fmt.Fprintln(opts.Stderr, "Troubleshooting:") - _, _ = fmt.Fprintln(opts.Stderr, " - Verify your URL is correct (should include https://)") - _, _ = fmt.Fprintln(opts.Stderr, " - Check your email and API token") - _, _ = fmt.Fprintln(opts.Stderr, " - Ensure your API token hasn't expired") - _, _ = fmt.Fprintln(opts.Stderr, " - Verify you have permission to access Confluence") - _, _ = fmt.Fprintln(opts.Stderr) - _, _ = fmt.Fprintln(opts.Stderr, "To regenerate an API token:") - _, _ = fmt.Fprintln(opts.Stderr, " https://id.atlassian.com/manage-profile/security/api-tokens") + _ = cflpresent.Emit(opts, cflpresent.ConfigPresenter{}.PresentTestFailure()) return fmt.Errorf("connection test failed: %w", err) } - _, _ = fmt.Fprintln(opts.Stderr, "success!") - _, _ = fmt.Fprintln(opts.Stderr) - // Get current user details user, err := client.GetCurrentUser(ctx) if err != nil { // User details failed but connection worked - show basic success - _, _ = fmt.Fprintln(opts.Stderr, "Your cfl configuration is working correctly.") - return nil - } - - _, _ = fmt.Fprintln(opts.Stderr, "Authentication successful") - _, _ = fmt.Fprintln(opts.Stderr, "API access verified") - _, _ = fmt.Fprintln(opts.Stderr) - - // Display user info - try DisplayName first, fall back to PublicName - displayName := user.DisplayName - if displayName == "" { - displayName = user.PublicName - } - - if displayName != "" { - if user.Email != "" { - _, _ = fmt.Fprintf(opts.Stderr, "Authenticated as: %s (%s)\n", displayName, user.Email) - } else { - _, _ = fmt.Fprintf(opts.Stderr, "Authenticated as: %s\n", displayName) - } - } - if user.AccountID != "" { - _, _ = fmt.Fprintf(opts.Stderr, "Account ID: %s\n", user.AccountID) + return cflpresent.Emit(opts, cflpresent.ConfigPresenter{}.PresentTestSuccess(nil)) } - return nil + return cflpresent.Emit(opts, cflpresent.ConfigPresenter{}.PresentTestSuccess(user)) } diff --git a/tools/cfl/internal/cmd/configcmd/test_test.go b/tools/cfl/internal/cmd/configcmd/test_test.go index f28bc006..37d3a5b8 100644 --- a/tools/cfl/internal/cmd/configcmd/test_test.go +++ b/tools/cfl/internal/cmd/configcmd/test_test.go @@ -43,8 +43,31 @@ func TestRunTest_Success(t *testing.T) { err := runTest(context.Background(), rootOpts) testutil.RequireNoError(t, err) - // Note: Output goes to real stdout via fmt.Print, not opts.Stdout - // Just verifying no error is sufficient for this test + testutil.Equal(t, "", rootOpts.Stdout.(*bytes.Buffer).String()) + testutil.Equal(t, "Testing connection... success!\n\nAuthentication successful\nAPI access verified\n\nAuthenticated as: Test User (test@example.com)\nAccount ID: 123\n", rootOpts.Stderr.(*bytes.Buffer).String()) +} + +func TestRunTest_SuccessUserLookupFallback(t *testing.T) { + t.Parallel() + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if strings.Contains(r.URL.Path, "/user/current") { + w.WriteHeader(http.StatusInternalServerError) + _, _ = w.Write([]byte(`{"message": "user lookup failed"}`)) + return + } + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`{"results": []}`)) + })) + defer server.Close() + + rootOpts := newTestRootOptions() + client := api.NewClient(server.URL, "test@example.com", "token") + rootOpts.SetAPIClient(client) + + err := runTest(context.Background(), rootOpts) + testutil.RequireNoError(t, err) + testutil.Equal(t, "", rootOpts.Stdout.(*bytes.Buffer).String()) + testutil.Equal(t, "Testing connection... success!\n\nYour cfl configuration is working correctly.\n", rootOpts.Stderr.(*bytes.Buffer).String()) } func TestRunTest_AuthFailure(t *testing.T) { @@ -62,6 +85,8 @@ func TestRunTest_AuthFailure(t *testing.T) { err := runTest(context.Background(), rootOpts) testutil.RequireError(t, err) testutil.Contains(t, err.Error(), "connection test failed") + testutil.Equal(t, "", rootOpts.Stdout.(*bytes.Buffer).String()) + testutil.Equal(t, "Testing connection... failed!\n\nTroubleshooting:\n - Verify your URL is correct (should include https://)\n - Check your email and API token\n - Ensure your API token hasn't expired\n - Verify you have permission to access Confluence\n\nTo regenerate an API token:\n https://id.atlassian.com/manage-profile/security/api-tokens\n", rootOpts.Stderr.(*bytes.Buffer).String()) } func TestRunTest_ServerError(t *testing.T) { @@ -79,4 +104,6 @@ func TestRunTest_ServerError(t *testing.T) { err := runTest(context.Background(), rootOpts) testutil.RequireError(t, err) testutil.Contains(t, err.Error(), "connection test failed") + testutil.Equal(t, "", rootOpts.Stdout.(*bytes.Buffer).String()) + testutil.Equal(t, "Testing connection... failed!\n\nTroubleshooting:\n - Verify your URL is correct (should include https://)\n - Check your email and API token\n - Ensure your API token hasn't expired\n - Verify you have permission to access Confluence\n\nTo regenerate an API token:\n https://id.atlassian.com/manage-profile/security/api-tokens\n", rootOpts.Stderr.(*bytes.Buffer).String()) } diff --git a/tools/cfl/internal/present/config.go b/tools/cfl/internal/present/config.go new file mode 100644 index 00000000..d99a8a29 --- /dev/null +++ b/tools/cfl/internal/present/config.go @@ -0,0 +1,121 @@ +package present + +import ( + "fmt" + "strings" + + "github.com/open-cli-collective/atlassian-go/keyring" + sharedpresent "github.com/open-cli-collective/atlassian-go/present" + + "github.com/open-cli-collective/confluence-cli/api" +) + +type ConfigPresenter struct{} + +func (ConfigPresenter) PresentTestFailure() *sharedpresent.OutputModel { + return stderrOnly(strings.Join([]string{ + "Testing connection... failed!", + "", + "Troubleshooting:", + " - Verify your URL is correct (should include https://)", + " - Check your email and API token", + " - Ensure your API token hasn't expired", + " - Verify you have permission to access Confluence", + "", + "To regenerate an API token:", + " https://id.atlassian.com/manage-profile/security/api-tokens", + }, "\n")) +} + +func (ConfigPresenter) PresentTestSuccess(user *api.User) *sharedpresent.OutputModel { + lines := []string{"Testing connection... success!", ""} + if user == nil { + lines = append(lines, "Your cfl configuration is working correctly.") + return stderrOnly(strings.Join(lines, "\n")) + } + + lines = append(lines, "Authentication successful", "API access verified", "") + displayName := user.DisplayName + if displayName == "" { + displayName = user.PublicName + } + if displayName != "" { + if user.Email != "" { + lines = append(lines, fmt.Sprintf("Authenticated as: %s (%s)", displayName, user.Email)) + } else { + lines = append(lines, fmt.Sprintf("Authenticated as: %s", displayName)) + } + } + if user.AccountID != "" { + lines = append(lines, fmt.Sprintf("Account ID: %s", user.AccountID)) + } + + return stderrOnly(strings.Join(lines, "\n")) +} + +func (ConfigPresenter) PresentClearDefaultPlan(plan keyring.ClearPlan) *sharedpresent.OutputModel { + lines := []string{ + fmt.Sprintf("This will delete key %q from keyring %s.", plan.ToolKey, plan.Ref), + "Warning: this is the SHARED token (api_token). jtk will also lose access (cfl and jtk resolve the same key).", + } + return stderrOnly(strings.Join(lines, "\n")) +} + +func (ConfigPresenter) PresentClearAllPlan(plan keyring.ClearPlan, keyringErr error) *sharedpresent.OutputModel { + lines := []string{fmt.Sprintf("This will remove the ENTIRE shared keyring bundle %s%s.", + plan.Ref, + existingKeysSuffix(plan.ExistingKeys), + )} + if plan.SharedConfigPath != "" { + lines = append(lines, fmt.Sprintf("It will also delete the shared config file: %s", plan.SharedConfigPath)) + } + if plan.OldSharedConfigPath != "" { + lines = append(lines, fmt.Sprintf("It will also delete the prior shared config file: %s", plan.OldSharedConfigPath)) + } + for _, lp := range plan.LegacyPaths { + lines = append(lines, fmt.Sprintf("It will scrub the legacy plaintext file: %s", lp)) + } + if keyringErr != nil { + lines = append(lines, fmt.Sprintf("Note: the keyring could not be opened (%v); plaintext artifacts will still be cleaned, but the keyring bundle will be left intact.", keyringErr)) + } + return stderrOnly(strings.Join(lines, "\n")) +} + +func (ConfigPresenter) PresentClearNoStoredToken(plan keyring.ClearPlan) *sharedpresent.OutputModel { + return stderrOnly(joinWithEnvNote( + fmt.Sprintf("No stored API token in keyring %s for cfl; nothing to clear.", plan.Ref), + plan.EnvActive, + )) +} + +func (ConfigPresenter) PresentClearCancelled() *sharedpresent.OutputModel { + return stderrOnly("Cancelled. Nothing was cleared.") +} + +func (ConfigPresenter) PresentClearDefaultSuccess(plan keyring.ClearPlan) *sharedpresent.OutputModel { + return stderrOnly(joinWithEnvNote( + fmt.Sprintf("Removed key %q from keyring %s.", plan.ToolKey, plan.Ref), + plan.EnvActive, + )) +} + +func (ConfigPresenter) PresentClearAllSuccess(plan keyring.ClearPlan) *sharedpresent.OutputModel { + return stderrOnly(joinWithEnvNote("Removed the shared keyring bundle and config file.", plan.EnvActive)) +} + +func existingKeysSuffix(keys []string) string { + if len(keys) == 0 { + return "" + } + return fmt.Sprintf(" (keys: %s)", strings.Join(keys, ", ")) +} + +func joinWithEnvNote(message string, envActive []string) string { + if len(envActive) == 0 { + return message + } + return message + "\n" + fmt.Sprintf( + "Note: %s still set in the environment and will continue to override at runtime (not cleared).", + strings.Join(envActive, ", "), + ) +} diff --git a/tools/cfl/internal/present/config_test.go b/tools/cfl/internal/present/config_test.go new file mode 100644 index 00000000..97dab5f5 --- /dev/null +++ b/tools/cfl/internal/present/config_test.go @@ -0,0 +1,112 @@ +package present + +import ( + "errors" + "testing" + + "github.com/open-cli-collective/atlassian-go/keyring" + sharedpresent "github.com/open-cli-collective/atlassian-go/present" + "github.com/open-cli-collective/atlassian-go/testutil" + + "github.com/open-cli-collective/confluence-cli/api" +) + +func TestConfigPresenter_PresentTestSuccess(t *testing.T) { + t.Parallel() + + model := ConfigPresenter{}.PresentTestSuccess(&api.User{ + AccountID: "acct-1", + DisplayName: "Test User", + Email: "test@example.com", + }) + + msg := requireMessageSection(t, model, 0) + testutil.Equal(t, sharedpresent.StreamStderr, msg.Stream) + testutil.Equal(t, "Testing connection... success!\n\nAuthentication successful\nAPI access verified\n\nAuthenticated as: Test User (test@example.com)\nAccount ID: acct-1", msg.Message) +} + +func TestConfigPresenter_PresentTestSuccessFallback(t *testing.T) { + t.Parallel() + + model := ConfigPresenter{}.PresentTestSuccess(nil) + + msg := requireMessageSection(t, model, 0) + testutil.Equal(t, sharedpresent.StreamStderr, msg.Stream) + testutil.Equal(t, "Testing connection... success!\n\nYour cfl configuration is working correctly.", msg.Message) +} + +func TestConfigPresenter_PresentTestFailure(t *testing.T) { + t.Parallel() + + model := ConfigPresenter{}.PresentTestFailure() + + msg := requireMessageSection(t, model, 0) + testutil.Equal(t, sharedpresent.StreamStderr, msg.Stream) + testutil.Equal(t, "Testing connection... failed!\n\nTroubleshooting:\n - Verify your URL is correct (should include https://)\n - Check your email and API token\n - Ensure your API token hasn't expired\n - Verify you have permission to access Confluence\n\nTo regenerate an API token:\n https://id.atlassian.com/manage-profile/security/api-tokens", msg.Message) +} + +func TestConfigPresenter_PresentClearDefaultPlan(t *testing.T) { + t.Parallel() + + model := ConfigPresenter{}.PresentClearDefaultPlan(keyring.ClearPlan{Ref: "atlassian-cli", ToolKey: "api_token"}) + + msg := requireMessageSection(t, model, 0) + testutil.Equal(t, sharedpresent.StreamStderr, msg.Stream) + testutil.Equal(t, "This will delete key \"api_token\" from keyring atlassian-cli.\nWarning: this is the SHARED token (api_token). jtk will also lose access (cfl and jtk resolve the same key).", msg.Message) +} + +func TestConfigPresenter_PresentClearAllPlan(t *testing.T) { + t.Parallel() + + model := ConfigPresenter{}.PresentClearAllPlan(keyring.ClearPlan{ + Ref: "atlassian-cli", + ExistingKeys: []string{"api_token", "other"}, + SharedConfigPath: "/tmp/shared.yml", + OldSharedConfigPath: "/tmp/old-shared.yml", + LegacyPaths: []string{"/tmp/.cfl.yml"}, + }, errors.New("locked")) + + msg := requireMessageSection(t, model, 0) + testutil.Equal(t, sharedpresent.StreamStderr, msg.Stream) + testutil.Equal(t, "This will remove the ENTIRE shared keyring bundle atlassian-cli (keys: api_token, other).\nIt will also delete the shared config file: /tmp/shared.yml\nIt will also delete the prior shared config file: /tmp/old-shared.yml\nIt will scrub the legacy plaintext file: /tmp/.cfl.yml\nNote: the keyring could not be opened (locked); plaintext artifacts will still be cleaned, but the keyring bundle will be left intact.", msg.Message) +} + +func TestConfigPresenter_PresentClearNoStoredToken(t *testing.T) { + t.Parallel() + + model := ConfigPresenter{}.PresentClearNoStoredToken(keyring.ClearPlan{Ref: "atlassian-cli", EnvActive: []string{"CFL_API_TOKEN"}}) + + msg := requireMessageSection(t, model, 0) + testutil.Equal(t, sharedpresent.StreamStderr, msg.Stream) + testutil.Equal(t, "No stored API token in keyring atlassian-cli for cfl; nothing to clear.\nNote: CFL_API_TOKEN still set in the environment and will continue to override at runtime (not cleared).", msg.Message) +} + +func TestConfigPresenter_PresentClearCancelled(t *testing.T) { + t.Parallel() + + model := ConfigPresenter{}.PresentClearCancelled() + + msg := requireMessageSection(t, model, 0) + testutil.Equal(t, sharedpresent.StreamStderr, msg.Stream) + testutil.Equal(t, "Cancelled. Nothing was cleared.", msg.Message) +} + +func TestConfigPresenter_PresentClearDefaultSuccess(t *testing.T) { + t.Parallel() + + model := ConfigPresenter{}.PresentClearDefaultSuccess(keyring.ClearPlan{Ref: "atlassian-cli", ToolKey: "api_token"}) + + msg := requireMessageSection(t, model, 0) + testutil.Equal(t, sharedpresent.StreamStderr, msg.Stream) + testutil.Equal(t, "Removed key \"api_token\" from keyring atlassian-cli.", msg.Message) +} + +func TestConfigPresenter_PresentClearAllSuccess(t *testing.T) { + t.Parallel() + + model := ConfigPresenter{}.PresentClearAllSuccess(keyring.ClearPlan{EnvActive: []string{"ATLASSIAN_API_TOKEN"}}) + + msg := requireMessageSection(t, model, 0) + testutil.Equal(t, sharedpresent.StreamStderr, msg.Stream) + testutil.Equal(t, "Removed the shared keyring bundle and config file.\nNote: ATLASSIAN_API_TOKEN still set in the environment and will continue to override at runtime (not cleared).", msg.Message) +} From 1261a3a038bd741f2e0f0353b24190dd320fe5e1 Mon Sep 17 00:00:00 2001 From: Rian Stockbower Date: Wed, 24 Jun 2026 17:47:14 -0400 Subject: [PATCH 2/4] Preserve config test progress timing --- .../271-436-cfl-diagnostics-advisories.md | 7 +++++-- shared/present/model.go | 7 ++++--- shared/present/render.go | 16 ++++++++++------ shared/present/render_test.go | 18 ++++++++++++++++++ tools/cfl/internal/cmd/configcmd/test.go | 2 ++ tools/cfl/internal/present/config.go | 15 +++++++++++++-- tools/cfl/internal/present/config_test.go | 12 +++++++++--- 7 files changed, 61 insertions(+), 16 deletions(-) diff --git a/docs/proofs/271-436-cfl-diagnostics-advisories.md b/docs/proofs/271-436-cfl-diagnostics-advisories.md index 5bff8268..ae999cbe 100644 --- a/docs/proofs/271-436-cfl-diagnostics-advisories.md +++ b/docs/proofs/271-436-cfl-diagnostics-advisories.md @@ -12,6 +12,8 @@ The migrated commands now: - orchestrate config, API, keyring, and prompt control flow only - emit presenter-owned diagnostic/status models through `tools/cfl/internal/present/config.go` +- preserve `config test` progress timing through an explicit no-newline + presenter message section - keep one-shot confirmation prompt text as an explicit direct `stderr` exception - keep `cfl init` wizard output outside this ticket @@ -43,7 +45,7 @@ rtk go test ./tools/cfl/internal/present ./tools/cfl/internal/cmd/configcmd ./to Result: ```text -Go test: 283 passed in 5 packages +Go test: 284 passed in 5 packages ``` Executed: @@ -67,7 +69,7 @@ rtk go test ./tools/cfl/... ./shared/... Result: ```text -Go test: 1633 passed in 34 packages +Go test: 1634 passed in 34 packages ``` Executed: @@ -119,6 +121,7 @@ Presenter tests in `tools/cfl/internal/present/config_test.go` assert exact `OutputModel` messages and stderr routing for: - config-test success with user details +- config-test no-newline progress - config-test fallback success when user lookup fails after connectivity works - config-test failure/troubleshooting - clear planned default action diff --git a/shared/present/model.go b/shared/present/model.go index fb22dc2e..5af578b5 100644 --- a/shared/present/model.go +++ b/shared/present/model.go @@ -95,9 +95,10 @@ const ( // MessageSection displays a status message (mutations, confirmations). type MessageSection struct { - Kind MessageKind - Message string - Stream Stream // Explicit stream routing (zero value = StreamStdout) + Kind MessageKind + Message string + Stream Stream // Explicit stream routing (zero value = StreamStdout) + NoNewline bool // For progress messages that intentionally complete later. } func (*MessageSection) sectionMarker() {} diff --git a/shared/present/render.go b/shared/present/render.go index ea152376..b54291fa 100644 --- a/shared/present/render.go +++ b/shared/present/render.go @@ -147,20 +147,24 @@ func renderTSVTable(sec *TableSection) string { } func renderMessage(sec *MessageSection, style Style) string { + terminator := "\n" + if sec.NoNewline { + terminator = "" + } if style == StyleAgent { // Plain text, no decorators - return sec.Message + "\n" + return sec.Message + terminator } // Human style with decorators switch sec.Kind { case MessageSuccess: - return "✓ " + sec.Message + "\n" + return "✓ " + sec.Message + terminator case MessageWarning: - return "⚠ " + sec.Message + "\n" + return "⚠ " + sec.Message + terminator case MessageError: - return "✗ " + sec.Message + "\n" + return "✗ " + sec.Message + terminator case MessageInfo: - return sec.Message + "\n" + return sec.Message + terminator } - return sec.Message + "\n" + return sec.Message + terminator } diff --git a/shared/present/render_test.go b/shared/present/render_test.go index ff434603..9d16a2d3 100644 --- a/shared/present/render_test.go +++ b/shared/present/render_test.go @@ -210,6 +210,24 @@ func TestRender_MessageSection_Info_Human(t *testing.T) { } } +func TestRender_MessageSection_NoNewline(t *testing.T) { + t.Parallel() + model := &OutputModel{ + Sections: []Section{ + &MessageSection{Kind: MessageInfo, Message: "Testing connection... ", Stream: StreamStderr, NoNewline: true}, + &MessageSection{Kind: MessageInfo, Message: "success!", Stream: StreamStderr}, + }, + } + + out := Render(model, StyleHuman) + if out.Stdout != "" { + t.Errorf("progress stdout should be empty, got: %q", out.Stdout) + } + if out.Stderr != "Testing connection... success!\n" { + t.Errorf("progress stderr:\ngot: %q\nwant: %q", out.Stderr, "Testing connection... success!\n") + } +} + func TestRender_MixedSections_WithWarning(t *testing.T) { t.Parallel() model := &OutputModel{ diff --git a/tools/cfl/internal/cmd/configcmd/test.go b/tools/cfl/internal/cmd/configcmd/test.go index e0110d9f..1e3450de 100644 --- a/tools/cfl/internal/cmd/configcmd/test.go +++ b/tools/cfl/internal/cmd/configcmd/test.go @@ -35,6 +35,8 @@ func runTest(ctx context.Context, opts *root.Options) error { return fmt.Errorf("configuration error: %w", err) } + _ = cflpresent.Emit(opts, cflpresent.ConfigPresenter{}.PresentTestProgress()) + // Try to list spaces (limit 1) to verify connectivity _, err = client.ListSpaces(ctx, nil) if err != nil { diff --git a/tools/cfl/internal/present/config.go b/tools/cfl/internal/present/config.go index d99a8a29..82711245 100644 --- a/tools/cfl/internal/present/config.go +++ b/tools/cfl/internal/present/config.go @@ -12,9 +12,20 @@ import ( type ConfigPresenter struct{} +func (ConfigPresenter) PresentTestProgress() *sharedpresent.OutputModel { + return &sharedpresent.OutputModel{Sections: []sharedpresent.Section{ + &sharedpresent.MessageSection{ + Kind: sharedpresent.MessageInfo, + Message: "Testing connection... ", + Stream: sharedpresent.StreamStderr, + NoNewline: true, + }, + }} +} + func (ConfigPresenter) PresentTestFailure() *sharedpresent.OutputModel { return stderrOnly(strings.Join([]string{ - "Testing connection... failed!", + "failed!", "", "Troubleshooting:", " - Verify your URL is correct (should include https://)", @@ -28,7 +39,7 @@ func (ConfigPresenter) PresentTestFailure() *sharedpresent.OutputModel { } func (ConfigPresenter) PresentTestSuccess(user *api.User) *sharedpresent.OutputModel { - lines := []string{"Testing connection... success!", ""} + lines := []string{"success!", ""} if user == nil { lines = append(lines, "Your cfl configuration is working correctly.") return stderrOnly(strings.Join(lines, "\n")) diff --git a/tools/cfl/internal/present/config_test.go b/tools/cfl/internal/present/config_test.go index 97dab5f5..2b5754ec 100644 --- a/tools/cfl/internal/present/config_test.go +++ b/tools/cfl/internal/present/config_test.go @@ -14,6 +14,12 @@ import ( func TestConfigPresenter_PresentTestSuccess(t *testing.T) { t.Parallel() + progressModel := ConfigPresenter{}.PresentTestProgress() + progressMsg := requireMessageSection(t, progressModel, 0) + testutil.Equal(t, sharedpresent.StreamStderr, progressMsg.Stream) + testutil.Equal(t, true, progressMsg.NoNewline) + testutil.Equal(t, "Testing connection... ", progressMsg.Message) + model := ConfigPresenter{}.PresentTestSuccess(&api.User{ AccountID: "acct-1", DisplayName: "Test User", @@ -22,7 +28,7 @@ func TestConfigPresenter_PresentTestSuccess(t *testing.T) { msg := requireMessageSection(t, model, 0) testutil.Equal(t, sharedpresent.StreamStderr, msg.Stream) - testutil.Equal(t, "Testing connection... success!\n\nAuthentication successful\nAPI access verified\n\nAuthenticated as: Test User (test@example.com)\nAccount ID: acct-1", msg.Message) + testutil.Equal(t, "success!\n\nAuthentication successful\nAPI access verified\n\nAuthenticated as: Test User (test@example.com)\nAccount ID: acct-1", msg.Message) } func TestConfigPresenter_PresentTestSuccessFallback(t *testing.T) { @@ -32,7 +38,7 @@ func TestConfigPresenter_PresentTestSuccessFallback(t *testing.T) { msg := requireMessageSection(t, model, 0) testutil.Equal(t, sharedpresent.StreamStderr, msg.Stream) - testutil.Equal(t, "Testing connection... success!\n\nYour cfl configuration is working correctly.", msg.Message) + testutil.Equal(t, "success!\n\nYour cfl configuration is working correctly.", msg.Message) } func TestConfigPresenter_PresentTestFailure(t *testing.T) { @@ -42,7 +48,7 @@ func TestConfigPresenter_PresentTestFailure(t *testing.T) { msg := requireMessageSection(t, model, 0) testutil.Equal(t, sharedpresent.StreamStderr, msg.Stream) - testutil.Equal(t, "Testing connection... failed!\n\nTroubleshooting:\n - Verify your URL is correct (should include https://)\n - Check your email and API token\n - Ensure your API token hasn't expired\n - Verify you have permission to access Confluence\n\nTo regenerate an API token:\n https://id.atlassian.com/manage-profile/security/api-tokens", msg.Message) + testutil.Equal(t, "failed!\n\nTroubleshooting:\n - Verify your URL is correct (should include https://)\n - Check your email and API token\n - Ensure your API token hasn't expired\n - Verify you have permission to access Confluence\n\nTo regenerate an API token:\n https://id.atlassian.com/manage-profile/security/api-tokens", msg.Message) } func TestConfigPresenter_PresentClearDefaultPlan(t *testing.T) { From 6cc4838e7f087954d2c39305e7ec774fdc344260 Mon Sep 17 00:00:00 2001 From: Rian Stockbower Date: Wed, 24 Jun 2026 17:49:23 -0400 Subject: [PATCH 3/4] Split config test connection success output --- tools/cfl/internal/cmd/configcmd/test.go | 3 +++ tools/cfl/internal/present/config.go | 10 ++++++---- tools/cfl/internal/present/config_test.go | 9 +++++++-- 3 files changed, 16 insertions(+), 6 deletions(-) diff --git a/tools/cfl/internal/cmd/configcmd/test.go b/tools/cfl/internal/cmd/configcmd/test.go index 1e3450de..d3163152 100644 --- a/tools/cfl/internal/cmd/configcmd/test.go +++ b/tools/cfl/internal/cmd/configcmd/test.go @@ -43,6 +43,9 @@ func runTest(ctx context.Context, opts *root.Options) error { _ = cflpresent.Emit(opts, cflpresent.ConfigPresenter{}.PresentTestFailure()) return fmt.Errorf("connection test failed: %w", err) } + if err := cflpresent.Emit(opts, cflpresent.ConfigPresenter{}.PresentTestConnectionSuccess()); err != nil { + return err + } // Get current user details user, err := client.GetCurrentUser(ctx) diff --git a/tools/cfl/internal/present/config.go b/tools/cfl/internal/present/config.go index 82711245..bd49466f 100644 --- a/tools/cfl/internal/present/config.go +++ b/tools/cfl/internal/present/config.go @@ -38,14 +38,16 @@ func (ConfigPresenter) PresentTestFailure() *sharedpresent.OutputModel { }, "\n")) } +func (ConfigPresenter) PresentTestConnectionSuccess() *sharedpresent.OutputModel { + return stderrOnly("success!\n") +} + func (ConfigPresenter) PresentTestSuccess(user *api.User) *sharedpresent.OutputModel { - lines := []string{"success!", ""} if user == nil { - lines = append(lines, "Your cfl configuration is working correctly.") - return stderrOnly(strings.Join(lines, "\n")) + return stderrOnly("Your cfl configuration is working correctly.") } - lines = append(lines, "Authentication successful", "API access verified", "") + lines := []string{"Authentication successful", "API access verified", ""} displayName := user.DisplayName if displayName == "" { displayName = user.PublicName diff --git a/tools/cfl/internal/present/config_test.go b/tools/cfl/internal/present/config_test.go index 2b5754ec..0ce88ba0 100644 --- a/tools/cfl/internal/present/config_test.go +++ b/tools/cfl/internal/present/config_test.go @@ -20,6 +20,11 @@ func TestConfigPresenter_PresentTestSuccess(t *testing.T) { testutil.Equal(t, true, progressMsg.NoNewline) testutil.Equal(t, "Testing connection... ", progressMsg.Message) + connectionModel := ConfigPresenter{}.PresentTestConnectionSuccess() + connectionMsg := requireMessageSection(t, connectionModel, 0) + testutil.Equal(t, sharedpresent.StreamStderr, connectionMsg.Stream) + testutil.Equal(t, "success!\n", connectionMsg.Message) + model := ConfigPresenter{}.PresentTestSuccess(&api.User{ AccountID: "acct-1", DisplayName: "Test User", @@ -28,7 +33,7 @@ func TestConfigPresenter_PresentTestSuccess(t *testing.T) { msg := requireMessageSection(t, model, 0) testutil.Equal(t, sharedpresent.StreamStderr, msg.Stream) - testutil.Equal(t, "success!\n\nAuthentication successful\nAPI access verified\n\nAuthenticated as: Test User (test@example.com)\nAccount ID: acct-1", msg.Message) + testutil.Equal(t, "Authentication successful\nAPI access verified\n\nAuthenticated as: Test User (test@example.com)\nAccount ID: acct-1", msg.Message) } func TestConfigPresenter_PresentTestSuccessFallback(t *testing.T) { @@ -38,7 +43,7 @@ func TestConfigPresenter_PresentTestSuccessFallback(t *testing.T) { msg := requireMessageSection(t, model, 0) testutil.Equal(t, sharedpresent.StreamStderr, msg.Stream) - testutil.Equal(t, "success!\n\nYour cfl configuration is working correctly.", msg.Message) + testutil.Equal(t, "Your cfl configuration is working correctly.", msg.Message) } func TestConfigPresenter_PresentTestFailure(t *testing.T) { From 9463246b8da2fd1622fb6375606249e02643d2d0 Mon Sep 17 00:00:00 2001 From: Rian Stockbower Date: Wed, 24 Jun 2026 17:54:49 -0400 Subject: [PATCH 4/4] Cover config clear advisory edge cases --- .../271-436-cfl-diagnostics-advisories.md | 13 +++- tools/cfl/internal/cmd/configcmd/clear.go | 9 ++- .../cfl/internal/cmd/configcmd/clear_test.go | 65 +++++++++++++++++++ 3 files changed, 82 insertions(+), 5 deletions(-) diff --git a/docs/proofs/271-436-cfl-diagnostics-advisories.md b/docs/proofs/271-436-cfl-diagnostics-advisories.md index ae999cbe..e9d69b09 100644 --- a/docs/proofs/271-436-cfl-diagnostics-advisories.md +++ b/docs/proofs/271-436-cfl-diagnostics-advisories.md @@ -33,7 +33,7 @@ rtk go test ./tools/cfl/internal/cmd/configcmd ./tools/cfl/internal/present Result: ```text -Go test: 54 passed in 2 packages +Go test: 57 passed in 2 packages ``` Executed: @@ -45,7 +45,7 @@ rtk go test ./tools/cfl/internal/present ./tools/cfl/internal/cmd/configcmd ./to Result: ```text -Go test: 284 passed in 5 packages +Go test: 287 passed in 5 packages ``` Executed: @@ -69,7 +69,7 @@ rtk go test ./tools/cfl/... ./shared/... Result: ```text -Go test: 1634 passed in 34 packages +Go test: 1637 passed in 34 packages ``` Executed: @@ -142,11 +142,18 @@ Command tests now assert exact stdout/stderr for: - `config clear` no-op with an environment override note, without exposing the token value - `config clear --force` default deletion +- `config clear --force` default deletion with an environment override note, + without exposing the token value - interactive `config clear` confirm/cancel prompt behavior - `config clear --non-interactive` without `--force`, proving no preview or warning leaks before `prompt.ErrConfirmationRequired` - `config clear --non-interactive --force` - `config clear --all` +- `config clear --all` when keyring planning fails but plaintext cleanup can + still proceed +- an executable source gate that permits only the prompt `fmt.Fprint` exception + and rejects migrated diagnostic/status strings in `configcmd/test.go` and + `configcmd/clear.go` ## Live CLI Transcript diff --git a/tools/cfl/internal/cmd/configcmd/clear.go b/tools/cfl/internal/cmd/configcmd/clear.go index b38dacb0..7832f45e 100644 --- a/tools/cfl/internal/cmd/configcmd/clear.go +++ b/tools/cfl/internal/cmd/configcmd/clear.go @@ -22,6 +22,11 @@ type clearOptions struct { stdin io.Reader // For testing } +var ( + planClear = keyring.PlanClear + clearAll = keyring.ClearAll +) + func newClearCmd(opts *root.Options) *cobra.Command { clearOpts := &clearOptions{ Options: opts, @@ -74,7 +79,7 @@ func runClear(opts *clearOptions) error { // store the delete/clear step reuses (no second passphrase prompt). // The env + plaintext-file fields are populated even when the keyring // cannot be opened, so `--all` can still clean plaintext artifacts. - plan, store, err := keyring.PlanClear(credstore.ToolCFL, opts.all) + plan, store, err := planClear(credstore.ToolCFL, opts.all) if store != nil { defer func() { _ = store.Close() }() } @@ -98,7 +103,7 @@ func runClear(opts *clearOptions) error { if !ok { return cflpresent.Emit(opts.Options, cflpresent.ConfigPresenter{}.PresentClearCancelled()) } - cleared, aerr := keyring.ClearAll(store) + cleared, aerr := clearAll(store) if aerr != nil { return aerr } diff --git a/tools/cfl/internal/cmd/configcmd/clear_test.go b/tools/cfl/internal/cmd/configcmd/clear_test.go index 6143cf61..fbb2cd5b 100644 --- a/tools/cfl/internal/cmd/configcmd/clear_test.go +++ b/tools/cfl/internal/cmd/configcmd/clear_test.go @@ -62,6 +62,19 @@ func TestRunClear_DeletesSharedKey_WithForce(t *testing.T) { testutil.Equal(t, fmt.Sprintf("This will delete key %q from keyring %s.\nWarning: this is the SHARED token (api_token). jtk will also lose access (cfl and jtk resolve the same key).\nRemoved key %q from keyring %s.\n", keyring.KeyAPIToken, keyring.Ref, keyring.KeyAPIToken, keyring.Ref), errBuf.String()) } +func TestRunClear_DeletesSharedKey_WithForceAndEnvOverrideNote(t *testing.T) { + credtest.Hermetic(t) + t.Setenv("CFL_API_TOKEN", "env-token") + credtest.SeedToken(t, "shared-secret") + + opts, _, errBuf := newClearOpts(true, "") + testutil.RequireNoError(t, runClear(opts)) + + testutil.False(t, tokenPresent(t, keyring.KeyAPIToken)) + testutil.Equal(t, fmt.Sprintf("This will delete key %q from keyring %s.\nWarning: this is the SHARED token (api_token). jtk will also lose access (cfl and jtk resolve the same key).\nRemoved key %q from keyring %s.\nNote: CFL_API_TOKEN still set in the environment and will continue to override at runtime (not cleared).\n", keyring.KeyAPIToken, keyring.Ref, keyring.KeyAPIToken, keyring.Ref), errBuf.String()) + testutil.NotContains(t, errBuf.String(), "env-token") +} + func TestRunClear_DeletesSharedKey_Confirmed(t *testing.T) { credtest.Hermetic(t) credtest.SeedToken(t, "shared-secret") @@ -151,3 +164,55 @@ func TestRunClear_All(t *testing.T) { testutil.True(t, os.IsNotExist(statErr)) testutil.Equal(t, fmt.Sprintf("This will remove the ENTIRE shared keyring bundle %s (keys: %s).\nIt will also delete the shared config file: %s\nRemoved the shared keyring bundle and config file.\n", keyring.Ref, keyring.KeyAPIToken, sharedPath), opts.Stderr.(*bytes.Buffer).String()) } + +func TestRunClear_All_KeyringUnavailableStillReportsPlanAndCleansPlaintext(t *testing.T) { + credtest.Hermetic(t) + sharedPath := credtest.SharedConfigPath(t) + testutil.RequireNoError(t, os.WriteFile(sharedPath, []byte("default:\n url: https://x\n"), 0o600)) + origPlanClear := planClear + planClear = func(string, bool) (keyring.ClearPlan, *keyring.Store, error) { + return keyring.ClearPlan{Ref: keyring.Ref, SharedConfigPath: sharedPath}, nil, errors.New("locked") + } + t.Cleanup(func() { planClear = origPlanClear }) + + opts, _, errBuf := newClearOpts(true, "") + opts.all = true + err := runClear(opts) + testutil.RequireError(t, err) + testutil.Contains(t, err.Error(), "plaintext artifacts were cleaned") + testutil.Contains(t, err.Error(), "keyring bundle") + _, statErr := os.Stat(sharedPath) + testutil.True(t, os.IsNotExist(statErr)) + testutil.Contains(t, errBuf.String(), fmt.Sprintf("This will remove the ENTIRE shared keyring bundle %s.\n", keyring.Ref)) + testutil.Contains(t, errBuf.String(), fmt.Sprintf("It will also delete the shared config file: %s\n", sharedPath)) + testutil.Contains(t, errBuf.String(), "Note: the keyring could not be opened") + testutil.Contains(t, errBuf.String(), "plaintext artifacts will still be cleaned") +} + +func TestConfigDiagnosticsPresenterBoundaryGrepGate(t *testing.T) { + t.Parallel() + + testSource, err := os.ReadFile("test.go") //nolint:gosec // test reads package source. + testutil.RequireNoError(t, err) + clearSource, err := os.ReadFile("clear.go") //nolint:gosec // test reads package source. + testutil.RequireNoError(t, err) + + combined := string(testSource) + "\n" + string(clearSource) + for _, phrase := range []string{ + "Testing connection", + "Troubleshooting", + "Authenticated as", + "No stored API token", + "Cancelled. Nothing was cleared", + "Removed key", + "Removed the shared keyring bundle", + "Warning: this is the SHARED token", + "Note: %s still set in the environment", + } { + testutil.NotContains(t, combined, phrase) + } + + testutil.Equal(t, 1, strings.Count(string(clearSource), "fmt.Fprint(")) + testutil.Contains(t, string(clearSource), `fmt.Fprint(opts.Stderr, promptText+" [y/N]: ")`) + testutil.NotContains(t, string(testSource), "fmt.Fprint") +}