docs: add Terragrunt migration skill and fix stale PRD statuses - #2878
docs: add Terragrunt migration skill and fix stale PRD statuses#2878Erik Osterman (Cloud Posse) (osterman) wants to merge 26 commits into
Conversation
…tion research Checkpoint before syncing this branch with origin/main — these fixes were made against an older snapshot and will likely need rework once current upstream content is merged in. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… stale PRD statuses Adds the atmos-migration skill's Terragrunt reference (classic and Stacks patterns, concept mapping, migration workflow), hands-on-validated against a real Terragrunt Stacks example run end to end on the floci/aws emulator. Corrects four PRD status headers that had gone stale relative to shipped code, fixes pre-existing EditorConfig indentation violations the commit hook surfaced in two of those files, and documents the mocks/--use-mocks feature in the website Terragrunt migration guide as the direct equivalent of mock_outputs. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Tip Atmos Pro
No affected stacks workflow was detected for this pull request. |
Dependency Review✅ No vulnerabilities or license issues found.Scanned FilesNone |
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR adds Terragrunt migration guidance, updates Terraform mock fallback behavior, improves provenance filtering, records shipped PRD functionality, updates snapshots and documentation, and adjusts Windows-sensitive session test timeout documentation. ChangesTerragrunt migration guidance
Terraform mock behavior
Provenance filtering
PRD status documentation
Windows session test timeouts
Estimated code review effort: 3 (Moderate) | ~25 minutes Merge Risk: ⚪ Minimal · up to The PR adds migration guidance and documentation corrections alongside localized internal updates. Remaining issues are minor documentation accuracy and formatting follow-ups, with no actionable merge-blocking risk. Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 61.54% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 13 functions across 7 files. (19 skipped: 19 unsupported.) ✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@agent-skills/skills/atmos-migration/references/from-terragrunt.md`:
- Around line 323-324: Update the dependency translation guidance in the “Wire
dependency blocks” step so Terragrunt mock_outputs maps to the Atmos mocks
component field and explicitly uses --use-mocks, rather than the // "default"
fallback. Reserve // "default" for real dependencies that have not deployed yet.
- Around line 97-103: Update the `generate:` stack section description to
explicitly list all five inheritance levels, including the
base-component/catalog-default level before `component` and `override`. Use the
established terminology from the generation documentation or changelog while
preserving the existing backend/provider distinction.
In `@docs/prd/custom-hooks.md`:
- Line 3: Update the status statement in the document introduction to replace
the relative “today” wording with a concrete release or absolute date, while
preserving the existing shipped and outstanding feature details.
In `@docs/prd/dag-concurrent-execution.md`:
- Around line 3-5: Reconcile the PRD’s status claims with the implementation:
update the stale `ExecuteTerraformQuery()` description to reflect
scheduler/DAG-aware routing, and revise the Phase 2 checklist around scheduler
wiring and `--max-concurrency` so completed work is marked shipped or removed.
Keep the remaining Phase 4 items explicitly open and ensure the overview and
detailed sections describe the same state.
🪄 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: 7cb7c56c-6c4a-4d28-b218-c610b3d1a5ac
📒 Files selected for processing (7)
agent-skills/skills/atmos-migration/SKILL.mdagent-skills/skills/atmos-migration/references/from-terragrunt.mddocs/prd/code-generation.mddocs/prd/custom-hooks.mddocs/prd/dag-concurrent-execution.mddocs/prd/import-adapter-registry.mdwebsite/docs/migration/terragrunt.mdx
…entation CI caught two real issues in the new Terragrunt migration reference: - Two examples used the legacy doubled-double-quote YQ escaping (!terraform.state x ".field // ""default""") instead of the clean current syntax (!terraform.state x .field // "default"), which scripts/check- terraform-example-syntax.sh flags outside its designated compatibility fixtures. - The "Migration Workflow" numbered list used 3-space continuation indentation, not a multiple of the repo's 2-space EditorConfig setting. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2878 +/- ##
=======================================
Coverage 83.61% 83.61%
=======================================
Files 1933 1933
Lines 189424 189462 +38
=======================================
+ Hits 158385 158428 +43
+ Misses 23126 23119 -7
- Partials 7913 7915 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
…unt migration guide Reconciles PRD status claims that contradicted themselves (dag-concurrent-execution.md Phase 3 is only partially shipped, not fully; custom-hooks.md's relative "today" date), completes the from-terragrunt.md 5-level merge listing, and fixes a hallucinated `settings.terraform.provider_overrides` key found via hands-on field testing. Also recommends `atmos list affected` over `atmos describe affected` for human-run migration comparisons (table output vs. a wall of YAML), notes both diff committed trees only, and updates the Change Tracking table to the current `dependencies.files`/`folders` syntax instead of the legacy inline `kind: file`/`kind: folder` form. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 `@agent-skills/skills/atmos-migration/references/from-terragrunt.md`:
- Around line 325-326: Update step 3 in the dependency-block workflow to map
Terragrunt mock_outputs to the producer component’s mocks field, using
--use-mocks for read-only planning or component description. Remove the //
"default" mapping for mock_outputs, reserving YQ defaults for real dependencies
that are not yet deployed.
- Line 296: Quote each YQ default expression as a single argument in both
affected examples: update
agent-skills/skills/atmos-migration/references/from-terragrunt.md lines 296-296
to quote the .arn default expression, and lines 179-183 to quote the .vpc_id
default expression. Preserve the existing Terraform state references and
fallback values.
In `@docs/prd/dag-concurrent-execution.md`:
- Around line 841-847: Resolve the contradictory `os.Stdout` documentation by
updating the stale status near the terraform plan diff discussion to reflect
that the swap was replaced with stream injection, or revise the Phase 1 shipped
claim if that implementation is not complete. Ensure the PRD consistently
describes only the actual current behavior.
- Around line 3-5: Clarify the cross-type dependency status in the document’s
overview and the Phase 3 status sections: distinguish the shipped
`dependencies.components.kind` syntax from the still-open legacy
`settings.depends_on` support, or update the status consistently if both are
shipped. Align the references near `dependencies.components`, the Phase 3
multi-type-DAG item, and the PR `#2193` note.
In `@website/docs/migration/terragrunt.mdx`:
- Around line 857-858: Update the documented atmos list affected example and its
adjacent comment so they agree: pass --base main to explicitly compare against
main, preserving the stated comparison behavior.
🪄 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: 178b6585-1d19-44de-8438-23d192668df9
📒 Files selected for processing (4)
agent-skills/skills/atmos-migration/references/from-terragrunt.mddocs/prd/custom-hooks.mddocs/prd/dag-concurrent-execution.mdwebsite/docs/migration/terragrunt.mdx
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/prd/custom-hooks.md
…tion guide Step 3 of the migration workflow still mapped mock_outputs to the YQ // "default" pattern, contradicting the mocks:/--use-mocks mapping documented a few paragraphs earlier. Quotes the YQ default expressions for consistency with atmos-yaml-functions/SKILL.md and atmos-components/SKILL.md. dag-concurrent-execution.md had two more self-contradictions: the Subprocess Execution section still described the os.Stdout race that Phase 1 already fixed (terraform_plan_diff.go now captures via bytes.Buffer), and the Resolved Questions section claimed cross-type dependency syntax was "solved by PR #2193" — traced the code and found pkg/scheduler/adapters/terraform.go explicitly skips any dependency whose kind isn't "terraform", so the kind field is schema-parseable but not yet consumed by the scheduler; corrected to match the already-accurate Phase 3 status. terragrunt.mdx's list-affected example claimed to compare against main by default without passing --ref; list affected has no --base flag (unlike describe affected), so made the comparison explicit with --ref main instead. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
agent-skills/skills/atmos-migration/references/from-terragrunt.md (1)
331-334: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAdd the explicit base reference to
list affected.The migration workflow must document
atmos list affected --ref main. The current command omits--ref main, so the required comparison target is not explicit.Suggested fix
- `atmos list affected` (human-readable table; commit your change first — it diffs committed trees, not + `atmos list affected --ref main` (human-readable table; commit your change first — it diffs committed trees, not🤖 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 `@agent-skills/skills/atmos-migration/references/from-terragrunt.md` around lines 331 - 334, Update the migration workflow step referencing `atmos list affected` to include the explicit `--ref main` option, while preserving the existing guidance about committing changes and using `atmos describe affected` for scripted or CI output.
🤖 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.
Outside diff comments:
In `@agent-skills/skills/atmos-migration/references/from-terragrunt.md`:
- Around line 331-334: Update the migration workflow step referencing `atmos
list affected` to include the explicit `--ref main` option, while preserving the
existing guidance about committing changes and using `atmos describe affected`
for scripted or CI output.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 50007ed0-acdf-464e-8f28-b38dda05438b
📒 Files selected for processing (3)
agent-skills/skills/atmos-migration/references/from-terragrunt.mddocs/prd/dag-concurrent-execution.mdwebsite/docs/migration/terragrunt.mdx
🚧 Files skipped from review as they are similar to previous changes (1)
- website/docs/migration/terragrunt.mdx
…lt parity A field test of --use-mocks found `describe component` silently rendering empty output whenever a component's provenance path wasn't matched due to an unnormalized lookup, a mock-output error that mislabeled the output name as a component name, and a YQ `//` default that only rescued a missing key inside a declared `mocks` map, not a component with no `mocks` section at all -- inconsistent with how `//` already rescues real state. Also cross-references the mocks:/--use-mocks feature from the docs pages and skill most likely to be read first. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
internal/exec/terraform_mocks_test.go (2)
85-85: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winBuild the fixture path with
filepath.Join.At Line 85, the new test hard-codes a slash-separated relative path. Use
filepath.Joinand importpath/filepathto keep the test portable.Proposed fix.
+ "path/filepath" + - sandbox, err := testhelpers.SetupSandbox(t, "../../tests/fixtures/scenarios/terraform-component-mocks") + sandbox, err := testhelpers.SetupSandbox(t, filepath.Join("..", "..", "tests", "fixtures", "scenarios", "terraform-component-mocks"))As per coding guidelines, tests must use
filepath.Joinfor paths.🤖 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 `@internal/exec/terraform_mocks_test.go` at line 85, Update the test setup path in the relevant test function to use filepath.Join with each path component, and add the path/filepath import. Preserve the existing fixture location and pass the joined path to testhelpers.SetupSandbox.Source: Coding guidelines
97-99: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winCover the
!terraform.outputfallback path.At Line 97, the regression test calls only
processTagTerraformState. Add the equivalentprocessTagTerraformOutputcase for a component withoutmocks. The changed resolver and documentation promise the same//behavior for both tags.As per coding guidelines, prefer behavior-focused tests with comprehensive 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 `@internal/exec/terraform_mocks_test.go` around lines 97 - 99, Add a behavior-focused test alongside the existing processTagTerraformState fallback case that invokes processTagTerraformOutput for a component without mocks, using the same missing-value and // fallback scenario. Assert no error and the expected fallback value to cover the promised equivalent behavior for both tags.Source: Coding guidelines
🤖 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 `@errors/errors.go`:
- Line 218: Update the ErrTerraformMockOutputNotDeclared sentinel message to
remove its trailing period, preserving the existing wording so wrapped errors in
terraform mock handling produce clear punctuation.
In `@pkg/provenance/data_transform_test.go`:
- Around line 77-105: Extend TestFilterEmptySectionsKeepsComponentSections to
record provenance for the array-element path
components.terraform.app.vars[0].foo, then verify that the vars section remains
after filterEmptySections. Preserve the existing dot-nested, backend, and
metadata assertions while adding coverage for the array-element provenance
branch.
In `@pkg/provenance/tree_renderer.go`:
- Around line 28-30: Update the inline comments for defaultSeparatorWidth,
commentSpaceNeeded, and maxLineLength so each ends with a period, without
changing the constants or their values.
---
Nitpick comments:
In `@internal/exec/terraform_mocks_test.go`:
- Line 85: Update the test setup path in the relevant test function to use
filepath.Join with each path component, and add the path/filepath import.
Preserve the existing fixture location and pass the joined path to
testhelpers.SetupSandbox.
- Around line 97-99: Add a behavior-focused test alongside the existing
processTagTerraformState fallback case that invokes processTagTerraformOutput
for a component without mocks, using the same missing-value and // fallback
scenario. Assert no error and the expected fallback value to cover the promised
equivalent behavior for both tags.
🪄 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: 6525373d-ff2a-445d-b545-32e9eb214373
📒 Files selected for processing (12)
agent-skills/skills/atmos-yaml-functions/SKILL.mderrors/errors.gointernal/exec/terraform_mocks.gointernal/exec/terraform_mocks_test.gopkg/provenance/data_transform.gopkg/provenance/data_transform_test.gopkg/provenance/tree_renderer.gowebsite/blog/2026-07-15-terraform-component-mocks.mdxwebsite/docs/components/terraform/stack-config.mdxwebsite/docs/functions/yaml/terraform.output.mdxwebsite/docs/functions/yaml/terraform.state.mdxwebsite/docs/migration/terragrunt.mdx
🚧 Files skipped from review as they are similar to previous changes (1)
- website/docs/migration/terragrunt.mdx
… provenance fix The filterEmptySections fix (6c22503) corrected describe_component to stop silently dropping real sections (backend, metadata, env, overrides) that lack a stack-root section of the same name. CI caught the resulting golden snapshot drift on both linux and macos; regenerated via `-regenerate-snapshots` per CLAUDE.md, verified the diffs only add the previously-hidden, now-correct content. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… flake Acceptance Tests (linux, shard 9/10) failed TestCLICommands/atmos_exit_code_should_be_same_as_command_exit_code_(2) with "Expected exit code 2, got 1". The real cause was tofu init timing out reaching registry.opentofu.org (context deadline exceeded) before any plan could run -- confirmed the fixture has no registry-mirror config to regress, and the sibling (0)/(1) exit-code cases in the same file passed. No code change: there's nothing in this repo that fixes a transient outage on a public third-party registry, and loosening the exit-code assertion would mask a real CLI exit-code-propagation regression if one ever occurs. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
All five attached failure logs (Acceptance Tests linux/macos/windows,
[k3s] demo-helmfile, Build windows) traced to one event: workflow run
33394180592 on this PR was cancelled (confirmed via gh api), not failed. The
test/k3s matrix jobs were skipped as a result, but the -required gate jobs
(if: always()) still ran and misreported the cancellation as a hard failure
("expected 10 shard jobs, found 0" / "k3s matrix result was 'skipped'").
needs.test.result and needs.k3s.result both report "skipped" for a genuine
upstream failure and for a whole-run cancellation alike, so they can't
distinguish the two - cancelled() can, and is the fix. It's only valid in an
if:, not inside a run: script (caught by actionlint), so both gates get a
"Skip verification" step under if: cancelled() plus if: !cancelled() on
their existing check steps, leaving the fail-loudly-on-genuine-anomalies
logic untouched for real failures.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Important Cloud Posse Engineering Team Review RequiredThis 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 |
…igration-skill # Conflicts: # .github/workflows/test.yml
|
Warning SHA Pin Verification Passed — with documented exceptionsAll 229 third-party action reference(s) are covered, but 2 rely on a documented allowlist entry in
See the action run for full details. |
Review Dependency Licenses failed: NOTICE had "URL: Unknown" for cuelang.org/go, but a fresh generate-notice.sh run resolved a real URL, tripping the out-of-date check. Root cause was a race, not one bad run: this branch's merge commit already had the correct URL, but a subsequent [autocommit] formatting fixes commit (atmos-pro[bot]) regenerated NOTICE under a network condition where go-licenses' live resolution for cuelang.org/go failed, silently reverting it to "Unknown" and committing that regression - exactly the oscillation scripts/generate-notice.sh's REPO_OVERRIDES mechanism exists to prevent for modules go-licenses can't resolve reliably, cuelang.org/go just wasn't in the list yet. Added it (repo github.com/cue-lang/cue, no tag prefix, LICENSE path), which reconstructs the exact URL CI itself resolved (https://github.com/cue-lang/cue/blob/v0.16.1/LICENSE) from go.mod's pinned v0.16.1 with no network dependency, and applied that one-line NOTICE fix by hand: a local generate-notice.sh run silently produced a truncated 102-dependency report (vs. CI's 643) with 0 Apache-2.0/BSD licenses found, consistent with this machine lacking a Linux-targeting C cross-compiler for CGO_ENABLED=1 GOOS=linux GOARCH=amd64 - so that broken local output was discarded rather than committed, and the NOTICE line was hand-verified against the override's own URL-construction formula and go.mod's version instead. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…lures Review Dependency Licenses failed installing go-licenses@v1.6.0: a mid-stream HTTP/2 reset (stream ID 1155; INTERNAL_ERROR) reading sum.golang.org during go install's go.sum verification, unrelated to any actual dependency problem and unrelated to the immediately preceding commit on this branch (confirmed via gh api against head_sha 5ac5d97, which only touched an unrelated NOTICE URL override). Same failure class already fixed once for go mod download (docs/fixes/2026-08-25-build-atmos-go-mod-download-retry.md, later ported to magefiles/build.go's runGoModDownload) - just hit a different network call (go install's dependency-graph resolution) in a different script. Wrapped generate-notice.sh's bare go install in the same 3-attempt/15s-backoff until loop, matching .github/actions/download-artifact-retry's convention. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Run pre-commit hooks failed atmos-validate-editorconfig: a fenced code block in docs/fixes/2026-08-31-notice-go-licenses-install-retry.md quoted a Go toolchain error message verbatim, including its original tab-indented continuation line - violating this repo's *.md indent_style=space rule. Replaced the literal tab with two spaces (matching indent_size=2), content otherwise unchanged. Scanned every other 2026-08-31 fix-log doc added this session for the same issue; none found. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…igration-skill # Conflicts: # agent-skills/skills/atmos-migration/SKILL.md
…hanged fix(mocks) commit 6c22503 edited this post's body (the // default behavior clarification) on 2026-08-06, but the post kept displaying/sorting under its original 2026-07-15 publish date since Docusaurus has no separate date. Added an explicit date: frontmatter override for the edit date, matching this repo's existing convention for date overrides (e.g. 2026-01-02-unified-task-runner.mdx). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
what
agent-skills/skills/atmos-migration/references/from-terragrunt.md: concept mapping (include,dependency/mock_outputs, source pinning,before_hook) and a migration workflow for both classic Terragrunt and Terragrunt Stacks, hands-on-validated by migrating a real Terragrunt Stacks example (Lambda + DynamoDB + IAM role) end to end against thefloci/awsemulator.agent-skills/skills/atmos-migration/SKILL.md(routing table, frontmatter, and the previously-stale "not covered by this skill" line).dag-concurrent-execution.md,custom-hooks.md,code-generation.md,import-adapter-registry.md) that had gone stale relative to already-shipped code, and fixes pre-existing EditorConfig indentation violations the commit hook surfaced in two of those files.mocks/--use-mockscomponent feature inwebsite/docs/migration/terragrunt.mdxas the direct equivalent of Terragrunt'smock_outputs, matching the doc's existing tabbed format, plus a corresponding migration checklist item.why
atmos-migrationskill previously routed Terragrunt users to an external doc with no agent-actionable guidance ("not covered by this skill"); this fills that gap with a validated, working recipe.mocks/--use-mocksfeature is a closer match to Terragrunt'smock_outputs_allowed_terraform_commandssemantics (explicit, command-scoped, fail-closed) than the YAML-function default-value pattern previously documented, and the website guide had no mention of it at all.references
Summary by CodeRabbit
New Features
mocksconfiguration and--use-mockssupport.Documentation
Bug Fixes