Skip to content

feat(description): add first-class description field to components and stacks via metadata.description#2200

Open
Copilot wants to merge 18 commits intomainfrom
copilot/add-description-field-to-components
Open

feat(description): add first-class description field to components and stacks via metadata.description#2200
Copilot wants to merge 18 commits intomainfrom
copilot/add-description-field-to-components

Conversation

Copy link
Contributor

Copilot AI commented Mar 15, 2026

  • Fix logic bug: move description propagation into processComponentEntry (order-independent, idempotent) and stamp pre-created entries immediately
  • Fix website build: change blog post author from [rb] to [atmos]
  • Add regression test TestProcessStackFile_DescriptionOnPreCreatedEntry

📱 Kick off Copilot coding agent tasks wherever you are with GitHub Mobile, available on iOS and Android.

@mergify mergify bot added triage Needs triage wip Work in Progress: Not ready for final review or merge labels Mar 15, 2026
@nitrocode nitrocode added minor New features that do not break anything and removed triage Needs triage labels Mar 15, 2026
Copilot AI and others added 2 commits March 15, 2026 01:42
…d stacks

- Add DescriptionSectionName constant to pkg/config/const.go
- Extract and propagate description in stack_processor_process_stacks.go at stack level
- Extract ComponentDescription in stack_processor_process_stacks_helpers_extraction.go
- Add ComponentDescription to ComponentProcessorResult struct
- Add description to final comp map in stack_processor_merge.go
- Add description to all 4 component type loops in describe_stacks.go (terraform/helmfile/packer/ansible)
- Add setStackDescription helper for stack-level description surfacing
- Fix empty stacks filter to use sections filter for relevance check
- Add description to FilterComputedFields whitelist in describe_component.go
- Update 3 JSON schema files with description field for components and stacks
- Add description to top-level-component1 and us-east-2 test fixtures
- Add 4 unit tests for description field support

Co-authored-by: nitrocode <7775707+nitrocode@users.noreply.github.com>
…blog post

- Add description to default checkSections list to prevent pruning components with only description
- Remove speculative template variable example from blog post
- Add stack-config schema (stacks/stack-config/1.0.json) with description field

Co-authored-by: nitrocode <7775707+nitrocode@users.noreply.github.com>
@github-actions github-actions bot added the size/m Medium size PR label Mar 15, 2026
@nitrocode nitrocode marked this pull request as ready for review March 15, 2026 01:51
@nitrocode nitrocode requested a review from a team as a code owner March 15, 2026 01:51
Copilot AI changed the title [WIP] Add description field to components and stacks feat(description): add first-class description field to components and stacks Mar 15, 2026
Copilot AI requested a review from nitrocode March 15, 2026 01:51
@mergify mergify bot removed the wip Work in Progress: Not ready for final review or merge label Mar 15, 2026
@github-actions
Copy link

github-actions bot commented Mar 15, 2026

Dependency Review

✅ No vulnerabilities or license issues found.

Snapshot Warnings

⚠️: No snapshots were found for the head SHA d88addf.
Ensure that dependencies are being submitted on PR branches and consider enabling retry-on-snapshot-warnings. See the documentation for more information and troubleshooting advice.

Scanned Files

None

@nitrocode

This comment was marked as outdated.

Co-authored-by: nitrocode <7775707+nitrocode@users.noreply.github.com>

This comment was marked as outdated.

@nitrocode

This comment was marked as outdated.

…cribe component output

Co-authored-by: nitrocode <7775707+nitrocode@users.noreply.github.com>

This comment was marked as outdated.

@codecov
Copy link

codecov bot commented Mar 15, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 77.27%. Comparing base (2be2909) to head (d88addf).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2200      +/-   ##
==========================================
+ Coverage   77.21%   77.27%   +0.05%     
==========================================
  Files        1018     1018              
  Lines       96368    96392      +24     
==========================================
+ Hits        74415    74484      +69     
+ Misses      17755    17709      -46     
- Partials     4198     4199       +1     
Flag Coverage Δ
unittests 77.27% <100.00%> (+0.05%) ⬆️

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

Files with missing lines Coverage Δ
internal/exec/describe_component.go 69.10% <100.00%> (+0.10%) ⬆️
...ternal/exec/describe_stacks_component_processor.go 95.47% <100.00%> (+0.14%) ⬆️
internal/exec/stack_processor_merge.go 79.11% <100.00%> (+0.16%) ⬆️
internal/exec/stack_processor_process_stacks.go 81.85% <100.00%> (+0.29%) ⬆️

... and 10 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions github-actions bot added the size/m Medium size PR label Mar 24, 2026
@nitrocode

This comment was marked as outdated.

@mergify mergify bot removed the conflict This PR has conflicts label Mar 24, 2026
@coderabbitai

This comment was marked as outdated.

Copy link
Contributor Author

Copilot AI commented Mar 24, 2026

@copilot resolve merge conflicts

Resolved in df8a763. Merged main (38 commits ahead, including PR #2204's major refactor of describe_stacks.go from 1436 lines → 251 lines using a processor pattern).

Conflict resolution:

  • describe_stacks.go: took main's refactored version entirely
  • describe_stacks_component_processor.go: added setStackDescription() helper and modified processStackFile() to propagate stack-level metadata.description to newly created stack entries using a before/after snapshot of finalStacksMap
  • describe_stacks_test.go: combined main's new tests with updated TestSetStackDescription (simplified function signature: takes description string directly)

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • us.i.posthog.com
    • Triggering command: /tmp/atmos-test-32873/atmos /tmp/atmos-test-32873/atmos describe config -f yaml -bool t ux-amd64/pkg/tool/linux_amd64/compile -errorsas -ifaceassert t ux-amd64/pkg/too-trimpath -ato�� -bool t ux-amd64/pkg/tool/linux_amd64/compile -errorsas -ifaceassert t ux-amd64/pkg/too-buildtags (dns block)
    • Triggering command: /tmp/atmos-test-32873/atmos /tmp/atmos-test-32873/atmos describe config -f yaml se go ux-amd64/pkg/tool/linux_amd64/compile -errorsas -ifaceassert t ux-amd64/pkg/too-trimpath -ato�� v3.6.1/pkg/validation/validators/validators.go t ux-amd64/pkg/tool/linux_amd64/vet -errorsas -ifaceassert t ux-amd64/pkg/too-buildtags (dns block)
    • Triggering command: /tmp/atmos-test-32873/atmos /tmp/atmos-test-32873/atmos --chdir examples/demo-stacks describe config -f yaml ux-amd64/pkg/tool/linux_amd64/asm -errorsas -ifaceassert t ux-amd64/pkg/too-trimpath -ato�� -bool t ux-amd64/pkg/tool/linux_amd64/vet -errorsas -ifaceassert t ux-amd64/pkg/too-buildtags (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

@coderabbitai

This comment was marked as outdated.

1. Fix compile error: declare stackDescription in processStackFile
2. Gofmt already applied by autofix.ci (no-op)
3. Fix vacuous test in TestDescribeStacksNoDescriptionField
4. Add TestDescribeStacksHelmfileComponentDescription for coverage
5. Add CLI golden snapshot for --sections description filter"

Co-authored-by: nitrocode <7775707+nitrocode@users.noreply.github.com>
Agent-Logs-Url: https://github.com/cloudposse/atmos/sessions/6d2569f8-5d96-49fa-89ae-bc60931eebd3
@nitrocode

This comment was marked as outdated.

@coderabbitai

This comment was marked as outdated.

- Move description application into processComponentEntry so it's
  order-independent when multiple files contribute to the same stack name
- Stamp description immediately on pre-created entries (includeEmptyStacks path)
- Remove the fragile existingStacks snapshot approach
- Add regression test TestProcessStackFile_DescriptionOnPreCreatedEntry
- Fix blog post author: change [rb] to [atmos]"

Agent-Logs-Url: https://github.com/cloudposse/atmos/sessions/a22ac351-da61-40f2-b79a-2cf9ff77a3ed

Co-authored-by: nitrocode <7775707+nitrocode@users.noreply.github.com>
@nitrocode nitrocode marked this pull request as ready for review March 27, 2026 00:40
@nitrocode

This comment was marked as outdated.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 27, 2026

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Get current CI check status
gh pr view 2200 --repo cloudposse/atmos --json title,state,statusCheckRollup 2>/dev/null | jq '{title, state, checks: [.statusCheckRollup[]? | {name: .name, conclusion: .conclusion, status: .status}] | sort_by(.name)}'

Length of output: 6685


🏁 Script executed:

#!/bin/bash
# Check includeEmptyStacks snapshot ordering - is the existingStacks snapshot taken before or after pre-creation?
grep -n "existingStacks\|includeEmptyStacks\|setStackDescription\|finalStacksMap" internal/exec/describe_stacks_component_processor.go | head -60

# Show the relevant section
sed -n '100,190p' internal/exec/describe_stacks_component_processor.go

Length of output: 6048


🏁 Script executed:

#!/bin/bash
# Check website blog post author slug
find . -name "*.mdx" -path "*/blog/*2026*" 2>/dev/null | head -5
find . -name "*.mdx" -path "*/blog/*description*" 2>/dev/null | head -5
# Check if rb author exists
find . -name "authors.yml" -o -name "authors.yaml" 2>/dev/null | head -5

Length of output: 372


🏁 Script executed:

#!/bin/bash
# Check describe_stacks_test.go for packer/ansible tests and section filter tests
grep -n "TestDescribeStacks\|func Test\|packer\|ansible" pkg/describe/describe_stacks_test.go | head -60
grep -n "TestDescribeStacks\|func Test\|packer\|ansible" internal/exec/describe_stacks_test.go | head -40

Length of output: 4695


🏁 Script executed:

#!/bin/bash
# Check the blog post MDX file
find . -name "*description*" -name "*.mdx" 2>/dev/null
cat website/blog/2026-03-15-description-field-on-components-and-stacks.mdx 2>/dev/null | head -30

Length of output: 1030


🏁 Script executed:

#!/bin/bash
# Get full diff since last known state
gh pr diff 2200 --repo cloudposse/atmos 2>/dev/null | wc -l
gh pr diff 2200 --repo cloudposse/atmos 2>/dev/null | grep "^+" | grep -i "description\|existingStacks\|includeEmpty\|packer\|ansible\|author\|rb\]\|snapshot" | head -60

Length of output: 4382


🏁 Script executed:

#!/bin/bash
# Check the authors.yml for atmos slug
cat website/blog/authors.yml 2>/dev/null | grep -A5 "^atmos\|^  atmos\|key: atmos\|- atmos"
# Also just show the file
head -60 website/blog/authors.yml 2>/dev/null

Length of output: 1739


🏁 Script executed:

#!/bin/bash
# Get details on Windows acceptance test failure
gh run list --repo cloudposse/atmos --branch "$(gh pr view 2200 --repo cloudposse/atmos --json headRefName -q .headRefName)" --limit 5 --json databaseId,name,conclusion,status 2>/dev/null

# Find the failing Windows acceptance test run ID
gh run list --repo cloudposse/atmos --limit 10 --json databaseId,name,conclusion,workflowName 2>/dev/null | jq '.[] | select(.conclusion == "failure")' | head -30

Length of output: 962


🏁 Script executed:

#!/bin/bash
# Check TestDescribeStacksDescriptionSectionFilter body for completeness
grep -n "TestDescribeStacksDescriptionSectionFilter\|settings\|env\|metadata\|workspace\|backend\|inheritance" pkg/describe/describe_stacks_test.go | head -50
# Show the function body
sed -n '533,575p' pkg/describe/describe_stacks_test.go

Length of output: 2803


🏁 Script executed:

#!/bin/bash
# Check TestExecuteDescribeStacks_IncludeEmptyStacks for description assertions
sed -n '437,475p' internal/exec/describe_stacks_test.go

# Check TestProcessStackFile_DescriptionOnPreCreatedEntry
grep -n "TestProcessStackFile_DescriptionOnPreCreatedEntry" internal/exec/describe_stacks_test.go

Length of output: 1682


🏁 Script executed:

#!/bin/bash
# Check the includeEmptyStacks block ordering in full detail
sed -n '85,145p' internal/exec/describe_stacks_component_processor.go

# Check processComponentEntry for setStackDescription call
sed -n '280,310p' internal/exec/describe_stacks_component_processor.go

Length of output: 4096


🏁 Script executed:

#!/bin/bash
# Check if packer/ansible description tests were added in pkg/describe
grep -n "packer\|ansible\|Packer\|Ansible" pkg/describe/describe_stacks_test.go | head -20

# Check describe_stacks_test.go for description in packer/ansible tests
grep -n "description\|Description" internal/exec/describe_stacks_test.go | grep -i "packer\|ansible" | head -10
sed -n '153,200p' internal/exec/describe_stacks_test.go

Length of output: 1283


@nitrocodeAudit Round 4


1. Executive Summary

Round 4 shows material progress: the includeEmptyStacks description-drop logic bug is cleanly resolved (inline stamp at pre-creation + idempotent setStackDescription; regression test added), and website-deploy-preview now passes. The sole remaining CI blocker is Acceptance Tests (windows): FAILURE — the cause needs triage before merge. Three medium-priority coverage gaps carry over from Round 3: packer/ansible description tests in pkg/describe, a description assertion in TestExecuteDescribeStacks_IncludeEmptyStacks, and an incomplete section-exclusion check in TestDescribeStacksDescriptionSectionFilter. A new low-severity YAML defect was introduced: authors.yml contains a duplicate atmos: key (two identical entries). Do not merge until the Windows acceptance failure is investigated and confirmed pre-existing or fixed.


2. Findings Table

# Issue Severity Pre-existing? Suggested Fix
1 Acceptance Tests (windows): FAILURE — only remaining CI blocker; cause unknown (pre-existing flaky or regression from this PR) 🔴 Unknown Inspect the failing Windows run log; if pre-existing, document and add to known-flaky list; if new, identify and fix the failing test
2 No packer/ansible description coverage in pkg/describeTestExecuteDescribeStacks_Packer and TestExecuteDescribeStacks_Ansible exist in internal/exec but neither asserts the description field; pkg/describe/describe_stacks_test.go has no packer/ansible description tests at all (Round 2/3 carry) 🟠 No Add TestDescribeStacksPackerComponentDescription and TestDescribeStacksAnsibleComponentDescription in pkg/describe/describe_stacks_test.go using existing packer/ansible fixtures; or extend the existing TestExecuteDescribeStacks_Packer/Ansible tests to assert metadata.description propagates
3 TestDescribeStacksDescriptionSectionFilter incomplete — only asserts vars is absent; settings, env, metadata, workspace, backend, backend_type, inheritance, and providers are unchecked (Round 3 carry) 🟠 No Add assert.False(t, has, ...) for each standard section name that must be excluded when --sections description is the only filter
4 TestExecuteDescribeStacks_IncludeEmptyStacks has no description assertion — only checks count ≥ baseline; the regression test TestProcessStackFile_DescriptionOnPreCreatedEntry covers the unit path but there is no integration-level assertion that the description actually appears in the returned map (Round 3 carry) 🟠 No Add metadata.description to the authmanager-propagation fixture (or a second fixture), then assert stackEntry["description"] equals the expected value inside TestExecuteDescribeStacks_IncludeEmptyStacks
5 Duplicate atmos: key in website/blog/authors.yml — the file defines the atmos: key twice (lines ~1–6 and ~8–13 of the block); duplicate YAML keys are technically invalid (RFC disallows them); most parsers silently take the last value, which is identical here but fragile if either entry diverges 🟡 No Remove the duplicate entry; retain one atmos: block and optionally remove the # Default atmos author comment
6 Fixture-coupled description stringsTestDescribeStacksDescriptionSectionFilter, TestDescribeStacksComponentDescription, TestDescribeStacksHelmfileComponentDescription, and TestDescribeStacksStackDescription all hard-code description strings (e.g., "Tenant1 US East 2 development stack.") that will silently break if fixture YAML is edited (Round 3 carry) 🟡 No Either read expected value from the fixture YAML at test setup, or add a comment explicitly marking the string as fixture-bound with a pointer to the source file
7 stackManifestDescription ~708-line declaration-to-use distance in ProcessStackConfig — declared at line ~62 in stack_processor_process_stacks.go, used at line ~766; separate from the now-local stackDescription in processStackFile (Round 3 carry) 🟡 No Move the declaration immediately above its use site at line ~766, or extract a getStackManifestDescription(config map[string]any) string helper
8 TestSetStackDescription missing "absent stackName" subtest — no subtest covers passing a stackName that does not exist as a key in finalStacksMap; the zero-value nil cast .(map[string]any) silently no-ops, but this branch is unverified (Round 3 carry) 🟡 No Add t.Run("missing stack key – no-op", ...) with a stackName not present in finalMap; assert no panic and no description set
9 No multi-stack CLI test for description without --stack filter — the golden snapshot covers only tenant1-ue2-dev; behavior when multiple stacks are returned is untested at the CLI layer (Round 3 carry) 🟡 No Add a second CLI test entry (or extend snapshot) without --stack to verify per-stack descriptions appear correctly across multiple stacks
10 TestDescribeStacksDescriptionSectionFilter does not assert stack-level description survives the section filter on the component map — it checks the top-level stack description and the component description, but does not verify that sections absent at the stack level (e.g., stack-level vars) are also excluded 🟡 No Add _, hasStackVars := stackMap["vars"]; assert.False(t, hasStackVars, ...) alongside the component-level assertion
11 Blog post date 2026-03-15 remains 12 days backdated at time of this audit (2026-03-27); will be further backdated on merge 🟢 No Update to actual merge/publish date
12 TestExecuteDescribeStacks_Packer and TestExecuteDescribeStacks_Ansible use os.Unsetenv directly instead of t.Setenv / t.Cleanup pattern — env not restored if test panics (pre-existing in test file, but this PR adds description responsibility to the same code paths) 🟢 Yes Follow-up refactor; replace bare os.Unsetenv with t.Setenv / t.Cleanup(func() { os.Unsetenv(...) })
13 No CHANGELOG.md section — only roadmap.js was updated; teams tracking conventional CHANGELOG will miss this feature (Round 3 carry) 🟢 No Add entry under Unreleased in CHANGELOG.md
14 //nolint:revive // cyclomatic on processStackFile — pre-existing debt; continues to grow 🟢 Yes Track in a follow-up issue
15 setStackDescription first-wins semantic undocumented at the call sites — both call sites in processStackFile and processComponentEntry rely on idempotency, but the comment on the processComponentEntry call site is the only one explaining the intent 🟢 No Add a short comment at the includeEmptyStacks call site (line ~128) noting "first-wins: noop if already set" so the two-call pattern is self-documenting

3. Scores Table

Score Number Grade Delta vs. Round 3
Merge Safety 78/100 C+ +10 (includeEmptyStacks bug fixed, website fixed; Windows acceptance failure blocks merge)
Test Coverage 77/100 C+ +3 (regression test added; packer/ansible in pkg/describe + IncludeEmptyStacks description assertion still missing)
Code Quality 82/100 B- +8 (idempotent design is clean; duplicate atmos: YAML key introduced)
Security Posture 90/100 A- 0 (no change)

4. Top 5 Copilot Action Items

  1. Triage Acceptance Tests (windows): FAILURE — inspect the failing run log to determine if this is a pre-existing flake or a regression introduced by this PR; if new, fix it before merge; if confirmed pre-existing, document it.
  2. Remove duplicate atmos: key from website/blog/authors.yml — delete the second identical atmos: block (lines ~8–13); the file currently has two entries with the same key which is invalid YAML.
  3. Add packer and ansible description tests in pkg/describe — add TestDescribeStacksPackerComponentDescription and TestDescribeStacksAnsibleComponentDescription (or extend existing TestExecuteDescribeStacks_Packer/Ansible) using fixtures to assert description propagates through the packer and ansible component type loops.
  4. Add description assertion to TestExecuteDescribeStacks_IncludeEmptyStacks — add metadata.description to the authmanager-propagation fixture and assert stackEntry["description"] equals the expected value; this closes the integration-level gap that TestProcessStackFile_DescriptionOnPreCreatedEntry does not cover.
  5. Extend TestDescribeStacksDescriptionSectionFilter — after the existing hasVars assertion, add assert.False checks for settings, env, metadata, workspace, backend, backend_type, inheritance, and providers so the section-filter contract is fully verified.

5. Suggested Fix Prompt

Fix the following issues in PR `#2200` (feat: add `description` field to components and stacks via `metadata.description`).
Apply in order of severity.

═══════════════════════════════════════════════
1. [CRITICAL – CI BLOCKER] Acceptance Tests (windows): FAILURE
═══════════════════════════════════════════════
Inspect the failing Windows acceptance test run log to determine root cause.
- If the failure is pre-existing (exists on main branch too), add the test name to the
  known-flaky list and document it in a GitHub issue for follow-up.
- If the failure is new (only on this PR's branch), identify which test is failing and
  fix it before merge.

═══════════════════════════════════════════════
2. [LOW – YAML DEFECT] website/blog/authors.yml duplicate atmos: key
═══════════════════════════════════════════════
Remove the duplicate `atmos:` block from `website/blog/authors.yml`. The file currently
defines the key twice (both identical). Keep only one:

  atmos:
    name: Atmos Team
    title: Atmos Team
    url: https://github.com/cloudposse/atmos
    image_url: https://github.com/cloudposse/atmos/raw/main/docs/img/atmos-logo-128.svg

Also remove the comment `# Default atmos author - replace with actual contributors`
if it is no longer accurate.

═══════════════════════════════════════════════
3. [HIGH – COVERAGE] Add packer and ansible description tests in pkg/describe
═══════════════════════════════════════════════
In `pkg/describe/describe_stacks_test.go`, add:

  func TestDescribeStacksPackerComponentDescription(t *testing.T) {
      // Initialize atmosConfig pointing at tests/fixtures/scenarios/packer
      // Call ExecuteDescribeStacks with componentTypes=["packer"]
      // Assert that the relevant component's "description" field equals the expected string
      // from the fixture (add metadata.description to the packer fixture first if absent)
  }

  func TestDescribeStacksAnsibleComponentDescription(t *testing.T) {
      // Same pattern for ansible using examples/demo-ansible or a dedicated fixture
  }

Add `metadata.description` to the relevant packer and ansible fixture components before
writing the assertions.

═══════════════════════════════════════════════
4. [HIGH – COVERAGE] Add description assertion to TestExecuteDescribeStacks_IncludeEmptyStacks
═══════════════════════════════════════════════
In `tests/fixtures/scenarios/authmanager-propagation/` (or whichever stack file is used
by the include-empty-stacks fixture), add:

  metadata:
    description: "Test empty stack description."

Then in `internal/exec/describe_stacks_test.go`, inside `TestExecuteDescribeStacks_IncludeEmptyStacks`,
add after the count assertion:

  // Assert that stack-level description is propagated for includeEmptyStacks path.
  for stackName, stackVal := range withEmpty {
      if stackEntry, ok := stackVal.(map[string]any); ok {
          if desc, exists := stackEntry["description"]; exists {
              assert.Equal(t, "Test empty stack description.", desc,
                  "stack %q should carry its description when includeEmptyStacks=true", stackName)
              break // found at least one stack with the description
          }
      }
  }

═══════════════════════════════════════════════
5. [MEDIUM – TEST COMPLETENESS] Extend TestDescribeStacksDescriptionSectionFilter
═══════════════════════════════════════════════
After the existing `assert.False(t, hasVars, ...)` line in `TestDescribeStacksDescriptionSectionFilter`,
add assertions for all other standard sections:

  for _, absent := range []string{
      "settings", "env", "metadata", "workspace",
      "backend", "backend_type", "inheritance", "providers",
  } {
      _, has := component[absent]
      assert.False(t, has,
          "section %q should not be present when filtering by --sections description only", absent)
  }
  // Also verify stack-level non-description keys are absent:
  _, hasStackVars := stackMap["vars"]
  assert.False(t, hasStackVars, "stack-level vars should not be present when filtering by description only")

🧠 Learnings used
Learnt from: nitrocode
Repo: cloudposse/atmos PR: 0
File: :0-0
Timestamp: 2026-03-23T04:53:54.115Z
Learning: When nitrocode requests an audit on cloudposse/atmos PRs, respond with: (1) a 3–5 sentence Executive Summary, (2) a Findings table with columns “Number # | Issue | Severity | Pre-existing? | Suggested Fix” using 🔴/🟠/🟡/🟢, (3) a Scores table for Merge Safety, Test Coverage, Code Quality, Security Posture with numeric score, letter grade, and delta vs last audit, (4) Top 3 Copilot Action Items, and (5) a copyable “Suggested fix prompt” block.

Learnt from: nitrocode
Repo: cloudposse/atmos PR: 0
File: :0-0
Timestamp: 2026-03-23T05:23:10.817Z
Learning: When auditing cloudposse/atmos PRs on request from nitrocode, the expected output format is: (1) 3–5 sentence Executive Summary, (2) Findings Table with columns “Number # | Issue | Severity | Pre-existing? | Suggested Fix” using 🔴/🟠/🟡/🟢, (3) Scores Table for Merge Safety, Test Coverage, Code Quality, Security Posture with numeric score, letter grade, and delta vs last audit, (4) Top 3 Copilot Action Items, and (5) a copyable “Suggested fix prompt” block.

Learnt from: Cerebrovinny
Repo: cloudposse/atmos PR: 764
File: internal/exec/describe_stacks.go:289-295
Timestamp: 2024-11-13T21:37:07.852Z
Learning: In the `internal/exec/describe_stacks.go` file of the `atmos` project written in Go, avoid extracting the stack name handling logic into a helper function within the `ExecuteDescribeStacks` method, even if the logic appears duplicated.

Learnt from: nitrocode
Repo: cloudposse/atmos PR: 0
File: :0-0
Timestamp: 2026-03-24T03:03:32.815Z
Learning: When nitrocode requests an audit on cloudposse/atmos PRs, include Top 5 Copilot Action Items (not Top 3) along with Executive Summary, Findings Table, Scores Table, and a copyable Suggested fix prompt.

Learnt from: osterman
Repo: cloudposse/atmos PR: 1874
File: internal/exec/describe_affected_utils_test.go:436-468
Timestamp: 2025-12-17T20:55:47.884Z
Learning: In the Atmos codebase, there are two different paths for the `locked` flag: (1) filtering logic in `internal/exec/component_utils.go` (`isComponentLocked()`) reads from `componentSection["metadata"]["locked"]` to determine which components to include/exclude, and (2) extraction/rendering logic in `pkg/list/extract/affected.go` reads from `settings.metadata.locked` to display the locked status in output. Tests for filtering behavior should use `metadata.locked`.

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: nitrocode
Repo: cloudposse/atmos PR: 0
File: :0-0
Timestamp: 2026-03-22T18:02:49.053Z
Learning: In cloudposse/atmos lint stacks, current behavior when --stack has no matching raw manifest is to fall back to repo-scope; this can produce orphan noise. Prefer failing closed or requiring explicit opt-in to repo-scope for L-07 when no seed is found.

Learnt from: osterman
Repo: cloudposse/atmos PR: 808
File: pkg/config/config.go:478-483
Timestamp: 2024-12-02T21:26:32.337Z
Learning: In the 'atmos' project, when reviewing Go code like `pkg/config/config.go`, avoid suggesting file size checks after downloading remote configs if such checks aren't implemented elsewhere in the codebase.

Learnt from: aknysh
Repo: cloudposse/atmos PR: 2161
File: pkg/describe/component_processor.go:159-161
Timestamp: 2026-03-10T20:00:48.986Z
Learning: In `pkg/describe/component_processor.go` (`ProcessComponentFromContext`), the `ProcessTmpl` call that renders `stackNameTemplate` (e.g. `{{ .vars.environment }}-{{ .vars.stage }}` → `dev-us-east-1`) is mandatory infrastructure for computing the stack name from context variables. It is completely separate from the `WithProcessTemplates` flag, which only controls Go template resolution inside component body/vars downstream in `ExecuteDescribeComponent`. The stack-name rendering is unconditional and must not be gated by the `WithProcessTemplates` option.

Learnt from: nitrocode
Repo: cloudposse/atmos PR: 0
File: :0-0
Timestamp: 2026-03-22T17:40:43.317Z
Learning: In cloudposse/atmos lint stacks: (1) severity overrides are normalized/validated in-engine; invalid values are ignored; (2) orphan detection L-07 is scoped via a BFS from seed raw manifests when --stack is set; if no seed matches, current code falls back to repo scope; (3) disabled_rules is not supported by design (documented in cmd/lint/lint.go); use severity demotion instead.

Learnt from: nitrocode
Repo: cloudposse/atmos PR: 0
File: :0-0
Timestamp: 2026-03-19T16:14:01.659Z
Learning: In cloudposse/atmos PR `#1956`, the feature-flags PRD (docs/prd/feature-flags.md) uses `features:` as a dual-semantic key: in stack files it is an activation list (list of strings/maps), but in feature files it is a context key-value map providing `.features.*` template variables. This dual semantics was flagged as a critical weakness.

…n tests

- Add windowsTerraformWait() helper in yaml_func_terraform_output_test.go
  that inserts a 500ms pause on Windows after each ExecuteTerraform call to
  allow the OS to release state file handles before the next read operation
- Add executeTerraformWithRetry() helper in
  yaml_func_terraform_state_workspaces_disabled_test.go that retries the deploy
  up to 3 times with 500ms delays on Windows to handle transient state lock errors
- Use executeTerraformWithRetry in TestWorkspacesDisabledStateLocation

All tests in the package share the same mock-component directory
(tests/fixtures/components/terraform/mock), so Windows file-locking semantics
can cause TestYamlFuncTerraformOutput and TestWorkspacesDisabledStateLocation
to fail with "file locked by another process" when Terraform processes from
prior tests still hold handles on the state file.

Agent-Logs-Url: https://github.com/cloudposse/atmos/sessions/67ad4c8f-fd75-4853-ad15-cfb55b25993a

Co-authored-by: nitrocode <7775707+nitrocode@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Mar 27, 2026

@copilot fix tests

Fixed in 3e6f7ae. The Windows acceptance tests (TestYamlFuncTerraformOutput and TestWorkspacesDisabledStateLocation) were failing with "file locked by another process" — all three yaml-function test scenarios share tests/fixtures/components/terraform/mock, and on Windows, Terraform subprocesses briefly retain file locks on state files after exiting. Added windowsTerraformWait() (500ms pause on Windows after each ExecuteTerraform call) in the output test, and executeTerraformWithRetry() (3 attempts × 500ms backoff on Windows) for the workspace-disabled state location test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

minor New features that do not break anything needs-cloudposse Needs Cloud Posse assistance size/m Medium size PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants