Skip to content

fix(git): tolerate config errors for CI git-clone bootstrap pre-Cobra - #2879

Open
Erik Osterman (Cloud Posse) (osterman) wants to merge 21 commits into
mainfrom
osterman/test-container-fields-ignored
Open

fix(git): tolerate config errors for CI git-clone bootstrap pre-Cobra#2879
Erik Osterman (Cloud Posse) (osterman) wants to merge 21 commits into
mainfrom
osterman/test-container-fields-ignored

Conversation

@osterman

@osterman Erik Osterman (Cloud Posse) (osterman) commented Aug 5, 2026

Copy link
Copy Markdown
Member

what

  • Fixes atmos git clone failing before it ever attempts a clone in a fresh CI workspace, when a referenced config profile doesn't exist yet (e.g. ATMOS_PROFILE=github with no .atmos/profiles/ checked out) — ATMOS_CI=true had no effect on this failure.
  • Adds a combined regression test case to pkg/container's build-arg builder covering engine, driver, cache, custom dockerfile/context, and tags together in one config (previously only tested individually).

why

  • cmd/root.go's Execute() runs an initial cfg.InitCliConfig before Cobra resolves any subcommand. Only the second InitCliConfig call (inside PersistentPreRun) knew how to tolerate the CI git-clone bootstrap's expected missing config (applyCIGitCloneBootstrap). The first call's error handler had no such tolerance, so a profile not found error aborted the process before Cobra — and therefore before PersistentPreRun — ever ran, regardless of ATMOS_CI.
  • Adds isCIGitCloneBootstrapArgs (an os.Args-based equivalent of the existing Cobra-aware bootstrap check) to the pre-Cobra handler, and a new exported CIGitCloneModeRequestedFromEnv in cmd/git so both code paths defer to the same ATMOS_CI/CI-provider resolution logic.
  • The container test addition closes the one remaining gap in buildBuildArgs coverage: individual fields (driver, cache, tags, custom dockerfile/context) each had their own case, but nothing asserted they all survive together in a single build.

references

  • N/A

Summary by CodeRabbit

  • Bug Fixes

    • CI git clone bootstrap commands now proceed when profiles or configuration files are unavailable.
    • Custom container build settings now load consistently, including caching, tags, Dockerfiles, and contexts.
    • Nested component names are safely converted into valid workdir path segments.
    • Cached Terraform outputs now display fetch success or failure notifications.
  • Documentation

    • Added documentation covering CI bootstrap, custom container configuration, and nested workdir path fixes.
  • Tests

    • Expanded coverage for clone parsing, container builds, configuration decoding, workdir paths, output notifications, and build arguments.

Strengthens pkg/container's pure arg-building test with a case combining
engine, driver, cache, custom dockerfile/context, and tags in a single
config, closing the one remaining gap versus per-field-only coverage.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
atmos git clone in a fresh CI workspace (no atmos.yaml yet, e.g. a profile
referenced by CI config) failed with "profile not found" before ever
attempting the clone, and ATMOS_CI=true had no effect. Execute() runs an
initial cfg.InitCliConfig before Cobra resolves any command; only the
second, PersistentPreRun-scoped InitCliConfig call knew how to tolerate
the CI bootstrap clone's expected missing config (applyCIGitCloneBootstrap),
so the first call's error aborted the process before that check could run.

Add isCIGitCloneBootstrapArgs, an os.Args-based equivalent of the existing
cmd-aware bootstrap check, so the pre-Cobra handler recognizes the same
no-argument `atmos git clone` shape and defers to the same
ATMOS_CI/CI-provider resolution (via the new exported
CIGitCloneModeRequestedFromEnv) before Cobra ever parses the command.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@atmos-pro

atmos-pro Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Tip

Atmos Pro  

No affected stacks workflow was detected for this pull request.
If this is expected, no action is needed.
Learn More. Ask AI.

@osterman Erik Osterman (Cloud Posse) (osterman) added the patch A minor, backward compatible change label Aug 5, 2026
@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

The PR adds pre-Cobra CI clone detection, fixes polymorphic container configuration decoding, sanitizes nested workdir names, standardizes cached Terraform output resolution, and blocks gofmt in Claude permissions.

Changes

CI git clone bootstrap

Layer / File(s) Summary
Raw clone argument parsing
cmd/git/bootstrap.go, cmd/git/bootstrap_test.go
Raw clone arguments use the real Cobra clone flags. Tests cover value forms, positional repositories, --all, CI detection, and malformed values.
Pre-Cobra configuration error handling
cmd/root.go, cmd/root_helpers_test.go, docs/fixes/...
Eligible CI git clone commands tolerate configuration initialization errors. Tests cover accepted and rejected command forms.

Container task configuration

Layer / File(s) Summary
Polymorphic container task decoding
pkg/schema/task.go, pkg/schema/task_test.go, pkg/config/custom_command_container_with_test.go
The mapstructure path decodes with: blocks into typed container fields. Tests compare workflow YAML and custom-command decoding.
Buildx command integration
pkg/container/common_test.go, cmd/custom_command_container_build_test.go, docs/fixes/...
Tests verify Buildx builder, cache, tag, Dockerfile, and context arguments and reject the fallback Docker build.

Workdir path sanitization

Layer / File(s) Summary
Sanitized workdir paths
pkg/provisioner/workdir/types.go, pkg/provisioner/workdir/types_test.go, internal/terraform_backend/..., docs/fixes/...
BuildPath replaces slashes in component names with hyphens. Tests verify nested component paths and JIT state lookup.

Terraform output cache resolution

Layer / File(s) Summary
Cached output resolution
pkg/terraform/output/executor_utils.go, pkg/terraform/output/executor.go, pkg/terraform/output/executor_test.go, pkg/terraform/output/config_test.go
Cached output lookups use a shared resolver and emit visible success or failure notifications. Tests cover cache hits, misses, errors, and stack-path containment.

Repository command permissions

Layer / File(s) Summary
Claude command deny rule
.claude/settings.json
The Claude permission deny list now blocks gofmt commands.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related PRs

Suggested labels: patch

Suggested reviewers: aknysh, goruha, johncblandii

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the primary CI git-clone bootstrap fix implemented by the pull request.
Docstring Coverage ✅ Passed Docstring coverage is 80.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch osterman/test-container-fields-ignored

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.

@github-actions github-actions Bot added the size/m Medium size PR label Aug 5, 2026
@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown

Dependency Review

✅ No vulnerabilities or license issues found.

Scanned Files

  • website/pnpm-lock.yaml

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown

Resource Changes Found for bucket in test

Atmos CI

create

Plan: 4 to add, 0 to change, 0 to destroy.
To reproduce this locally, run:

atmos terraform plan bucket -s test

Create

+ aws_s3_bucket.checkov_target
+ aws_s3_bucket.this
+ aws_s3_bucket.trivy_target
+ aws_s3_bucket_public_access_block.trivy_target
Terraform Plan Summary
  # aws_s3_bucket.checkov_target will be created
  + resource "aws_s3_bucket" "checkov_target" {
      + acceleration_status         = (known after apply)
      + acl                         = (known after apply)
      + arn                         = (known after apply)
      + bucket                      = "atmos-native-ci-e2e-checkov-test"
      + bucket_domain_name          = (known after apply)
      + bucket_prefix               = (known after apply)
      + bucket_regional_domain_name = (known after apply)
      + force_destroy               = false
      + hosted_zone_id              = (known after apply)
      + id                          = (known after apply)
      + object_lock_enabled         = (known after apply)
      + policy                      = (known after apply)
      + region                      = (known after apply)
      + request_payer               = (known after apply)
      + tags_all                    = (known after apply)
      + website_domain              = (known after apply)
      + website_endpoint            = (known after apply)

      + cors_rule (known after apply)

      + grant (known after apply)

      + lifecycle_rule (known after apply)

      + logging (known after apply)

      + object_lock_configuration (known after apply)

      + replication_configuration (known after apply)

      + server_side_encryption_configuration (known after apply)

      + versioning (known after apply)

      + website (known after apply)
    }

  # aws_s3_bucket.this will be created
  + resource "aws_s3_bucket" "this" {
      + acceleration_status         = (known after apply)
      + acl                         = (known after apply)
      + arn                         = (known after apply)
      + bucket                      = "atmos-native-ci-e2e-test"
      + bucket_domain_name          = (known after apply)
      + bucket_prefix               = (known after apply)
      + bucket_regional_domain_name = (known after apply)
      + force_destroy               = false
      + hosted_zone_id              = (known after apply)
      + id                          = (known after apply)
      + object_lock_enabled         = (known after apply)
      + policy                      = (known after apply)
      + region                      = (known after apply)
      + request_payer               = (known after apply)
      + tags                        = {
          + "AtmosFixture" = "native-ci-e2e"
          + "Stage"        = "test"
        }
      + tags_all                    = {
          + "AtmosFixture" = "native-ci-e2e"
          + "Stage"        = "test"
        }
      + website_domain              = (known after apply)
      + website_endpoint            = (known after apply)

      + cors_rule (known after apply)

      + grant (known after apply)

      + lifecycle_rule (known after apply)

      + logging (known after apply)

      + object_lock_configuration (known after apply)

      + replication_configuration (known after apply)

      + server_side_encryption_configuration (known after apply)

      + versioning (known after apply)

      + website (known after apply)
    }

  # aws_s3_bucket.trivy_target will be created
  + resource "aws_s3_bucket" "trivy_target" {
      + acceleration_status         = (known after apply)
      + acl                         = (known after apply)
      + arn                         = (known after apply)
      + bucket                      = "atmos-native-ci-e2e-trivy-test"
      + bucket_domain_name          = (known after apply)
      + bucket_prefix               = (known after apply)
      + bucket_regional_domain_name = (known after apply)
      + force_destroy               = false
      + hosted_zone_id              = (known after apply)
      + id                          = (known after apply)
      + object_lock_enabled         = (known after apply)
      + policy                      = (known after apply)
      + region                      = (known after apply)
      + request_payer               = (known after apply)
      + tags_all                    = (known after apply)
      + website_domain              = (known after apply)
      + website_endpoint            = (known after apply)

      + cors_rule (known after apply)

      + grant (known after apply)

      + lifecycle_rule (known after apply)

      + logging (known after apply)

      + object_lock_configuration (known after apply)

      + replication_configuration (known after apply)

      + server_side_encryption_configuration (known after apply)

      + versioning (known after apply)

      + website (known after apply)
    }

  # aws_s3_bucket_public_access_block.trivy_target will be created
  + resource "aws_s3_bucket_public_access_block" "trivy_target" {
      + block_public_acls       = true
      + block_public_policy     = true
      + bucket                  = (known after apply)
      + id                      = (known after apply)
      + ignore_public_acls      = true
      + restrict_public_buckets = true
    }

Plan: 4 to add, 0 to change, 0 to destroy.

Changes to Outputs:
  + bucket_name = "atmos-native-ci-e2e-test"

…stic

The pre-Cobra CI git-clone bootstrap check (added in the prior commit)
disqualified the bootstrap on any bare, non-"-"-prefixed token, including a
space-separated flag value like the "0" in `--depth 0`. That misread a
value-taking flag's argument as a positional repo name/URI, so the exact
reported reproduction (`atmos git clone --ci --depth 0` in a fresh CI
workspace) still failed on "profile not found".

Replace the heuristic with CIGitCloneBootstrapRequestedFromRawArgs, which
parses the clone-specific args against a throwaway command carrying the
real clone flag set (a fresh newCloneParser() instance, never the shared
singleton) via actual pflag parsing, then defers to the existing
CICloneBootstrapRequested. This also lets an explicit --ci/--ci=false in
the raw args be honored before Cobra resolves the command, which the
removed env-only CIGitCloneModeRequestedFromEnv could not do.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@cmd/git/bootstrap_test.go`:
- Around line 154-220: Add table cases to
TestCIGitCloneBootstrapRequestedFromRawArgs for rawArgs containing “--ci=false”
and “-- --no-tags”, each with CI detected and wantRequest false. Ensure
CIGitCloneBootstrapRequestedFromRawArgs recognizes both the explicit CI opt-out
and native Git arguments after the separator as disqualifying bootstrap
requests.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 2035d8c0-15d7-4f58-b624-5ba29baa8033

📥 Commits

Reviewing files that changed from the base of the PR and between d2b8e81 and 14c9abc.

📒 Files selected for processing (6)
  • cmd/git/bootstrap.go
  • cmd/git/bootstrap_test.go
  • cmd/root.go
  • cmd/root_helpers_test.go
  • docs/fixes/2026-08-05-git-clone-ci-bootstrap-profile-not-found.md
  • pkg/container/common_test.go

Comment thread cmd/git/bootstrap_test.go
Comment on lines +154 to +220
func TestCIGitCloneBootstrapRequestedFromRawArgs(t *testing.T) {
tests := []struct {
name string
rawArgs []string
ciDetected bool
wantRequest bool
}{
{
name: "no args, CI detected",
rawArgs: nil,
ciDetected: true,
wantRequest: true,
},
{
name: "--ci --depth 0 (space-separated value flag)",
rawArgs: []string{"--ci", "--depth", "0"},
ciDetected: true,
wantRequest: true,
},
{
name: "--ci --depth=0 (equals-form value flag)",
rawArgs: []string{"--ci", "--depth=0"},
ciDetected: true,
wantRequest: true,
},
{
name: "--branch main (space-separated string value flag)",
rawArgs: []string{"--branch", "main"},
ciDetected: true,
wantRequest: true,
},
{
name: "explicit positional repo URI disqualifies bootstrap",
rawArgs: []string{"flux-deploy", "--depth", "0"},
ciDetected: true,
wantRequest: false,
},
{
name: "--all disqualifies bootstrap",
rawArgs: []string{"--all"},
ciDetected: true,
wantRequest: false,
},
{
name: "no CI provider detected",
rawArgs: []string{"--ci", "--depth", "0"},
ciDetected: false,
wantRequest: false,
},
{
name: "malformed flag value returns false, deferring to RunE",
rawArgs: []string{"--depth", "not-a-number"},
ciDetected: true,
wantRequest: false,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if tt.ciDetected {
t.Setenv("GITHUB_ACTIONS", "true")
} else {
t.Setenv("GITHUB_ACTIONS", "false")
}
withCleanATMOSCIEnv(t, "")

assert.Equal(t, tt.wantRequest, CIGitCloneBootstrapRequestedFromRawArgs(tt.rawArgs))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== files =="
git ls-files | rg '(^|/)bootstrap(_test)?\.go$|cmd/git|pkg/git' || true

echo "== outline bootstrap_test =="
if [ -f cmd/git/bootstrap_test.go ]; then
  ast-grep outline cmd/git/bootstrap_test.go --view expanded || true
  echo "== relevant lines =="
  nl -ba cmd/git/bootstrap_test.go | sed -n '1,260p'
fi

echo "== find function =="
rg -n "func CIGitCloneBootstrapRequestedFromRawArgs|CIGitCloneBootstrapRequestedFromRawArgs" .

Repository: cloudposse/atmos

Length of output: 3242


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== bootstrap_test.go relevant lines =="
awk '{printf "%6d\t%s\n", NR, $0}' cmd/git/bootstrap_test.go | sed -n '1,260p'

echo "== bootstrap.go outline and relevant lines =="
awk '{printf "%6d\t%s\n", NR, $0}' cmd/git/bootstrap.go | sed -n '1,260p'

echo "== imports and flag definitions =="
rg -n "clone-bootstr|Boot|bootstrap|CIGitCloneBootstrapRequestedFromRawArgs|flags|pflag|Bool|String|Parse|native|ArgsTo" cmd/git/bootstrap.go cmd/git/clone.go cmd/git/flags.go cmd -g '*.go' || true

Repository: cloudposse/atmos

Length of output: 50373


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== cmd/git/bootstrap.go first 360 lines =="
awk 'BEGIN {RS=""; FNR>=1 && FNR<=360} /^func CIGitCloneBootstrapRequestedFromRawArgs/{buf=$0; for(i=1;i<=140;i++)getline buf; print buf}' cmd/git/bootstrap.go || true

echo "== cmd/git/bootstrap_test.go raw cases with marker =="
awk '/func TestCIGitCloneBootstrapRequestedFromRawArgs|CI opt|explicit --ci=false/native arguments/ {ok=1} ok {print FNR"\t"$0} ok && /^}/ && FNR>220 {exit}' cmd/git/bootstrap_test.go || true

echo "== focused source =="
sed -n '1,180p' cmd/git/bootstrap.go
echo "== focused tests =="
sed -n '140,230p' cmd/git/bootstrap_test.go

Repository: cloudposse/atmos

Length of output: 7399


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== go.mod pflag/cobra versions =="
sed -n '1,220p' go.mod | rg -n "pflag|cobra|module " || true

echo "== SplitArgsAtDash definitions/usages =="
rg -n "func SplitArgsAtDash|SplitArgsAtDash" internal pkg cmd -g '*.go' || true

echo "== resolveCICloneMode/CI env usages =="
rg -n "func resolveCICloneMode|resolveCICloneMode|flagCI|CI clone|ATMOS_CI|ciCloneModeDisabled|CloneCI" cmd pkg -g '*.go' || true

echo "== git commands surrounding clone flags =="
sed -n '1,260p' cmd/git/clone.go
echo "== git.go commands =="
sed -n '1,260p' cmd/git/git.go

Repository: cloudposse/atmos

Length of output: 33107


Cover the raw opt-out paths.

Add --ci=false and -- --no-tags cases with wantRequest: false. These raw-input paths still define the earlier config-init-error bootstrap path, and they should not bypass explicit user opt-out or native Git arguments.

Proposed test cases.
 		{
+			name:        "explicit --ci=false opts out",
+			rawArgs:     []string{"--ci=false"},
+			ciDetected:  true,
+			wantRequest: false,
+		},
+		{
+			name:        "native arguments after -- disqualify bootstrap",
+			rawArgs:     []string{"--", "--no-tags"},
+			ciDetected:  true,
+			wantRequest: false,
+		},
 			name:        "--ci --depth 0 (space-separated value flag)",
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cmd/git/bootstrap_test.go` around lines 154 - 220, Add table cases to
TestCIGitCloneBootstrapRequestedFromRawArgs for rawArgs containing “--ci=false”
and “-- --no-tags”, each with CI detected and wantRequest false. Ensure
CIGitCloneBootstrapRequestedFromRawArgs recognizes both the explicit CI opt-out
and native Git arguments after the separator as disqualifying bootstrap
requests.

Source: Coding guidelines

…ands

Fixes #2876. A custom command's `type: container` step with
a `with:` block (engine, driver, cache, tags, etc.) silently dropped
everything, falling back to a bare `docker build -f Dockerfile .`, when
loaded from a commands.yaml merged into atmos.yaml's Viper config tree.

Root cause: `with:` is polymorphic -- decoded into Build/Run/Push/Inspect
for `type: container` steps, or the generic With map otherwise -- but that
promotion lives entirely in Task.UnmarshalYAML/WorkflowStep.UnmarshalYAML
(go-yaml's yaml.Unmarshaler interface), invoked only when something calls
yaml.Node.Decode directly (e.g. standalone workflows/*.yaml files via
pkg/utils.UnmarshalYAMLFromFile). Custom commands merged into atmos.yaml
decode via Viper's mapstructure pipeline (TasksDecodeHook ->
decodeTaskFromMap), which never invokes yaml.Unmarshaler and had no
equivalent promotion, so `with:` only ever reached the raw generic map.

decodeTaskFromMap now pulls `with:` out before the mapstructure decode and
replays the same polymorphic decode via decodeStepWith, round-tripping the
value through YAML so both code paths share one implementation and can't
drift apart.

Reproduced through the real production paths per the bug report's request:
config loaded via InitCliConfig (pkg/config), and the full custom command
executed via RootCmd through a fake logging docker executable (cmd/) --
not by manually constructing schema.Task/WorkflowStep/ContainerBuildStep
literals, which would have bypassed the actual decode bug. Added a
complementary test proving workflow-file and custom-command steps decode
with: identically, per the report's public-contract requirement.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
A component name containing "/" (e.g. a nested layout like ecs/cluster)
made workdir.BuildPath produce a real extra subdirectory instead of a
single path segment, since the name was interpolated into "<stack>-<name>"
without escaping and then filepath.Join'd. That put the nested component's
workdir one level deeper than a flat component's at the same stack.

Any path computed relative to the workdir -- most visibly a relative
`backend.local.path` template like `../../../.context/tfstate/...` --
therefore climbed to a different real ancestor for the nested component
than for the flat one, silently writing state under a different root
(<repo>/.workdir/.context/... instead of <repo>/.context/...) even though
both components used the identical backend config.

Sanitize the component name the same way internal/exec/terraform_generate_
backends.go already does for backend template context: replace "/" with
"-" before building the workdir directory name. BuildPath is the single
formula reused by the source provisioner and by internal/terraform_backend's
JIT-workdir state lookup, so both pick up the fix.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@cmd/custom_command_container_build_test.go`:
- Around line 107-115: Strengthen the test around the Buildx argument assertions
in the relevant custom command container build test: verify each cache flag is
paired with the configured cache reference and mode=max, and validate the buildx
create invocation includes the configured docker-container driver and driver
image option. Use behavior-focused table-driven assertions with the existing
mocked invocation data, while preserving the current checks for tags,
Dockerfile, and context.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: ce2ecf83-e7b5-4ede-9a53-102a97cdf402

📥 Commits

Reviewing files that changed from the base of the PR and between 14c9abc and 28e59ac.

📒 Files selected for processing (7)
  • cmd/custom_command_container_build_test.go
  • internal/terraform_backend/terraform_backend_local_test.go
  • pkg/config/custom_command_container_with_test.go
  • pkg/provisioner/workdir/types.go
  • pkg/provisioner/workdir/types_test.go
  • pkg/schema/task.go
  • pkg/schema/task_test.go

Comment on lines +107 to +115
assert.Contains(t, fields, "--builder", "configured Buildx driver must be applied")
assert.Contains(t, fields, "atmos-native-ci")
assert.Contains(t, fields, "--cache-from", "configured registry cache-from must be applied")
assert.Contains(t, fields, "--cache-to", "configured registry cache-to must be applied")
assert.Contains(t, fields, "-t", "configured tag must be applied")
assert.Contains(t, fields, "example.invalid/demo:sha-test")
assert.Contains(t, fields, "-f", "configured Dockerfile must be applied")
assert.Contains(t, fields, "Dockerfile")
assert.Contains(t, fields, "app", "configured context must be applied")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Assert the configured Buildx option values.

The test checks only that cache flags exist. It does not verify the cache references, mode=max, docker-container driver, or configured driver image. A regression can emit these flags with fallback values and still pass.

Assert the argument paired with each cache flag. Also assert the buildx create invocation contains the configured driver and driver option.

As per coding guidelines, “Prefer behavior-focused, table-driven unit tests with mocks” and target comprehensive feature coverage.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cmd/custom_command_container_build_test.go` around lines 107 - 115,
Strengthen the test around the Buildx argument assertions in the relevant custom
command container build test: verify each cache flag is paired with the
configured cache reference and mode=max, and validate the buildx create
invocation includes the configured docker-container driver and driver image
option. Use behavior-focused table-driven assertions with the existing mocked
invocation data, while preserving the current checks for tags, Dockerfile, and
context.

Source: Coding guidelines

…rsal; surface cached output lookups

BuildPath now sanitizes "/" out of component names, so the containment
guard test's traversal-via-component vector no longer escapes BasePath.
Retarget it at the stack argument, which isn't sanitized the same way and
still needs the guard. Also make cache-hit output lookups emit the same
visible "Fetching ..." notification a real fetch would, instead of only a
Debug-level log, so a second output lookup on an already-cached component
isn't silently invisible.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@github-actions github-actions Bot added size/l Large size PR and removed size/m Medium size PR labels Aug 5, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/fixes/2026-08-05-custom-command-container-with-block-dropped.md`:
- Line 73: Update the validation bullet to name gofumpt instead of gofmt, and
run gofumpt on the affected Go files if it was not already run; retain the
existing go build ./... validation.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: ae8ca090-b11f-420f-8785-28a561b0152a

📥 Commits

Reviewing files that changed from the base of the PR and between 28e59ac and bf74e17.

📒 Files selected for processing (6)
  • docs/fixes/2026-08-05-custom-command-container-with-block-dropped.md
  • docs/fixes/2026-08-05-workdir-nested-component-path-depth.md
  • pkg/terraform/output/config_test.go
  • pkg/terraform/output/executor.go
  • pkg/terraform/output/executor_test.go
  • pkg/terraform/output/executor_utils.go

Comment thread docs/fixes/2026-08-05-custom-command-container-with-block-dropped.md Outdated
…ty; correct fix-log formatter name

CI forces color output (CI=true), which makes the markdown-based UI
renderer split "Fetching vpc_id ..." into multiple ANSI-styled runs right
at the literal underscore, without dropping or reordering any visible
characters. Strip ANSI before the assert.Contains checks, matching the
ansi.Strip convention already used elsewhere in the test suite.

Also correct the fix-log's "gofmt" validation bullet to "gofumpt", the
formatter this repo actually mandates and runs.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Repo mandates gofumpt, not gofmt (CLAUDE.md, .golangci.yml). Denying the
raw command prevents Claude Code from running gofmt directly.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@mergify

mergify Bot commented Aug 6, 2026

Copy link
Copy Markdown

Important

Cloud Posse Engineering Team Review Required

This pull request modifies files that require Cloud Posse's review. Please be patient, and a core maintainer will review your changes.

To expedite this process, reach out to us on Slack in the #pr-reviews channel.

@mergify mergify Bot added the needs-cloudposse Needs Cloud Posse assistance label Aug 6, 2026
@mergify
mergify Bot temporarily deployed to screengrabs August 6, 2026 01:05 Inactive
@codecov

codecov Bot commented Aug 6, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.78%. Comparing base (3ce4349) to head (6577f9e).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2879      +/-   ##
==========================================
+ Coverage   82.76%   82.78%   +0.01%     
==========================================
  Files        1861     1861              
  Lines      180478   180529      +51     
==========================================
+ Hits       149380   149448      +68     
+ Misses      23311    23296      -15     
+ Partials     7787     7785       -2     
Flag Coverage Δ
unittests 82.78% <100.00%> (+0.01%) ⬆️

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

Files with missing lines Coverage Δ
cmd/git/bootstrap.go 100.00% <100.00%> (ø)
cmd/root.go 78.61% <100.00%> (+0.19%) ⬆️
pkg/provisioner/workdir/types.go 100.00% <100.00%> (ø)
pkg/schema/task.go 97.86% <100.00%> (+0.08%) ⬆️
pkg/terraform/output/executor.go 89.86% <100.00%> (-0.09%) ⬇️
pkg/terraform/output/executor_utils.go 97.26% <100.00%> (+0.53%) ⬆️

... and 8 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Adds behavior-focused tests for the 5 lines Codecov flagged as uncovered
on this branch's added code: isCIGitCloneBootstrapArgs's len(args) < 1
guard, decodeTaskFromMap/decodeStepWithFromMapValue's three error-wrap
branches (invalid container action, yaml.Marshal failure via a
yaml.Marshaler that errors, yaml.Unmarshal failure via a dangling YAML
alias), and resolveOutputFromCache's cache-miss and getOutputVariable-
error branches. No production code changes; no assertions weakened.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@pkg/schema/task_test.go`:
- Around line 1109-1120: Update the test case around decodeTaskFromMap to use
the schema container task type instead of TaskTypeShell, ensuring
“not-a-real-action” reaches decodeContainerWith’s invalid-action validation and
preserves the expected ErrWorkflowControlStepInvalid assertion.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 5cfd02c1-5c12-4d0c-86c2-278de47a78dd

📥 Commits

Reviewing files that changed from the base of the PR and between 17efc2a and 427985e.

📒 Files selected for processing (3)
  • cmd/root_helpers_test.go
  • pkg/schema/task_test.go
  • pkg/terraform/output/executor_test.go
🚧 Files skipped from review as they are similar to previous changes (2)
  • cmd/root_helpers_test.go
  • pkg/terraform/output/executor_test.go

Comment thread pkg/schema/task_test.go
… typo

Two fixes from this branch (cache-hit output lookups now visible;
containment-guard test retargeted at the still-open stack-traversal
vector after the workdir fix closed the component-name one) had no
docs/fixes/ record. Also corrects a stale "gofmt" mention in the
git-clone-ci-bootstrap doc to "gofumpt", matching the correction already
applied to the container with-block doc.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Fixes a flaky Windows Acceptance Tests failure: resolving 10 platforms
took 784ms against a 750ms threshold, even though that's nowhere near the
1.5s serial floor the test guards against. Raises the bound to 4/5 of the
serial floor (1200ms) for headroom against normal CI timing variance,
Windows runners especially. No production code changed.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
createWorkdirDirectory duplicated the unsanitized stack-componentName
formula that BuildPath was already fixed to sanitize, so a local
(non-source) component with provision.workdir.enabled: true and a
nested name still got a workdir one level deeper than a flat sibling,
silently shifting where relative backend.local.path state resolves.
Now delegates to BuildPath so both formulas can't drift apart again.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
DetermineTargetDirectory's non-workdir fallback (the default vendoring
path when provision.workdir.enabled is unset) joined the component
base path with the raw component name with no containment check, so a
component named with ../ segments could vendor outside
components/terraform/. Adds the same absolutize-and-prefix containment
guard already used by the two other BuildPath-derived callers.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…rrides

Sibling gap to the with: block fix (docs/fixes/2026-08-05-custom-
command-container-with-block-dropped.md): a custom command step's
container: override went through three independent failures. The
bare boolean opt-out (container: false) broke InitCliConfig for the
whole atmos.yaml because decodeTaskFromMap never round-tripped
container: through YAML the way with: now does. The mapping form
decoded fine but was never consulted at execution time -- the
custom-command step loop always ran type: shell steps on the host.
And once both of those were fixed, container: false still ran the
step inside a container because cloneCommand's JSON round-trip
silently dropped WorkflowContainer.Enabled (json:"-"), inverting the
opt-out.

Fixes all three: decodeTaskContainerFromMapValue mirrors the with:
fix's round-trip for container:, cmd/cmd_utils.go's step loop now
reuses the same pkg/workflow/container.go session logic the
workflow-file path already uses, and WorkflowContainer gained
MarshalJSON/UnmarshalJSON so Enabled survives a JSON round-trip.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
ContainerRunStep.Restart/.HealthCheck decoded fine but EphemeralConfig
(the runtime config for type: container, action: run steps) had no
such fields, and buildRunConfig never populated them -- unlike the
persistent-component path, which already wires the same settings.
Adds the fields to EphemeralConfig and populates them via the
existing (previously unused for this path) RestartPolicyFromStep/
HealthCheckFromStep helpers, so --restart/--health-* flags now reach
the real docker/podman invocation for step-based container runs.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Two DX gaps where already-computed diagnostic detail never reached
the default log level: a YAML syntax error in an import:-loaded
commands file was silently swallowed (Debug-only), leaving only a
generic "Unknown command" with no hint a config file failed to parse;
and container-step validation (missing required field, invalid pull:
value) already computed the field/step/type and the bad value but
only exposed them via --verbose or dropped them entirely.

LocalAdapter now pairs its existing log.Debug with a ui.Warning
naming the file and parse error. ValidateRequired's default message
now names the field; invalidContainerField now echoes the actual
invalid value typed.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Bumps three pnpm.overrides pins to their patched releases, all within
the major-version line dependabot.yml's ignore policy allows:

- js-yaml 3.15.0 -> 3.15.1 (GHSA-5p4m-2wfm-xmqj, alert #269)
- js-yaml 4.3.0 -> 4.3.1 (GHSA-5p4m-2wfm-xmqj, alert #268)
- mermaid 11.16.0 -> 11.16.1 (GHSA-rhh3-jpg6-66xh #267,
  GHSA-c4c3-pg64-4m4v #266, GHSA-6x64-9x62-f2gx #265,
  GHSA-3rrr-jr9j-h3q3 #264, GHSA-2v8p-3f2j-5mp7 #263)

Verified via `pnpm run build` in website/; NOTICE unchanged (no
license changes from these patch bumps).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-cloudposse Needs Cloud Posse assistance patch A minor, backward compatible change size/l Large size PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant