Skip to content

fix: Fixing --queue-construct-as resulting in empty tokenization - #6720

Open
yhakbar wants to merge 1 commit into
mainfrom
fix/queue-construct-as-empty-tokenization
Open

fix: Fixing --queue-construct-as resulting in empty tokenization#6720
yhakbar wants to merge 1 commit into
mainfrom
fix/queue-construct-as-empty-tokenization

Conversation

@yhakbar

@yhakbar yhakbar commented Aug 21, 2026

Copy link
Copy Markdown
Collaborator

Description

Precents --as values that result in empty parsed cmd args from causing a panic.

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.
  • Update the changelog in the docs.
  • Run the relevant tests successfully, including pre-commit checks.
  • This change is backwards compatible.
  • If this change is not forwards compatible (e.g. a new feature), it is gated behind a feature flag.

Summary by CodeRabbit

  • Bug Fixes

    • find and list now reject queue construction values that do not include a command, including empty or shell-only values.
    • Error messages identify the invalid value and show the expected option format.
    • Valid commands with arguments continue to work as expected.
  • Documentation

    • Added a changelog entry documenting this validation improvement.

@vercel

vercel Bot commented Aug 21, 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 Aug 21, 2026 7:23pm

Request Review

@coderabbitai

coderabbitai Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

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: 3d854fdf-0e31-4bec-ba48-1a4fce5d32fc

📥 Commits

Reviewing files that changed from the base of the PR and between 286a33c and 2324e63.

📒 Files selected for processing (4)
  • docs/src/data/changelog/v1.1.4/queue-construct-as-empty-value.mdx
  • internal/discovery/constructor.go
  • internal/discovery/constructor_test.go
  • internal/discovery/errors.go

Included review availability: 1 review is currently available. Your included PR review attempts over the past 7 days set your current allowance at 5 reviews per hour.


📝 Walkthrough

Walkthrough

The discovery constructor now rejects QueueConstructAs values that contain no command. It returns a dedicated error with the supplied value. Tests cover shell punctuation, whitespace, quoted empty strings, valid arguments, and unbalanced quotes. The v1.1.4 changelog documents the fix.

Changes

Queue construct validation

Layer / File(s) Summary
Empty-value error contract
internal/discovery/errors.go
Adds EmptyQueueConstructAsError, its Value field, formatted error message, and constructor.
Discovery command validation and coverage
internal/discovery/constructor.go, internal/discovery/constructor_test.go, docs/src/data/changelog/v1.1.4/queue-construct-as-empty-value.mdx
Rejects parsed values with no command before command extraction. Tests cover invalid shell inputs, valid commands with arguments, and unbalanced quotes. The changelog documents the behavior.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 2324e

This localized change prevents empty parsed command arguments from causing a panic; no actionable merge-blocking risk remains after normal checks and review.

Suggested reviewers: denis256

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 3 files. (1 skipped: 1 unsupported.) Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the fix for empty tokenization from the --queue-construct-as option.
Description check ✅ Passed The description includes the required sections and explains the panic fix, although the issue placeholder and checklist items remain unchanged.
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 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/queue-construct-as-empty-tokenization

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

@yhakbar
yhakbar force-pushed the fix/queue-construct-as-empty-tokenization branch from 3bec53e to 2324e63 Compare August 21, 2026 19:22
@yhakbar
yhakbar marked this pull request as ready for review August 22, 2026 12:46
@yhakbar
yhakbar requested a review from denis256 as a code owner August 22, 2026 12:46
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.

1 participant