fix: Fix log level default and convert log.Info to ui.*/data.*

[osterman]

what

• Fixed log level default regression from "Warning" back to "Info" in pkg/flags/global_registry.go
• Converted result-oriented log.Info() calls to ui.Success() for user-visible outcomes
• Converted data output log.Info() calls to data.*() for pipeable output

why

• Regression: PR feat: Add core unified flag parsing infrastructure #1762 introduced pkg/flags/global_registry.go with Default: "Info" which conflicted with pkg/flags/global/flags.go:NewFlags() that correctly had LogsLevel: "Warning". The registry default takes precedence, causing verbose logging.
• UI Separation: User-facing result messages (auth success, logout complete, validation success) should use ui.Success() so they always display regardless of log level. Informational messages (starting, executing) correctly remain as log.Info().

references

• Regression introduced in: feat: Add core unified flag parsing infrastructure #1762 - "feat: Add core unified flag parsing infrastructure"

Summary by CodeRabbit

• New Features

  • Clearer user-facing success messages for authentication and logout steps.

• Improvements

  • Default log level lowered to Warning to reduce noise.
  • Command and validation outputs now routed through UI/data channels for direct user feedback.
  • Several informational logs converted to debug to declutter console output.

• Tests

  • Added tests covering subcommand display and dry-run validation behavior.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[github-actions]

Dependency Review

✅ No vulnerabilities or license issues found.

Scanned Files

None

[coderabbitai]

📝 Walkthrough

Walkthrough

Redirects user-facing messages from logger to UI/data channels and reduces log verbosity by changing several log.Info calls to data.Write/data.Writeln or ui.Successf, and changes default log level from Info to Warning. No control-flow or public API signature changes.

Changes

Cohort / File(s)	Summary
Output → data/ui cmd/cmd_utils.go, cmd/validate_editorconfig.go, internal/exec/atmos.go, pkg/auth/manager_logout.go, pkg/auth/providers/aws/saml.go, pkg/auth/providers/github/oidc.go	Replaced log.Info usages with data.Write/data.Writeln or ui.Successf; added corresponding imports; some log.Info calls downgraded to log.Debug. No control-flow or signature changes.
Logging defaults pkg/flags/global_registry.go	Changed default logs-level from "Info" to "Warning".
Tests / I/O wiring cmd/cmd_utils_test.go, cmd/validate_editorconfig_test.go	Added tests that initialize data writer and I/O context to cover dry-run and subcommand-display paths; new test-only imports for data, iolib, os, path/filepath, and config as needed.

Sequence Diagram(s)

(omitted — changes adjust output channels and logging defaults without introducing a new multi-component sequential flow)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• Logs validation on root level #946 — related changes to logging configuration and log-level handling.

Suggested reviewers

• aknysh

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main changes: fixing the log level default and converting log.Info calls to ui/data functions across multiple files.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch osterman/log-level-default

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Caution

Review failed

The head commit changed during the review from b0a9e1f to 0019677.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch osterman/log-level-default

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[codecov]

Codecov Report

❌ Patch coverage is 83.33333% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.43%. Comparing base (acf68be) to head (3c36d11).
⚠️ Report is 7 commits behind head on main.

Files with missing lines	Patch %	Lines
internal/exec/atmos.go	0.00%	1 Missing ⚠️
pkg/auth/providers/aws/saml.go	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1959      +/-   ##
==========================================
+ Coverage   75.37%   75.43%   +0.06%     
==========================================
  Files         792      792              
  Lines       73435    73516      +81     
==========================================
+ Hits        55350    55460     +110     
+ Misses      14574    14548      -26     
+ Partials     3511     3508       -3     

Flag	Coverage Δ
unittests	75.43% <83.33%> (+0.06%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
cmd/cmd_utils.go	56.80% <100.00%> (+1.04%)	⬆️
cmd/validate_editorconfig.go	41.37% <100.00%> (+11.20%)	⬆️
pkg/auth/manager_logout.go	88.28% <100.00%> (+0.28%)	⬆️
pkg/auth/providers/github/oidc.go	67.27% <100.00%> (ø)
pkg/flags/global_registry.go	100.00% <100.00%> (ø)
internal/exec/atmos.go	20.58% <0.00%> (ø)
pkg/auth/providers/aws/saml.go	79.80% <0.00%> (ø)

... and 11 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@pkg/auth/manager_logout.go`:
- Around line 56-58: The code currently calls ui.Successf("Logout completed for
identity %s", identityName) unconditionally even when the function encountered
failures (errs non-empty); update the logout flow in pkg/auth/manager_logout.go
(the block using log.Debug, ui.Successf, vars errs, deleteKeychain,
identityName) so that ui.Successf is only invoked when there are no errors
(e.g., len(errs) == 0) and any other success flags (like deleteKeychain)
indicate true success; apply the same conditional change to the other two
occurrences around the blocks identified (the ones at the regions corresponding
to lines 172-174 and 231-233) so success is emitted only on true success and not
when errors are returned.

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@cmd/cmd_utils_test.go`:
- Around line 1773-1819: The test only asserts the exit code but doesn't verify
that preCustomCommand actually printed the subcommand names; also it must use
NewTestKit(t) to manage RootCmd state. Update
TestPreCustomCommand_ShowsSubCommands to call NewTestKit(t) at the start (use
the kit's I/O context instead of manually creating iolib.NewContext), initialize
the data writer via data.InitWriter(kit.IOCtx), run preCustomCommand as before,
then read the captured output from the test I/O buffer (the data writer / kit IO
context) and assert that the output contains "sub1" and "sub2"; keep the
existing OsExit mocking and panic handling around preCustomCommand. Ensure
references: TestPreCustomCommand_ShowsSubCommands, preCustomCommand, data.Write,
NewTestKit.

🧹 Nitpick comments (3)

cmd/cmd_utils_test.go (1)

1810-1818: Add assertion for unexpected non-exit scenario.

If preCustomCommand returns without calling OsExit (a regression), this test would pass silently because no assertions run outside the recover block.

♻️ Add fail-safe assertion

 	defer func() {
 		if r := recover(); r != nil {
 			// Expected: panic from mocked OsExit.
 			assert.True(t, exitCalled, "OsExit should be called")
 			assert.Equal(t, 1, capturedExitCode, "Should exit with code 1")
 		}
 	}()
 
 	preCustomCommand(cmd, []string{}, parentCmd, commandConfig)
+
+	// If we reach here, exit was not called - fail the test.
+	t.Fatal("preCustomCommand should have called OsExit but didn't")
 }

cmd/validate_editorconfig_test.go (2)

80-81: Avoid hardcoded line numbers in comments.

Line references will drift as the target file changes. Consider rephrasing to describe behavior instead of a line number.

♻️ Suggested tweak

-// This covers the data.Writeln(file) call at line 172.
+// This covers the dry-run data output path.

---

82-126: Assert dry-run output, not just “no panic.”

This test currently verifies only that runMainLogic() doesn’t panic; it doesn’t validate the user-visible dry-run output. Consider capturing the data writer output and asserting it includes the discovered file path (e.g., test.txt) to verify behavior. Based on learnings, ...

[mergify]

💥 This pull request now has conflicts. Could you fix it @osterman? 🙏