Skip to content

fix: Preventing parse errors in stack generation in worktrees - #5826

Merged
yhakbar merged 4 commits into
mainfrom
fix/fixing-dependency-resolution-in-stacks
Apr 10, 2026
Merged

fix: Preventing parse errors in stack generation in worktrees#5826
yhakbar merged 4 commits into
mainfrom
fix/fixing-dependency-resolution-in-stacks

Conversation

@yhakbar

@yhakbar yhakbar commented Apr 9, 2026

Copy link
Copy Markdown
Collaborator

Description

Right now, users with negations in their filter expressions aren't preventing reading parsing from happening in worktrees during stack generation. This is because we aren't properly propagating the non-Git filters from the parent discovery down to child discoveries.

This fix this by properly propagating the non-Git filters from the parent discovery down to child discoveries.

Relates to recommended approach for handling #5821

TODOs

Read the Gruntwork contribution guidelines.

  • I authored this code entirely myself
  • I am submitting code based on open source software (e.g. MIT, MPL-2.0, Apache)]
  • I am adding or upgrading a dependency or adapted code and confirm it has a compatible open source license
  • Update the docs.
  • Run the relevant tests successfully, including pre-commit checks.
  • Include release notes. If this PR is backward incompatible, include a migration guide.

Release Notes (draft)

Added / Removed / Updated [X].

Migration Guide

Summary by CodeRabbit

  • Bug Fixes
    • Filter exclusions are now properly applied during worktree-based filtering operations.
    • Negated filters are now applied consistently in sub-discovery operations.
    • Improved stability by preventing parse errors from excluded catalog units.

@vercel

vercel Bot commented Apr 9, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
terragrunt-docs Ready Ready Preview, Comment Apr 9, 2026 7:21pm

Request Review

@coderabbitai

coderabbitai Bot commented Apr 9, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: b4dc8051-de1d-446d-93c1-740e29ffd467

📥 Commits

Reviewing files that changed from the base of the PR and between 9b8da9b and 5664cc9.

📒 Files selected for processing (6)
  • docs/src/data/changelog/v1.0.1.mdx
  • internal/discovery/phase_worktree.go
  • internal/discovery/phase_worktree_integration_test.go
  • internal/filter/filters.go
  • internal/filter/filters_test.go
  • internal/stacks/generate/generate.go

📝 Walkthrough

Walkthrough

Filter exclusions, including negated filters from configuration and command-line arguments, are now propagated into git worktree sub-discoveries used for git-based filtering modes. Previously excluded catalog units inside worktrees are no longer discovered, preventing parse errors caused by missing stack generation context.

Changes

Cohort / File(s) Summary
Documentation
docs/src/data/changelog/v1.0.1.mdx
Added changelog entry documenting filter exclusion propagation into worktree sub-discoveries and the elimination of parse/evaluation errors for excluded units.
Worktree Discovery Logic
internal/discovery/phase_worktree.go
Modified discoverInWorktree to concatenate parent discovery filters with existing filters; updated walkChangedStack to use non-git filters from parent discovery instead of empty filter set for both fromDiscovery and toDiscovery sub-discoveries.
Filter Core
internal/filter/filters.go
Added ExcludingGitFilters() method to remove filters containing git expressions; refactored RestrictToStacks() logic; repositioned internal helper functions for clearer code organization.
Filter Tests
internal/filter/filters_test.go
Added comprehensive test suite TestFilters_ExcludingGitFilters covering empty filters, git-only filters, non-git filters, mixed scenarios, and mutation validation.
Worktree Integration Tests
internal/discovery/phase_worktree_integration_test.go
Added three integration tests validating negated filter application in worktree sub-discoveries, exclusion of land-mine units via git filters, and stack discovery without parsing non-stack units.
Stack Generation
internal/stacks/generate/generate.go
Refactored discoverStacks to accept a readFiles bool flag instead of additional filters; updated worktreeStacksToGenerate to conditionally enable file-reading parsing for worktree discovery.

Sequence Diagram(s)

sequenceDiagram
    participant Parent as Parent Discovery
    participant Filter as Filter Component
    participant Worktree as Worktree Sub-Discovery
    participant Parser as Unit Parser
    
    Parent->>Filter: Collect all filters (including negations)
    Parent->>Filter: Extract non-git filters via ExcludingGitFilters()
    Filter-->>Parent: Return non-git filters
    Parent->>Worktree: Pass concatenated filters<br/>(parent non-git + worktree-specific)
    Worktree->>Worktree: Apply combined filters to units
    alt Unit matches negation filter
        Worktree-->>Parser: Skip unit (excluded)
    else Unit passes filters
        Worktree->>Parser: Process unit
        Parser-->>Worktree: Return parsed unit
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 46.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main fix: preventing parse errors in stack generation within worktrees by propagating filters.
Description check ✅ Passed The description follows the template structure with relevant sections filled out, though release notes placeholder text remains generic.

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

✨ 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 fix/fixing-dependency-resolution-in-stacks

Warning

Review ran into problems

🔥 Problems

Timed out fetching pipeline failures after 30000ms


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

yhakbar added 2 commits April 9, 2026 14:28
Right now, users with negations in their filter expressions aren't
preventing reading parsing from happening in worktrees during stack
generation. This is because we aren't properly propagating the non-Git
filters from the parent discovery down to child discoveries.

This fix this by properly propagating the non-Git filters from the
parent discovery down to child discoveries.
@yhakbar
yhakbar force-pushed the fix/fixing-dependency-resolution-in-stacks branch from 1282179 to 20a0ad7 Compare April 9, 2026 18:28
@yhakbar
yhakbar marked this pull request as ready for review April 9, 2026 20:45
return refs
}

func collectWorktreeExpressions(expr Expression) []*GitExpression {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

containsGitExpression and collectWorktreeExpressions share a lot of the same logic - can be room for deduplication

@yhakbar
yhakbar merged commit 98bcfda into main Apr 10, 2026
81 of 82 checks passed
@yhakbar
yhakbar deleted the fix/fixing-dependency-resolution-in-stacks branch April 10, 2026 14:50
denis256 pushed a commit that referenced this pull request Apr 14, 2026
* fix: Preventing parse errors in stack generation in worktrees

Right now, users with negations in their filter expressions aren't
preventing reading parsing from happening in worktrees during stack
generation. This is because we aren't properly propagating the non-Git
filters from the parent discovery down to child discoveries.

This fix this by properly propagating the non-Git filters from the
parent discovery down to child discoveries.

* fix: Adding some more testing for this

* docs: Documenting fix

* fix: Addressing lints
yhakbar added a commit that referenced this pull request Apr 15, 2026
* chore: coverage scripts collection simplifications

* chore: simplified code coverage scripts

* chore: extracted common scripts in github action

* chore: download cleanup

* chore: set threshold to report coverage

* chore: pr comments

* docs: Adding a changelog (#5754)

* docs: Updating docs dependencies

* docs: Adding changelog in docs

* docs: Fixing some styling

* docs: Adding prior releases section

* docs: Fixing links

* docs: Adding changelog copy button (#5755)

* docs: Updating docs dependencies

* docs: Adding changelog in docs

* docs: Fixing some styling

* docs: Adding prior releases section

* docs: Fixing links

* docs: Adding changelog copy button

* fix: Adding support for 'Unreleased'

* docs: Adding more `v1.0.0` changelog docs (#5756)

* chore: Adding tests to confirm #4395 is resolved (#5761)

* fix: Fixing #5624 (#5766)

* fix: Resetting config specific configurations when reading file

* fix: Adjusting guards for dependency resolution

* docs: Adding `v1.0.0` call out (#5768)

* fix: add retry patterns for transient provider/registry network errors (#5779)

* chore: updated retry patterns to reduce failing CICD tests

* chore: changelog update

* fix: Adding Windows symlink tip (#5778)

* fix: Adding Windows Symlink tip

* feat: Making tips adjustable

* chore: Better error message for OpenTofu

* fix: Reducing reliance on `opts`

* fix: Addressing `lll` findings in relevant code nearby

* chore: Using dependency injection to avoid the need to have a real Windows environment to test this

* fix: Cleanup of messages

* docs: Adding changelog entry for the tip

* chore: default error detection simplification

* chore: PR comments

* vfs cleanup

* cleanup

* cleanup

* docs cleanup

* docs cleanup

* cleanup

---------

Co-authored-by: Yousif Akbar <11247449+yhakbar@users.noreply.github.com>

* chore: Replace `runtime.NumCPU()` usage with `runtime.GOMAXPROCS(0)` (#5794)

* chore: Replace `runtime.NumCPU()` usage with `runtime.GOMAXPROCS(0)`

* docs: documenting logic for limits

* docs: Adding changelog entry for runtime.NumCPU --> runtime.GOMAXPROCS update

* various lint fixes (#5796)

* update uses of os.IsNotExist to errors.Is

* octal perms lint

* lint fixes

* fix calls to NewWorkTrees to match new signature

* futher lints caused by errors.Is update

* lint fixes

---------

Co-authored-by: Denis O <denis.o@linux.com>

* feat: add stack dependencies experiment (#5809)

* feat: add stack dependencies experiment

* docs: added basic changelog docs

* docs: data update

* docs update

* docs: updated implementation plan

* chore: Adding tip build workflows (#5823)

* chore: Adding tip build workflows

* docs: Documenting tip builds

* docs: Cleaning up tip build docs

* fix: Fixing script

* fix: Addressing review feedback

* fix: Cleaning up workflow scripts

* docs: Clarifying codesign status

* docs: Documenting tip/test build installation instructions (#5829)

* fix: Fixing `get_original_terragrunt_dir()` interaction with dependencies (#5828)

* fix: Fixing `get_original_terragrunt_dir()`

* fix: Using better designed reset for dependency config path

* fix: Better godocs

* docs: Documenting fix in changelog

* fix: Using cty.DynamicVal to avoid 'Unsupported Attribute' errors (#5827)

* fix: Using cty.DynamicVal to avoid 'Unsupported Attribute' errors

* fix: Adding more safety checks for dynamic values

* docs: Documenting fix for `hcl validate`

* fix: Addressing review feedback

* fix: Preventing parse errors in stack generation in worktrees (#5826)

* fix: Preventing parse errors in stack generation in worktrees

Right now, users with negations in their filter expressions aren't
preventing reading parsing from happening in worktrees during stack
generation. This is because we aren't properly propagating the non-Git
filters from the parent discovery down to child discoveries.

This fix this by properly propagating the non-Git filters from the
parent discovery down to child discoveries.

* fix: Adding some more testing for this

* docs: Documenting fix

* fix: Addressing lints

* docs: Adding TGS docs (#5831)

* docs: Adding TGS docs

* chore: Adding changelog entry

* fix: Suppressing errors on Discord link

* fix: Addressing review feedback

* chore: Upgrading go deps (#5795)

* chore: Upgrading go deps

* fix: Addressing lints

* fix: Fixing adapter

* fix: Addressing lints

* fix: Getting rid of teatest

* fix: Addressing regressions from charm updates

* fix: Disable git autoclrf to avoid issues with GOPROXY=direct

* fix: Disable lfs for no proxy build

* fix: Addressing #5828 feedback (#5876)

* misc testing and lint updates (#5885)

* disable gpg signing in repos made during tests

* remove unnecessary exist check from delete helpers

* remove redundant delete helpers, use helpers package

* lint

* fix: Fixing up lints (#5887)

* chore: add 7-day cooldown to dependabot updates (#5889)

Delay adoption of new releases across github-actions, gomod, and bun
ecosystems to reduce exposure to supply chain attacks that are
typically detected and yanked within days of publication.

* feat: Adding `catalog-redesign` experiment (#5894)

* chore: Updating install script to support tip builds (#5892)

* chore: Adding tip build scripts to install

* fix: Use jq for more reliable latest tip commit

* chore: Adding tip notification system (#5895)

* chore: simplified returned errors (#5664)

* chore: tmpfs cleanup

* chore: updated alert settings

---------

Co-authored-by: Yousif Akbar <11247449+yhakbar@users.noreply.github.com>
Co-authored-by: Travis Johnson <travis@thisguy.codes>
Co-authored-by: Diógenes Fernandes <diofeher@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants