Fix markdown rendering offset bug in toast messages by osterman · Pull Request #1968 · cloudposse/atmos

osterman · 2026-01-15T00:57:31Z

what

Added trimLeftSpaces() function to properly handle leading space trimming when Glamour wraps spaces in ANSI escape codes
Updated toastMarkdown() and renderInlineMarkdownWithBase() to use ANSI-aware trimming
Multi-line toast messages now have consistent bullet point indentation with color enabled

why

Glamour wraps spaces in ANSI codes when color is enabled. The previous code used strings.TrimLeft() which isn't ANSI-aware and failed to remove leading spaces, causing inconsistent indentation in multi-line markdown output.

references

Fixes: Markdown rendering with inconsistent bullet list indentation in terminal UI messages

Summary by CodeRabbit

Bug Fixes
- Improved trimming of leading/trailing spaces in styled/colored text so notifications, hints and inline markdown keep visual formatting intact.
User Experience
- Backend provisioning now collects and shows backend warnings after the progress spinner completes to avoid interleaved output.
Tests
- Added comprehensive tests covering whitespace trimming with styled/ANSI content.

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

Use ANSI-aware trimLeftSpaces() instead of strings.TrimLeft() to properly handle leading spaces in Glamour-rendered output where spaces are wrapped in ANSI escape codes. Fixes inconsistent bullet point indentation in multi-line toast messages with color enabled. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>

github-actions · 2026-01-15T00:58:39Z

Dependency Review

✅ No vulnerabilities or license issues found.

Scanned Files

None

coderabbitai · 2026-01-15T01:14:00Z

📝 Walkthrough

Walkthrough

Adds ANSI-aware leading-space trimming to UI markdown rendering and updates the provisioner backend API to return provisioning warnings (ProvisionResult). Tests and call sites adjusted to propagate and display warnings after UI spinner completes.

Changes

Cohort / File(s)	Summary
ANSI-aware trimming (UI) `pkg/ui/formatter.go`	Adds `trimLeftSpaces()` and ANSI helper routines (`isANSIStart`, `skipANSISequence`, `isANSITerminator`, `copyContentAndANSI`, `processTrailingANSICodes`, `shouldIncludeTrailingANSI`) and replaces plain left-trim calls in `toastMarkdown` and `renderInlineMarkdownWithBase` to preserve ANSI escape sequences when trimming.
UI tests `pkg/ui/formatter_test.go`	Adds `TestTrimLeftSpaces()` with many cases (plain, ANSI-colored, Glamour patterns, Unicode, mixed sequences). Verifies visual widths and absence of leading whitespace after trimming.
Provisioner API & plumbing `pkg/provisioner/backend/backend.go`, `pkg/provisioner/backend_hook.go`, `pkg/provisioner/provisioner.go`, `pkg/provisioner/backend/s3.go`, ...`pkg/provisioner/*`	Introduces `type ProvisionResult struct { Warnings []string }`. Changes backend creation and provisioner function signatures to return `(*ProvisionResult, error)` instead of `error`. Propagates warnings from backends (e.g., S3) through call sites, defers UI warning output until after spinner completion, and updates related logic and error handling.
Provisioner tests & mocks `pkg/provisioner/backend/backend_test.go`, `pkg/provisioner/provisioner_test.go`, `pkg/provisioner/backend/s3_test.go`, `pkg/provisioner/backend/s3_e2e_test.go`	Updates mocks and tests to the new `(*ProvisionResult, error)` returns, captures/discards the new result where appropriate, and asserts warnings in applicable tests.

Sequence Diagram(s)

sequenceDiagram
  participant CLI
  participant Spinner
  participant BackendCreate as Backend.CreateFunc
  participant UI

  CLI->>Spinner: start spinner "Provisioning..."
  Spinner->>BackendCreate: invoke create(ctx, config, auth)
  BackendCreate-->>Spinner: (*ProvisionResult{Warnings}, error)
  Spinner->>CLI: stop spinner
  alt error returned
    CLI->>UI: show error
  else success
    CLI->>UI: for each warning in ProvisionResult.Warnings -> ui.Warning(w)
    CLI->>UI: show success
  end

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

feat: Add ui.Toast() pattern for status notifications #1794 — Modifies pkg/ui/formatter.go and markdown trimming/formatting paths, overlapping with the ANSI-aware trimming changes in this PR.

Suggested labels

major

Suggested reviewers

aknysh

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Title check	⚠️ Warning	The PR title focuses on fixing a markdown rendering offset bug in toast messages, but the changeset includes two distinct feature sets: ANSI-aware trimming for markdown rendering AND a backend provisioning result structure for warning collection. The title accurately describes only the markdown trimming work, not the provisioning changes.	The title should reflect both major changes, or consider splitting into separate PRs. For example: 'Fix markdown rendering and defer backend provision warnings' or split the provisioning changes into a separate PR.
Docstring Coverage	⚠️ Warning	Docstring coverage is 29.27% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ 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/md-render-offset-fix

🧹 Recent nitpick comments

pkg/provisioner/provisioner_test.go (1)

211-214: Consistent mock updates throughout.

All mock provisioners follow the same pattern. Consider adding a test case that verifies warnings in ProvisionResult are properly propagated through the system - this would strengthen coverage for the new feature.

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between da2bbe2 and 0141ddd.

📒 Files selected for processing (8)

pkg/provisioner/backend/backend.go
pkg/provisioner/backend/backend_test.go
pkg/provisioner/backend/s3.go
pkg/provisioner/backend/s3_e2e_test.go
pkg/provisioner/backend/s3_test.go
pkg/provisioner/backend_hook.go
pkg/provisioner/provisioner.go
pkg/provisioner/provisioner_test.go

🧰 Additional context used

📓 Path-based instructions (3)

**/*.go

📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)

**/*.go: Use Viper for managing configuration, environment variables, and flags in CLI commands
Use interfaces for external dependencies to facilitate mocking and consider using testify/mock for creating mock implementations
All code must pass golangci-lint checks
Follow Go's error handling idioms: use meaningful error messages, wrap errors with context using fmt.Errorf("context: %w", err), and consider using custom error types for domain-specific errors
Follow standard Go coding style: use gofmt and goimports to format code, prefer short descriptive variable names, use kebab-case for command-line flags, and snake_case for environment variables
Document all exported functions, types, and methods following Go's documentation conventions
Document complex logic with inline comments in Go code
Support configuration via files, environment variables, and flags following the precedence order: flags > environment variables > config file > defaults
Provide clear error messages to users, include troubleshooting hints when appropriate, and log detailed errors for debugging

**/*.go: All comments must end with periods (enforced by godot linter) in Go code
Organize imports into three groups separated by blank lines, sorted alphabetically: Go stdlib, 3rd-party (NOT cloudposse/atmos), then Atmos packages with maintained aliases (cfg, log, u, errUtils)
All errors MUST be wrapped using static errors defined in errors/errors.go - use errors.Join for combining errors, fmt.Errorf with %w for context, and errors.Is() for error checking
Never manually create mocks - use go.uber.org/mock/mockgen with //go:generate directives in Go code
Keep files small and focused - under 600 lines with one cmd/impl per file, co-locate tests, never use //revive:disable:file-length-limit
Use colors from pkg/ui/theme/colors.go for all UI theming in Go code
Code must be compatible with Linux, macOS, and Windows - use SDKs over binaries, use filepath.Join() instead of h...

Files:

pkg/provisioner/provisioner.go
pkg/provisioner/backend/backend_test.go
pkg/provisioner/backend_hook.go
pkg/provisioner/backend/s3_e2e_test.go
pkg/provisioner/backend/s3_test.go
pkg/provisioner/backend/backend.go
pkg/provisioner/provisioner_test.go
pkg/provisioner/backend/s3.go

**/{pkg,internal,cmd}/**/*.go

📄 CodeRabbit inference engine (CLAUDE.md)

Add defer perf.Track(atmosConfig, "pkg.FuncName")() plus blank line to all public functions, using nil if no atmosConfig param - exceptions: trivial getters/setters, command constructors, simple factories, functions delegating to tracked functions

Files:

pkg/provisioner/provisioner.go
pkg/provisioner/backend/backend_test.go
pkg/provisioner/backend_hook.go
pkg/provisioner/backend/s3_e2e_test.go
pkg/provisioner/backend/s3_test.go
pkg/provisioner/backend/backend.go
pkg/provisioner/provisioner_test.go
pkg/provisioner/backend/s3.go

**/*_test.go

📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)

**/*_test.go: Every new feature must include comprehensive unit tests targeting >80% code coverage for all packages
Use table-driven tests for testing multiple scenarios in Go
Include integration tests for command flows and test CLI end-to-end when possible with test fixtures

Prefer unit tests with mocks over integration tests - use interfaces and dependency injection for testability, generate mocks with go.uber.org/mock/mockgen, use table-driven tests, target >80% coverage

Files:

pkg/provisioner/backend/backend_test.go
pkg/provisioner/backend/s3_e2e_test.go
pkg/provisioner/backend/s3_test.go
pkg/provisioner/provisioner_test.go

🧠 Learnings (23)

📓 Common learnings

Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: docs/prd/tool-dependencies-integration.md:58-64
Timestamp: 2025-12-13T06:07:37.766Z
Learning: cloudposse/atmos: For PRD docs (docs/prd/*.md), markdownlint issues like MD040/MD010/MD034 can be handled in a separate documentation cleanup commit and should not block the current PR.

Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1466
File: cmd/markdown/atmos_toolchain_aliases.md:2-4
Timestamp: 2025-09-13T16:39:20.007Z
Learning: In the cloudposse/atmos repository, CLI documentation files in cmd/markdown/ follow a specific format that uses " $ atmos command" (with leading space and dollar sign prompt) in code blocks. This is the established project convention and should not be changed to comply with standard markdownlint rules MD040 and MD014.

Learnt from: Listener430
Repo: cloudposse/atmos PR: 934
File: tests/fixtures/scenarios/docs-generate/README.md.gotmpl:99-118
Timestamp: 2025-01-25T03:51:57.689Z
Learning: For the cloudposse/atmos repository, changes to template contents should be handled in dedicated PRs and are typically considered out of scope for PRs focused on other objectives.

📚 Learning: 2025-09-13T18:06:07.674Z

Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1466
File: toolchain/list.go:39-42
Timestamp: 2025-09-13T18:06:07.674Z
Learning: In the cloudposse/atmos repository, for UI messages in the toolchain package, use utils.PrintfMessageToTUI instead of log.Error or fmt.Fprintln(os.Stderr, ...). Import pkg/utils with alias "u" to follow the established pattern.

Applied to files:

pkg/provisioner/provisioner.go
pkg/provisioner/backend_hook.go
pkg/provisioner/backend/s3.go

📚 Learning: 2025-07-05T20:59:02.914Z

Learnt from: aknysh
Repo: cloudposse/atmos PR: 1363
File: internal/exec/template_utils.go:18-18
Timestamp: 2025-07-05T20:59:02.914Z
Learning: In the Atmos project, gomplate v4 is imported with a blank import (`_ "github.com/hairyhenderson/gomplate/v4"`) alongside v3 imports to resolve AWS SDK version conflicts. V3 uses older AWS SDK versions that conflict with newer AWS modules used by Atmos. A full migration to v4 requires extensive refactoring due to API changes and should be handled in a separate PR.

Applied to files:

pkg/provisioner/provisioner.go
pkg/provisioner/backend_hook.go
pkg/provisioner/backend/s3.go

📚 Learning: 2026-01-04T00:55:21.720Z

Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-04T00:55:21.720Z
Learning: Applies to **/{pkg,internal,cmd}/**/*.go : Add `defer perf.Track(atmosConfig, "pkg.FuncName")()` plus blank line to all public functions, using `nil` if no atmosConfig param - exceptions: trivial getters/setters, command constructors, simple factories, functions delegating to tracked functions

Applied to files:

pkg/provisioner/provisioner.go
pkg/provisioner/backend_hook.go
pkg/provisioner/backend/s3.go

📚 Learning: 2025-12-21T04:10:29.030Z

Learnt from: osterman
Repo: cloudposse/atmos PR: 1891
File: internal/exec/describe_affected.go:468-468
Timestamp: 2025-12-21T04:10:29.030Z
Learning: In Go, package-level declarations (constants, variables, types, and functions) are visible to all files in the same package without imports. During reviews in cloudposse/atmos (and similar Go codebases), before suggesting to declare a new identifier, first check if it already exists in another file of the same package. If it exists, you can avoid adding a new declaration; if not, proceed with a proper package-level declaration.

Applied to files:

pkg/provisioner/provisioner.go
pkg/provisioner/backend/backend_test.go
pkg/provisioner/backend_hook.go
pkg/provisioner/backend/s3_e2e_test.go
pkg/provisioner/backend/s3_test.go
pkg/provisioner/backend/backend.go
pkg/provisioner/provisioner_test.go
pkg/provisioner/backend/s3.go

📚 Learning: 2025-11-09T19:06:58.470Z

Learnt from: osterman
Repo: cloudposse/atmos PR: 1752
File: pkg/profile/list/formatter_table.go:27-29
Timestamp: 2025-11-09T19:06:58.470Z
Learning: In the cloudposse/atmos repository, performance tracking with `defer perf.Track()` is enforced on all functions via linting, including high-frequency utility functions, formatters, and renderers. This is a repository-wide policy to maintain consistency and avoid making case-by-case judgment calls about which functions should have profiling.

Applied to files:

pkg/provisioner/provisioner.go
pkg/provisioner/backend_hook.go
pkg/provisioner/backend/s3.go

📚 Learning: 2026-01-04T00:55:21.720Z

Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-04T00:55:21.720Z
Learning: Applies to **/*.go : Organize imports into three groups separated by blank lines, sorted alphabetically: Go stdlib, 3rd-party (NOT cloudposse/atmos), then Atmos packages with maintained aliases (`cfg`, `log`, `u`, `errUtils`)

Applied to files:

pkg/provisioner/provisioner.go
pkg/provisioner/backend_hook.go
pkg/provisioner/backend/s3.go

📚 Learning: 2025-05-30T03:21:37.197Z

Learnt from: aknysh
Repo: cloudposse/atmos PR: 1274
File: go.mod:63-63
Timestamp: 2025-05-30T03:21:37.197Z
Learning: The redis dependency (github.com/redis/go-redis/v9) in the atmos project is only used in tests, not in production code.

Applied to files:

pkg/provisioner/provisioner.go
pkg/provisioner/backend_hook.go
pkg/provisioner/backend/s3.go

📚 Learning: 2024-12-11T18:40:12.808Z

Learnt from: Listener430
Repo: cloudposse/atmos PR: 844
File: cmd/helmfile.go:37-37
Timestamp: 2024-12-11T18:40:12.808Z
Learning: In the atmos project, `cliConfig` is initialized within the `cmd` package in `root.go` and can be used in other command files.

Applied to files:

pkg/provisioner/provisioner.go
pkg/provisioner/backend_hook.go
pkg/provisioner/backend/s3.go

📚 Learning: 2024-10-23T21:36:40.262Z

Learnt from: osterman
Repo: cloudposse/atmos PR: 740
File: cmd/cmd_utils.go:340-359
Timestamp: 2024-10-23T21:36:40.262Z
Learning: In the Go codebase for Atmos, when reviewing functions like `checkAtmosConfig` in `cmd/cmd_utils.go`, avoid suggesting refactoring to return errors instead of calling `os.Exit` if such changes would significantly increase the scope due to the need to update multiple call sites.

Applied to files:

pkg/provisioner/provisioner.go
pkg/provisioner/backend_hook.go
pkg/provisioner/backend/s3.go

📚 Learning: 2025-11-30T04:16:24.155Z

Learnt from: aknysh
Repo: cloudposse/atmos PR: 1821
File: pkg/merge/deferred.go:34-48
Timestamp: 2025-11-30T04:16:24.155Z
Learning: In the cloudposse/atmos repository, the `defer perf.Track()` guideline applies to functions that perform meaningful work (I/O, computation, external calls), but explicitly excludes trivial accessors/mutators (e.g., simple getters, setters with single integer increments, string joins, or map appends) where the tracking overhead would exceed the actual method cost and provide no actionable performance data. Hot-path methods called in tight loops should especially avoid perf.Track() if they perform only trivial operations.

Applied to files:

pkg/provisioner/provisioner.go
pkg/provisioner/backend_hook.go

📚 Learning: 2026-01-04T00:55:21.720Z

Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-04T00:55:21.720Z
Learning: Applies to **/*_test.go : Prefer unit tests with mocks over integration tests - use interfaces and dependency injection for testability, generate mocks with `go.uber.org/mock/mockgen`, use table-driven tests, target >80% coverage

Applied to files:

pkg/provisioner/backend/backend_test.go
pkg/provisioner/provisioner_test.go

📚 Learning: 2026-01-04T00:55:21.720Z

Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-04T00:55:21.720Z
Learning: Follow multi-provider registry pattern: define interface, implement per provider, register implementations, generate mocks - see `pkg/store/` example

Applied to files:

pkg/provisioner/backend/backend_test.go
pkg/provisioner/provisioner_test.go

📚 Learning: 2025-11-10T23:23:39.771Z

Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: toolchain/registry/aqua/aqua_test.go:417-442
Timestamp: 2025-11-10T23:23:39.771Z
Learning: In Atmos toolchain AquaRegistry, tests should not hit real GitHub. Use the options pattern via WithGitHubBaseURL to inject an httptest server URL and make GetLatestVersion/GetAvailableVersions deterministic.

Applied to files:

pkg/provisioner/backend/backend_test.go
pkg/provisioner/provisioner_test.go

📚 Learning: 2025-11-11T03:47:59.576Z

Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: toolchain/which_test.go:166-223
Timestamp: 2025-11-11T03:47:59.576Z
Learning: In the cloudposse/atmos repo, tests that manipulate environment variables should use testing.T.Setenv for automatic setup/teardown instead of os.Setenv/Unsetenv.

Applied to files:

pkg/provisioner/backend/backend_test.go

📚 Learning: 2025-11-11T03:47:45.878Z

Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: toolchain/add_test.go:67-77
Timestamp: 2025-11-11T03:47:45.878Z
Learning: In the cloudposse/atmos codebase, tests should prefer t.Setenv for environment variable setup/teardown instead of os.Setenv/Unsetenv to ensure test-scoped isolation.

Applied to files:

pkg/provisioner/backend/backend_test.go

📚 Learning: 2025-11-08T19:56:18.660Z

Learnt from: osterman
Repo: cloudposse/atmos PR: 1697
File: internal/exec/oci_utils.go:0-0
Timestamp: 2025-11-08T19:56:18.660Z
Learning: In the Atmos codebase, when a function receives an `*schema.AtmosConfiguration` parameter, it should read configuration values from `atmosConfig.Settings` fields rather than using direct `os.Getenv()` or `viper.GetString()` calls. The Atmos pattern is: viper.BindEnv in cmd/root.go binds environment variables → Viper unmarshals into atmosConfig.Settings via mapstructure → business logic reads from the Settings struct. This provides centralized config management, respects precedence, and enables testability. Example: `atmosConfig.Settings.AtmosGithubToken` instead of `os.Getenv("ATMOS_GITHUB_TOKEN")` in functions like `getGHCRAuth` in internal/exec/oci_utils.go.

Applied to files:

pkg/provisioner/backend/backend_test.go
pkg/provisioner/provisioner_test.go

📚 Learning: 2025-12-13T04:37:25.223Z

Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: cmd/root.go:0-0
Timestamp: 2025-12-13T04:37:25.223Z
Learning: In Atmos cmd/root.go Execute(), after cfg.InitCliConfig, we must call both toolchainCmd.SetAtmosConfig(&atmosConfig) and toolchain.SetAtmosConfig(&atmosConfig) so the CLI wrapper and the toolchain package receive configuration; missing either can cause nil-pointer panics in toolchain path resolution.

Applied to files:

pkg/provisioner/backend/backend_test.go

📚 Learning: 2025-08-16T23:32:40.412Z

Learnt from: aknysh
Repo: cloudposse/atmos PR: 1405
File: internal/exec/describe_dependents_test.go:455-456
Timestamp: 2025-08-16T23:32:40.412Z
Learning: In the cloudposse/atmos Go codebase, `InitCliConfig` returns a `schema.AtmosConfiguration` value (not a pointer), while `ExecuteDescribeDependents` expects a `*schema.AtmosConfiguration` pointer parameter. Therefore, when passing the result of `InitCliConfig` to `ExecuteDescribeDependents`, use `&atmosConfig` to pass the address of the value.

Applied to files:

pkg/provisioner/backend/backend_test.go

📚 Learning: 2025-01-16T11:41:35.531Z

Learnt from: osterman
Repo: cloudposse/atmos PR: 942
File: internal/exec/describe_affected_utils.go:802-807
Timestamp: 2025-01-16T11:41:35.531Z
Learning: When checking if a component is enabled in Atmos, use standardized helper function that includes logging. The function should check the `enabled` field in the component's metadata section and log a trace message when skipping disabled components.

Applied to files:

pkg/provisioner/backend/backend_test.go

📚 Learning: 2025-08-16T23:33:07.477Z

Learnt from: aknysh
Repo: cloudposse/atmos PR: 1405
File: internal/exec/describe_dependents_test.go:651-652
Timestamp: 2025-08-16T23:33:07.477Z
Learning: In the cloudposse/atmos Go codebase, ExecuteDescribeDependents expects a pointer to AtmosConfiguration (*schema.AtmosConfiguration), so when calling it with a value returned by cfg.InitCliConfig (which returns schema.AtmosConfiguration), the address-of operator (&) is necessary: ExecuteDescribeDependents(&atmosConfig, ...).

Applied to files:

pkg/provisioner/backend/backend_test.go

📚 Learning: 2025-11-24T17:35:37.209Z

Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to **/*.go : Use interfaces for external dependencies to facilitate mocking and consider using testify/mock for creating mock implementations

Applied to files:

pkg/provisioner/provisioner_test.go

📚 Learning: 2025-09-10T21:17:55.273Z

Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1466
File: toolchain/http_client_test.go:3-10
Timestamp: 2025-09-10T21:17:55.273Z
Learning: In the cloudposse/atmos repository, imports should never be changed as per samtholiya's coding guidelines.

Applied to files:

pkg/provisioner/backend/s3.go

🧬 Code graph analysis (6)

pkg/provisioner/provisioner.go (3)

pkg/provisioner/backend/backend.go (2)

ProvisionResult (14-16)

ProvisionBackend (199-251)

pkg/ui/spinner/spinner.go (1)

ExecWithSpinner (22-64)

pkg/ui/formatter.go (1)

Warning (248-255)

pkg/provisioner/backend/backend_test.go (1)

pkg/provisioner/backend/backend.go (2)

ProvisionResult (14-16)

ProvisionBackend (199-251)

pkg/provisioner/backend_hook.go (3)

pkg/provisioner/backend/backend.go (1)

ProvisionResult (14-16)

pkg/ui/spinner/spinner.go (1)

ExecWithSpinner (22-64)

pkg/ui/formatter.go (1)

Warning (248-255)

pkg/provisioner/backend/s3_e2e_test.go (1)

pkg/provisioner/backend/s3.go (1)

CreateS3Backend (141-185)

pkg/provisioner/backend/backend.go (3)

pkg/schema/schema.go (1)

AtmosConfiguration (54-102)

errors/builder.go (1)

Build (24-37)

errors/errors.go (4)

ErrProvisioningNotConfigured (707-707)

ErrBackendNotFound (706-706)

ErrBackendTypeRequired (161-161)

ErrCreateNotImplemented (708-708)

pkg/provisioner/provisioner_test.go (2)

pkg/schema/schema.go (3)

Context (575-590)

AtmosConfiguration (54-102)

AuthContext (665-675)

pkg/provisioner/backend/backend.go (1)

ProvisionResult (14-16)

⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)

GitHub Check: Acceptance Tests (linux)
GitHub Check: Acceptance Tests (macos)
GitHub Check: Acceptance Tests (windows)
GitHub Check: Summary

🔇 Additional comments (30)

pkg/provisioner/backend/backend.go (3)

13-16: Clean type definition for propagating warnings.

The ProvisionResult struct is well-documented and serves its purpose clearly. The comment explains when warnings should be displayed.

18-24: Signature update is consistent with the new return model.

The BackendCreateFunc signature change to return (*ProvisionResult, error) aligns with the broader PR objective of propagating warnings post-spinner.

197-250: ProvisionBackend changes look solid.

All error paths correctly return (nil, error). The success path delegates to createFunc and returns its result directly. Error wrapping follows the repo pattern with errUtils.Build() and fmt.Errorf.

pkg/provisioner/backend/s3_e2e_test.go (2)

87-88: Correctly discards the result when only error is relevant.

The test validates bucket creation succeeded; warnings aren't the focus here.

141-142: Same pattern, same approval.

Properly adapted to the new signature.

pkg/provisioner/provisioner.go (2)

14-14: Import added for UI warning functionality.

Correctly placed in the Atmos packages group.

90-118: Well-structured post-spinner warning handling.

The comments clearly explain the rationale for deferring warnings. The flow is correct:

Capture result/error inside spinner callback

Return error immediately if provisioning fails

Display warnings only after spinner completes

Discarding the ui.Warning error with _ follows existing patterns in the codebase.

pkg/provisioner/backend/s3_test.go (6)

708-715: Good test coverage for new bucket scenario.

Asserts no warnings are returned when creating a new bucket, which is the expected behavior.

741-746: Validates warning generation for existing buckets.

Tests both the count (1 warning) and content (contains expected message). Solid coverage.

756-759: Error path correctly discards warnings.

When testing error scenarios, warnings aren't the focus.

772-775: Consistent pattern for encryption failure test.

791-794: Consistent pattern for public access failure test.

813-816: Consistent pattern for tagging failure test.

pkg/provisioner/backend/backend_test.go (11)

27-29: Mock updated to new signature.

Returns &ProvisionResult{} for success - correct pattern.

49-55: Multiple mock provisioners follow consistent pattern.

Both s3 and gcs mocks return the expected type.

112-114: ResetRegistryForTesting test mock updated correctly.

138-140: ClearsAllEntries test mock follows same pattern.

183-186: ProvisionBackend call sites correctly discard result.

Tests focus on error behavior, so discarding the result is appropriate.

335-340: Success test captures relevant state.

Mock stores captured values for assertion, returns success result.

375-378: AuthContext test mock correctly implemented.

417-419: Failure test returns nil result with error.

Correct pattern for error scenarios.

452-460: Multiple backend types test has both mocks correctly updated.

518-523: Concurrent test mock is thread-safe.

The mutex protects the counter, and the mock returns correctly.

606-609: Table-driven test mock follows established pattern.

pkg/provisioner/backend_hook.go (1)

84-108: LGTM! Clean separation of concerns.

Good approach capturing ProvisionResult in the closure and displaying warnings post-spinner. The nil check on result before iterating is correct, though ranging over a nil Warnings slice would also be safe. The comments explaining the spinner concurrency rationale are helpful.

pkg/provisioner/provisioner_test.go (2)

92-104: Mocks updated correctly.

The signature change to (*backend.ProvisionResult, error) is properly reflected. Returning &backend.ProvisionResult{} on success aligns with the new contract.

146-148: Failure mock looks good.

Returning (nil, error) on failure is the right pattern.

pkg/provisioner/backend/s3.go (3)

141-185: Clean API change.

The signature change to return (*ProvisionResult, error) is well-executed. Error paths correctly return (nil, err) and the success path properly wraps warnings. The rich error builder on lines 158-164 provides good context.

371-415: Solid refactor removing UI side effects.

Moving warning collection out of this function improves testability and aligns with the post-spinner display strategy. The warning message clearly lists what modifications will be applied to existing buckets.

177-179: Helpful comment.

The inline explanation about spinner concurrency is valuable for future maintainers.

_{✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.}

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

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

codecov · 2026-01-15T01:14:43Z

Codecov Report

❌ Patch coverage is 72.22222% with 20 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.51%. Comparing base (c61842e) to head (0141ddd).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
pkg/provisioner/backend_hook.go	0.00%	11 Missing ⚠️
pkg/provisioner/backend/s3.go	70.58%	4 Missing and 1 partial ⚠️
pkg/provisioner/provisioner.go	81.81%	1 Missing and 1 partial ⚠️
pkg/ui/formatter.go	92.30%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1968      +/-   ##
==========================================
- Coverage   74.51%   74.51%   -0.01%     
==========================================
  Files         777      777              
  Lines       71003    71044      +41     
==========================================
+ Hits        52910    52939      +29     
- Misses      14626    14642      +16     
+ Partials     3467     3463       -4

Flag	Coverage Δ
unittests	`74.51% <72.22%> (-0.01%)`	⬇️

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

Files with missing lines	Coverage Δ
pkg/provisioner/backend/backend.go	`100.00% <100.00%> (ø)`
pkg/provisioner/provisioner.go	`96.00% <81.81%> (-1.81%)`	⬇️
pkg/ui/formatter.go	`85.23% <92.30%> (+0.25%)`	⬆️
pkg/provisioner/backend/s3.go	`87.55% <70.58%> (+0.11%)`	⬆️
pkg/provisioner/backend_hook.go	`49.23% <0.00%> (-7.92%)`	⬇️

... and 5 files with indirect coverage changes

🚀 New features to boost your workflow:

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

…nt output - Add ProvisionResult type to collect warnings from backend provisioning - Update BackendCreateFunc to return (*ProvisionResult, error) - Update applyS3BucketDefaults to return warnings instead of printing directly - Display warnings AFTER spinner completes to avoid text corruption - Add tests verifying warning collection for existing buckets This fixes the concurrent output issue where ui.Warning() was called inside the spinner callback, causing text interleaving and display corruption (e.g., bucket name + component name merged together). Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

github-actions · 2026-01-15T21:38:15Z

These changes were released in v1.204.0.

osterman requested a review from a team as a code owner January 15, 2026 00:57

github-actions bot added the size/m Medium size PR label Jan 15, 2026

coderabbitai bot previously approved these changes Jan 15, 2026

View reviewed changes

osterman dismissed coderabbitai[bot]’s stale review via 0141ddd January 15, 2026 18:00

coderabbitai bot approved these changes Jan 15, 2026

View reviewed changes

osterman added the patch A minor, backward compatible change label Jan 15, 2026

aknysh approved these changes Jan 15, 2026

View reviewed changes

aknysh merged commit 806f2e1 into main Jan 15, 2026
63 of 64 checks passed

aknysh deleted the osterman/md-render-offset-fix branch January 15, 2026 19:31

coderabbitai bot mentioned this pull request Jan 17, 2026

refactor: Remove error returns from ui.* functions #1980

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix markdown rendering offset bug in toast messages#1968

Fix markdown rendering offset bug in toast messages#1968
aknysh merged 2 commits intomainfrom
osterman/md-render-offset-fix

osterman commented Jan 15, 2026 •

edited by coderabbitai bot

Loading

Uh oh!

github-actions bot commented Jan 15, 2026 •

edited

Loading

Uh oh!

coderabbitai bot commented Jan 15, 2026 •

edited

Loading

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Possibly related PRs

Suggested labels

Suggested reviewers

Uh oh!

codecov bot commented Jan 15, 2026 •

edited

Loading

Uh oh!

Uh oh!

github-actions bot commented Jan 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

osterman commented Jan 15, 2026 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

what

why

references

Summary by CodeRabbit

Uh oh!

github-actions bot commented Jan 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Dependency Review

Scanned Files

Uh oh!

coderabbitai bot commented Jan 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Possibly related PRs

Suggested labels

Suggested reviewers

Uh oh!

codecov bot commented Jan 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

github-actions bot commented Jan 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

osterman commented Jan 15, 2026 •

edited by coderabbitai bot

Loading

github-actions bot commented Jan 15, 2026 •

edited

Loading

coderabbitai bot commented Jan 15, 2026 •

edited

Loading

codecov bot commented Jan 15, 2026 •

edited

Loading