Repository navigation
Fix helmfile CLI - #8
Merged
Merged
Conversation
Andriy Knysh (aknysh)
requested review from
Nuru (Nuru) and
Erik Osterman (Cloud Posse) (osterman)
November 29, 2020 22:28
Erik Osterman (Cloud Posse) (osterman)
approved these changes
Nov 29, 2020
Erik Osterman (Cloud Posse) (osterman)
added a commit
that referenced
this pull request
Nov 5, 2025
- Changed auth_parser.go to use cmd.Flags().Changed('duration')
- More reliable than checking non-empty string
- Correctly handles --duration='' vs no flag
- Prevents false positives from empty string values
Addresses code review item #8
Erik Osterman (Cloud Posse) (osterman)
added a commit
that referenced
this pull request
Dec 18, 2025
- Fix Comment #6: Update ErrPermissionDenied message to be concise - Fix Comment #7: Update ErrNoComponentsWithTags to mention tags - Fix Comment #8: Wire NoColor from global persistent flags - Fix Comment #9/#14: Replace direct os.Stdout/Stderr with ui/data abstractions - Fix Comment #10: Remove direct viper.BindEnv, use os.LookupEnv for TERM - Fix Comment #11: Use data.Write in writeOutput function - Fix Comment #13: Add Intro component to diff.mdx (replace :::note) - Fix Comment #15: Use ui.Warningf in executeComponentVendorDiff stub - Fix Comment #21/#22: Fix broken documentation links in diff.mdx - Add test I/O initialization for data.Write() and ui.Infof() calls 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Erik Osterman (Cloud Posse) (osterman)
added a commit
that referenced
this pull request
Dec 21, 2025
- Fix Comment #6: Update ErrPermissionDenied message to be concise - Fix Comment #7: Update ErrNoComponentsWithTags to mention tags - Fix Comment #8: Wire NoColor from global persistent flags - Fix Comment #9/#14: Replace direct os.Stdout/Stderr with ui/data abstractions - Fix Comment #10: Remove direct viper.BindEnv, use os.LookupEnv for TERM - Fix Comment #11: Use data.Write in writeOutput function - Fix Comment #13: Add Intro component to diff.mdx (replace :::note) - Fix Comment #15: Use ui.Warningf in executeComponentVendorDiff stub - Fix Comment #21/#22: Fix broken documentation links in diff.mdx - Add test I/O initialization for data.Write() and ui.Infof() calls 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Erik Osterman (Cloud Posse) (osterman)
added a commit
that referenced
this pull request
Dec 26, 2025
- Fix Comment #6: Update ErrPermissionDenied message to be concise - Fix Comment #7: Update ErrNoComponentsWithTags to mention tags - Fix Comment #8: Wire NoColor from global persistent flags - Fix Comment #9/#14: Replace direct os.Stdout/Stderr with ui/data abstractions - Fix Comment #10: Remove direct viper.BindEnv, use os.LookupEnv for TERM - Fix Comment #11: Use data.Write in writeOutput function - Fix Comment #13: Add Intro component to diff.mdx (replace :::note) - Fix Comment #15: Use ui.Warningf in executeComponentVendorDiff stub - Fix Comment #21/#22: Fix broken documentation links in diff.mdx - Add test I/O initialization for data.Write() and ui.Infof() calls 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Erik Osterman (Cloud Posse) (osterman)
added a commit
that referenced
this pull request
Jan 2, 2026
- cmd/auth/shell.go: Use envpkg.MergeGlobalEnv() for consistency with exec.go (addresses CodeRabbit comment #3 about env merging inconsistency) - cmd/auth/whoami.go: Use %w for error wrapping to preserve error chain (addresses CodeRabbit comment #4 about error wrapping) - tests/cli_describe_component_test.go: Use cross-platform TTY detection with term.IsTTYSupportForStdout() and close file handle properly (addresses CodeRabbit comments #5, #6) - tests/describe_test.go: Add skipIfNoTTY helper with cross-platform TTY detection and proper file handle cleanup (addresses CodeRabbit comments #7, #8) Note: Comments #1 and #2 (codeql clear-text logging) are false positives - the atmos auth env command intentionally outputs credentials for shell sourcing, similar to `aws configure export-credentials`. Suppression comments are already in place. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Erik Osterman (Cloud Posse) (osterman)
added a commit
that referenced
this pull request
Jan 4, 2026
- cmd/auth/shell.go: Use envpkg.MergeGlobalEnv() for consistency with exec.go (addresses CodeRabbit comment #3 about env merging inconsistency) - cmd/auth/whoami.go: Use %w for error wrapping to preserve error chain (addresses CodeRabbit comment #4 about error wrapping) - tests/cli_describe_component_test.go: Use cross-platform TTY detection with term.IsTTYSupportForStdout() and close file handle properly (addresses CodeRabbit comments #5, #6) - tests/describe_test.go: Add skipIfNoTTY helper with cross-platform TTY detection and proper file handle cleanup (addresses CodeRabbit comments #7, #8) Note: Comments #1 and #2 (codeql clear-text logging) are false positives - the atmos auth env command intentionally outputs credentials for shell sourcing, similar to `aws configure export-credentials`. Suppression comments are already in place. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Erik Osterman (Cloud Posse) (osterman)
added a commit
that referenced
this pull request
Jan 5, 2026
- cmd/auth/shell.go: Use envpkg.MergeGlobalEnv() for consistency with exec.go (addresses CodeRabbit comment #3 about env merging inconsistency) - cmd/auth/whoami.go: Use %w for error wrapping to preserve error chain (addresses CodeRabbit comment #4 about error wrapping) - tests/cli_describe_component_test.go: Use cross-platform TTY detection with term.IsTTYSupportForStdout() and close file handle properly (addresses CodeRabbit comments #5, #6) - tests/describe_test.go: Add skipIfNoTTY helper with cross-platform TTY detection and proper file handle cleanup (addresses CodeRabbit comments #7, #8) Note: Comments #1 and #2 (codeql clear-text logging) are false positives - the atmos auth env command intentionally outputs credentials for shell sourcing, similar to `aws configure export-credentials`. Suppression comments are already in place. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Erik Osterman (Cloud Posse) (osterman)
added a commit
that referenced
this pull request
Jan 22, 2026
- Add duration overflow guard in ParseDuration (Comment #6) - Fix non-workdir re-provisioning: skip metadata check for non-workdir targets (Comments #7, #11) - Detect version removal: trigger re-provisioning when version is removed (Comments #8, #14) - Fix blog date 2025 → 2026 (Comments #9, #16) - Surface metadata read failures as warnings in ListWorkdirs (Comment #10) - Add periods to comment block in needsProvisioning (Comment #12) - Treat .atmos-only directories as empty in isNonEmptyDir (Comment #13) - Skip .atmos during source walk in syncSourceToDest (Comment #15) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Erik Osterman (Cloud Posse) (osterman)
added a commit
that referenced
this pull request
Jan 23, 2026
- Add duration overflow guard in ParseDuration (Comment #6) - Fix non-workdir re-provisioning: skip metadata check for non-workdir targets (Comments #7, #11) - Detect version removal: trigger re-provisioning when version is removed (Comments #8, #14) - Fix blog date 2025 → 2026 (Comments #9, #16) - Surface metadata read failures as warnings in ListWorkdirs (Comment #10) - Add periods to comment block in needsProvisioning (Comment #12) - Treat .atmos-only directories as empty in isNonEmptyDir (Comment #13) - Skip .atmos during source walk in syncSourceToDest (Comment #15) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Erik Osterman (Cloud Posse) (osterman)
added a commit
that referenced
this pull request
Jan 24, 2026
- Add duration overflow guard in ParseDuration (Comment #6) - Fix non-workdir re-provisioning: skip metadata check for non-workdir targets (Comments #7, #11) - Detect version removal: trigger re-provisioning when version is removed (Comments #8, #14) - Fix blog date 2025 → 2026 (Comments #9, #16) - Surface metadata read failures as warnings in ListWorkdirs (Comment #10) - Add periods to comment block in needsProvisioning (Comment #12) - Treat .atmos-only directories as empty in isNonEmptyDir (Comment #13) - Skip .atmos during source walk in syncSourceToDest (Comment #15) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Andriy Knysh (aknysh)
added a commit
that referenced
this pull request
Feb 5, 2026
…2010) * fix: JIT source provisioning now takes precedence over local components When both source.uri and provision.workdir.enabled are configured on a component, the JIT source provisioner now always runs, even if a local component already exists. This ensures that source + workdir provisioning always vendors from the remote source to the workdir path, respecting the version specified in stack config rather than using a potentially stale local component. Added regression test to verify source provisioning takes precedence when both local component and source config are present. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> * feat: version-aware JIT source provisioning with TTL-based cleanup - Implement intelligent re-provisioning for remote sources based on version/URI changes - Add incremental local sync with per-file checksum comparison using SyncDir - Support TTL-based cleanup for stale workdirs via duration parsing - Move workdir metadata from flat file to .atmos/metadata.json for better organization - Track source_uri, source_version, and last_accessed timestamps - Add new CLI flags: --expired, --ttl, --dry-run for workdir clean command - Update workdir list and show commands with version and access information - Extract duration parsing to new pkg/duration package for reusability Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * refactor: Reduce cyclomatic and cognitive complexity in workdir/source packages - Extract helper functions to reduce function complexity: - duration.go: Use maps for unit multipliers and keywords, extract parseInteger/parseWithSuffix/parseKeyword - provision_hook.go: Extract isNonEmptyDir and checkMetadataChanges - clean.go: Extract checkWorkdirExpiry, getLastAccessedTime, getModTimeFromEntry - fs.go: Extract syncSourceToDest, fileNeedsCopy, deleteRemovedFiles - workdir.go: Extract validateComponentPath, computeContentHash, create localMetadataParams struct - Pass localMetadataParams by pointer to avoid hugeParam warning Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * docs: Update source-provisioning example to use demo-library Replace terraform-null-label (which is a module, not a component) with demo-library components that can actually be run with terraform apply. The example now demonstrates both source types: - weather: LOCAL source (../demo-library/weather) - ipinfo: REMOTE source (github.com/cloudposse/atmos//examples/demo-library/ipinfo) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: Address PR review comments for JIT source provisioning - Add duration overflow guard in ParseDuration (Comment #6) - Fix non-workdir re-provisioning: skip metadata check for non-workdir targets (Comments #7, #11) - Detect version removal: trigger re-provisioning when version is removed (Comments #8, #14) - Fix blog date 2025 → 2026 (Comments #9, #16) - Surface metadata read failures as warnings in ListWorkdirs (Comment #10) - Add periods to comment block in needsProvisioning (Comment #12) - Treat .atmos-only directories as empty in isNonEmptyDir (Comment #13) - Skip .atmos during source walk in syncSourceToDest (Comment #15) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: Update golden snapshot for atmos describe config Add the new provision.workdir section to the expected output, matching the new JIT source provisioning configuration schema. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: Address additional PR review comments - Guard against int64 overflow in parseWithSuffix (Comment #2) - Branch metadata writing by source type - local vs remote (Comment #3) - Add permission checks to fileNeedsCopy for mode changes (Comment #4) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * test: Add tests to improve coverage for workdir and source provisioning Adds comprehensive tests for: - CleanExpiredWorkdirs with mock filesystem - formatDuration for human-readable output - getLastAccessedTime with atime fallback to mtime - checkWorkdirExpiry with valid/corrupt/missing metadata - isLocalSource for local vs remote URI detection Also fixes linter issues: - godot: Fix comment in duration.go - revive: Refactor formatWithOptions to map-based dispatch Addresses CodeRabbit comment #1 requesting improved patch coverage. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: Return wrapped error from ReadMetadata instead of warning Changes error handling in ListWorkdirs to return a wrapped error when ReadMetadata fails, surfacing permission/corruption issues to callers. Directories without metadata (metadata == nil) still skip silently. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * test: Improve test coverage and address CodeRabbit review comments - Add metadata_test.go with tests for UpdateLastAccessed and readMetadataUnlocked - Add buildLocalMetadata tests covering all timestamp preservation branches - Add cleanExpiredWorkdirs and CleanExpiredWorkdirs tests - Fix ListWorkdirs to skip invalid metadata instead of failing entire operation - Fix zero timestamp display to show "-" instead of "0001-01-01" - Fix isLocalSource to use filepath.IsAbs for Windows path support - Fix godot lint issues in log_utils.go Coverage improvements: - pkg/provisioner/workdir: 82.1% -> 88.1% - cmd/terraform/workdir: 58.7% -> 92.2% (function coverage) - UpdateLastAccessed: 0% -> 84.2% - readMetadataUnlocked: 0% -> 100% - buildLocalMetadata: 57% -> 100% - cleanExpiredWorkdirs: 0% -> 100% Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * test: Add JIT source provisioning tests for destroy and init commands Add test coverage to confirm that JIT source provisioning correctly takes precedence over local components for all terraform subcommands, not just plan. These tests verify that when: - source.uri is configured - provision.workdir.enabled: true - A local component exists at components/terraform/<component>/ The workdir is populated from the remote source, NOT copied from the local component. This confirms the fix in ExecuteTerraform() works universally for destroy and init commands. Uses table-driven test pattern to avoid code duplication. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * test: Expand JIT source tests to cover all terraform subcommands Expand table-driven test to verify JIT source provisioning works for all 22 terraform subcommands that operate on a component with a stack: Core execution: apply, deploy, destroy, init, workspace State/resource: console, force-unlock, get, graph, import, output, refresh, show, state, taint, untaint Validation/info: metadata, modules, providers, test, validate All commands correctly trigger JIT source provisioning when: - source.uri is configured - provision.workdir.enabled: true - A local component exists The workdir is populated from remote source, not local component. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * test: Improve test coverage and address CodeRabbit review comments - Make tests fail-fast instead of silently skipping when files don't exist - Verify context.tf exists (proving remote source was used) - Assert main.tf does NOT exist (proving local component wasn't copied) - Remove unused strings import - Update roadmap with JIT source provisioning precedence milestone - Update vendoring initiative progress from 86% to 89% Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: Add JIT source provisioning to generate commands (#2019) - Add JIT source provisioning to terraform generate varfile - Add JIT source provisioning to terraform generate backend - Add JIT source provisioning to helmfile generate varfile - Add JIT source provisioning to packer output - Update golden snapshot for secrets-masking_describe_config test The generate commands were missing JIT source provisioning that exists in ExecuteTerraform(), causing them to fail with JIT-vendored components. This fix adds the same pattern to all affected commands. Closes #2019 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * docs: Add automatic component refresh milestone to roadmap Add new milestone to Vendoring & Resilience initiative: - "Automatic component refresh on version changes" - Links to PR #2010 and version-aware-jit-provisioning blog post - Update progress from 89% to 95% Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * test: Improve test coverage and address CodeRabbit review comments - Add tests for workdir clean command edge cases - Add tests for workdir show command scenarios - Add duration parsing tests for TTL validation - Add filesystem tests for workdir operations - Add metadata lock tests for Unix file locking Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: Windows test compatibility and improve error hint accuracy - Skip permission-based tests on Windows (Unix permissions not supported) - TestFileNeedsCopy_DifferentPermissions - TestCopyFile_PreservesPermissions - TestServiceProvision_WriteMetadataFails (read-only dirs work differently) - Use actual componentPath in error hint instead of hardcoded path Addresses CodeRabbit review feedback. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: Address CodeRabbit review comments - Wrap auto-provision error with ErrSourceProvision sentinel (packer_output.go) - Add error wrapping with ErrWorkdirMetadata in Windows metadata loader - Document circular import limitation preventing cmd.NewTestKit usage Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: Use runtime.GOOS instead of os.Getenv for Windows detection GOOS is a compile-time constant, not a runtime environment variable. os.Getenv("GOOS") returns empty unless explicitly set. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * chore: Ignore flaky kubernetes.io URLs in link checker The kubernetes.io domain frequently has connection failures/timeouts in CI, causing spurious link check failures. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * test: Improve JIT source test assertions with explicit failure Instead of silently passing when main.tf doesn't exist, the tests now: - Explicitly fail if main.tf exists (unexpected) - Read and check for LOCAL_VERSION_MARKER to provide better diagnostics - Use t.Fatalf to fail fast with clear error messages Addresses CodeRabbit feedback about test assertion clarity. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * test: improve coverage for JIT source provisioning Add comprehensive tests for: - pkg/provisioner/workdir/metadata.go: - MetadataPath function - WriteMetadata with all fields populated - ReadMetadata new location priority over legacy - UpdateLastAccessed preserves all fields - pkg/provisioner/workdir/clean.go: - checkWorkdirExpiry for expired/non-expired workdirs - getModTimeFromEntry - findExpiredWorkdirs with mixed workdirs - CleanExpiredWorkdirs with empty base path - Clean with Expired option precedence - formatDuration edge cases - pkg/provisioner/workdir/fs.go: - DefaultPathFilter.Match with patterns - SyncDir with nested directories - SyncDir updating changed files - pkg/provisioner/source/provision_hook.go: - checkMetadataChanges with version scenarios - isNonEmptyDir edge cases - needsProvisioning for non-workdir targets - writeWorkdirMetadata source type detection - writeWorkdirMetadata preserving ContentHash Coverage improvements: - workdir package: ~79% → 92.5% - source package: ~76% → 83.6% Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Haiku 4.5 <noreply@anthropic.com> Co-authored-by: aknysh <andriy.knysh@gmail.com>
3 of 4 tasks
This was referenced Mar 22, 2026
Merged
Open
Copilot AI
added a commit
that referenced
this pull request
Mar 22, 2026
… stacks Critical #1: Fail closed when --stack filter finds no matching raw manifests Critical #2: Normalize non-glob relative imports to absolute paths in buildImportGraph High #3: Use cfg.TerraformSectionName/cfg.HelmfileSectionName in L-05 High #4: Key L-02 baseVars by '<stack>/<component>' to prevent cross-stack collisions High #5: Add test for ATMOS_LINT_RULE env binding in cmd/lint High #6: Build StackNameToFileIndex in LintStacks for reliable L-08 file attribution Medium #7: Add CohesionMaxGroups to schema for configurable L-05 threshold Medium #8: Add golden test for rulesRelNorm consistency with L-07 relNorm Medium #9: Clarify L-03 depth semantics (node-count vs edge-count) in description Low #12: Add ui.Error call before returning from renderLintJSON error path Update authors.yml to fix duplicate nitrocode entry (RB, CEO @ Infralicious) Co-authored-by: nitrocode <7775707+nitrocode@users.noreply.github.com> Agent-Logs-Url: https://github.com/cloudposse/atmos/sessions/9708e2c0-9c09-48c8-a447-323fe5ad065d
Andriy Knysh (aknysh)
added a commit
that referenced
this pull request
May 12, 2026
…1919) * Fix markdown code fence in Nerd Fonts installation instructions (#1917) * Simplify Nerd Fonts installation instructions Removed Homebrew tap and search commands from installation instructions. Cask-fonts has been deprecated. * Fix markdown code fence formatting The previous commit accidentally removed the opening code fence when deleting the deprecated Homebrew tap commands. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> --------- Co-authored-by: Matt Topper <matt.topper@gmail.com> Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com> Co-authored-by: Andriy Knysh <aknysh@users.noreply.github.com> * Address PR review comments for auth registry refactor - Fix Comment #3: Parse global flags before InitCliConfig in all auth commands by adding BuildConfigAndStacksInfo helper to pkg/flags and cmd/auth/helpers.go - Fix Comment #4: Add guard for empty selectable array in configure.go to prevent index out of bounds when no AWS user identities found - Fix Comment #5: Remove duplicate IdentityFlagName constants by using cfg.IdentityFlagName from pkg/config/const.go as canonical source - Remove unused schema imports from login.go, exec.go, shell.go - Remove unnecessary nolint:gosec directives from env.go 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Fix NoOptDefVal preprocessing for identity flag in auth commands The identity flag uses Cobra's NoOptDefVal feature which only works with equals syntax (--identity=value), not space-separated syntax (--identity value). This caused explicit identity values to be ignored, falling back to interactive selection which fails without a TTY. Fix by adding NoOptDefVal preprocessing in preprocessCompatibilityFlags() to rewrite --identity value → --identity=value before Cobra parses. This ensures explicit identity values work correctly in all environments (CI, piped output, redirected stdin). Fixes: - TestInteractiveIdentitySelection/explicit_identity_value_should_work_even_with_piped_output - TestExplicitIdentityAlwaysWorks/with_CI=true 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Add documentation for --identity flag placement best practices Document recommended usage patterns for the --identity flag: - Use equals syntax (--identity=admin) for clarity - Place flag before -- separator and positional arguments - Both auth and terraform command docs updated 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Address PR review comments: constants and security annotations 1. Remove redundant constant aliases in cmd/identity_helpers.go - Use cfg.IdentityFlagName and cfg.IdentityFlagSelectValue directly - Eliminates duplicate definitions (CodeRabbit feedback) 2. Add CodeQL suppression comments in cmd/auth/env.go - Document intentional credential output for shell sourcing - Add codeql[go/clear-text-logging] annotations - Similar to aws configure export-credentials behavior 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Add --identity flag documentation to global flags reference Document the --identity flag in the Command-Specific Flags section: - Available in auth, terraform, and describe commands - Supports multiple modes: explicit value, interactive selector, disabled - Add ATMOS_IDENTITY environment variable reference - Include flag placement best practice tip (equals syntax recommended) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Refactor preprocessing architecture with preprocessArgs orchestrator Separate NoOptDefVal preprocessing and compatibility flag translation into sibling operations orchestrated by a new preprocessArgs() function. Architecture: - preprocessArgs() orchestrates all argument preprocessing - preprocessNoOptDefValFlags() rewrites --flag value → --flag=value for NoOptDefVal flags - preprocessCompatibilityFlags() separates Atmos flags from pass-through flags Key fixes: - Call SetArgs when NoOptDefVal changes args but no compat flags exist - This ensures --identity value works correctly for auth commands New pkg/flags/preprocess/ package: - Pipeline interface for extensible preprocessing - NoOptDefValPreprocessor with fixed hasSeparatedValue() using strings.Contains - FlagInfo interface to avoid circular imports Tests: - All auth tests pass (29 tests) - All preprocess package tests pass - Pager tests skip gracefully when TTY unavailable Also fixes pre-existing lint errors in: - errors/errors.go: Add ErrComponentPathNotFound sentinel - internal/exec/helmfile.go: Use sentinel errors instead of dynamic errors - internal/exec/stack_processor_merge.go: Use %w instead of %v for errors - pkg/downloader/file_downloader.go: Use %w instead of %v for errors - internal/exec/path_utils_test.go: Use constants for paths with separators 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Address PR review comments - cmd/auth/shell.go: Use envpkg.MergeGlobalEnv() for consistency with exec.go (addresses CodeRabbit comment #3 about env merging inconsistency) - cmd/auth/whoami.go: Use %w for error wrapping to preserve error chain (addresses CodeRabbit comment #4 about error wrapping) - tests/cli_describe_component_test.go: Use cross-platform TTY detection with term.IsTTYSupportForStdout() and close file handle properly (addresses CodeRabbit comments #5, #6) - tests/describe_test.go: Add skipIfNoTTY helper with cross-platform TTY detection and proper file handle cleanup (addresses CodeRabbit comments #7, #8) Note: Comments #1 and #2 (codeql clear-text logging) are false positives - the atmos auth env command intentionally outputs credentials for shell sourcing, similar to `aws configure export-credentials`. Suppression comments are already in place. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Add unit tests for cmd/auth package to improve coverage Add 15 new test files covering pure functions and mock-based tests: - helpers_test.go: formatDuration, displayAuthSuccess, BuildConfigAndStacksInfo - whoami_test.go: redactHomeDir, sanitizeEnvMap, buildWhoamiTableRows, validateCredentials - list_test.go: parseCommaSeparatedNames, filter functions, render functions - env_test.go: outputEnvAsExport, outputEnvAsDotenv - login_test.go: authenticateIdentity with MockAuthManager - shell_test.go: getSeparatedArgs, viper fallback tests - exec_test.go: getSeparatedArgsForExec, executeCommandWithEnv - console_test.go: resolveIdentityName, retrieveCredentials, resolveConsoleDuration - auth_test.go: AuthCommandProvider, GetIdentityFromFlags - completion_test.go: completion functions - validate_test.go: command structure - identity_resolution_test.go: shared identity resolution test helper - user/user_test.go: AuthUserCmd structure - user/configure_test.go: command structure - user/helpers_test.go: selectAWSUserIdentities, extractAWSUserInfo Coverage improved from ~25.57% to ~36.2% for cmd/auth package. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Fix cross-platform compatibility for exec tests - Replace Unix-specific "true" command with cross-platform "go version" - Move "false" command test to exec_unix_test.go with build constraint - Addresses CodeRabbit review comment about Windows compatibility 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * test: Add tests for --profile flag in auth commands (fixes #1973) Add tests to verify that the --profile global flag is properly extracted into ProfilesFromArg when using auth exec and auth shell commands. Test cases: - Single profile (--profile dev) - Multiple profiles (--profile dev --profile staging) - No profile - Profile with special characters (us-east-1/prod) - Environment variable fallback (ATMOS_PROFILE) These tests verify the fix for issue #1973 where --profile didn't work with auth exec and auth shell commands. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * docs: Reorganize terraform usage examples into distinct sections Split the "Clean Examples" section into three distinct subsections: - Clean Examples: terraform clean commands only - Workspace Examples: terraform workspace commands - Additional Flag Examples: plan commands with --/--append-user-agent flags This improves documentation readability by grouping related commands together. Addresses CodeRabbit review comment on PR #1919. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * test: Regenerate auth command golden snapshots Update snapshots for auth command refactoring: - auth exec --help: Updated usage format and added aws CLI example - auth invalid-command: Removed ecr-login from valid subcommands list Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: Windows test failures and website build - Make TestRedactHomeDir cross-platform using filepath.Join - Make TestRedactHomeDirWithOsPathSeparator use consistent path construction - Fix TestFormatExpiration flaky assertion (timing-dependent) - Add missing File component import to terraform/usage.mdx Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: Add IsExperimental method to AuthCommandProvider The CommandProvider interface now requires IsExperimental() after main branch updates. This fixes CI build failures across Linux, macOS, and Windows by implementing the required interface method. Auth commands return true as they are part of Pro Features. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * [autofix.ci] apply automated fixes * [autofix.ci] apply automated fixes (attempt 2/3) * [autofix.ci] apply automated fixes (attempt 3/3) * [autofix.ci] apply automated fixes * fix: Address CodeRabbit review comments for auth-registry-refactor - Remove undocumented IDENTITY env var fallback (comment #10) Only ATMOS_IDENTITY is documented and should be used - Make HOME path test cases OS-portable (comment #11) Use filepath.Join instead of hardcoded Unix paths in tests Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * chore: Regenerate golden snapshots for experimental auth command The auth command now returns IsExperimental() = true, which adds: - [EXPERIMENTAL] tag in command listings - Experimental feature warning message in stderr output Regenerated all affected golden snapshots to match the new output. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: Address CodeRabbit review findings in auth commands - console.go: Use data.Writeln/ui.Writef/ui.Success/ui.Warning instead of fmt.Fprintf(os.Stdout/Stderr) to follow Atmos I/O conventions - env.go: Wrap config/manager init errors with sentinel errors (ErrFailedToInitializeAtmosConfig, ErrFailedToInitializeAuthManager) - exec.go: Remove duplicate os.Environ() in executeCommandWithEnv; use envpkg.ConvertMapToSlice since prepareAuthenticatedEnv already includes the full OS environment - helpers.go: Stop scanning for --help at "--" separator so pass-through commands like `atmos auth exec -- terraform --help` work correctly - logout.go: Fix misleading --all-realms flag description to accurately reflect keychain cleanup scope Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: Migrate auth commands from legacy u.Print* to ui/data layer and fix test failures Replace all antiquated u.PrintfMarkdownToTUI/u.PrintfMessageToTUI calls in auth commands with the proper ui.MarkdownMessagef/ui.Writef functions. Replace u.PrintAsJSON with data.WriteJSON. Add missing Realm row to displayAuthSuccess output. Regenerate golden snapshots to match corrected stderr output. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * feat(auth): port main features to refactored cmd/auth package Backports features from main's cmd/auth_*.go that were not yet present in HEAD's refactored cmd/auth/* tree. Each item references the upstream PR. cmd/auth/env.go (#1984): - Add `--format=github` for $GITHUB_ENV with heredoc multiline support. - Add `--format=env` (lowercase env-style key=value). - Add `-o, --output-file` flag (with $GITHUB_ENV auto-detect for github). - Route formatting through pkg/env.Output() while keeping outputEnvAsExport and outputEnvAsDotenv helpers for unit-test coverage. - Bind ATMOS_AUTH_ENV_FORMAT and ATMOS_AUTH_ENV_OUTPUT_FILE env vars. - Extract loadAuthManagerForEnv, resolveIdentityNameForEnv, loginIfNeeded, resolveEnvOutputTarget, and resolveEnvOutputFile helpers to keep executeAuthEnvCommand within complexity and length budgets. cmd/auth/login.go (provider fallback, #2333): - Change authenticateIdentity signature to (whoami, needsProviderFallback, err). On ErrNoIdentitiesAvailable / ErrNoDefaultIdentity, return the fallback flag instead of wrapping; caller routes to provider auth. - Treat ErrUserAborted as a clean abort (no ErrAuthenticationFailed wrap). - Add getProviderForFallback / promptForProvider / isInteractive helpers for the auto-provision-identities first-login path. - Wire maybeOfferProfileFallbackOnAuthConfigError around identity-flow errors. cmd/auth/exec.go, cmd/auth/shell.go: - Surface identity-resolution errors through maybeOfferProfileFallbackOnAuthConfigError so a stale base profile no longer dead-ends with `no default identity`. Tests: - env_test.go: add TestGitHubEnvAutoDetect (auto-detect $GITHUB_ENV + heredoc framing), update TestSupportedFormats to expect 5 formats. - login_test.go: update TestAuthenticateIdentity for the new needsProviderFallback signal; add ErrNoIdentitiesAvailable case. - integration_test.go (new): TestAuthEnvFormatCompletion, TestAuthWhoamiOutputCompletion, TestAuthCommandCompletion_FlagInheritance. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * updates * [autocommit] formatting fixes * test(auth): add regression tests for issues #1973 and #2392 Both issues are addressed by the cmd/auth/* registry refactor + terraform StandardParser registration of --identity. These tests guard the contracts so a future regression cannot reintroduce either silent flag-drop bug. Issue #1973 (--profile global flag silently dropped on auth exec/shell): - cmd/auth/exec_test.go::TestAuthExec_ProfileFlagAppliedToConfig - cmd/auth/shell_test.go::TestAuthShell_ProfileFlagAppliedToConfig Both call BuildConfigAndStacksInfo(cmd, v) — the helper that exec.go and shell.go now use before cfg.InitCliConfig — and assert that the --profile values round-trip into ConfigAndStacksInfo.ProfilesFromArg for single, multiple, and absent profile cases. Issue #2392 (--identity silently dropped on atmos terraform plan): - internal/exec/cli_utils_test.go::TestProcessCommandLineArgs_TerraformIdentityFlag_Issue2392 Reproduces the bug-report arg shape verbatim — `terraform plan account-map -s core-gbl-root --identity core-root/admin` — and asserts ProcessCommandLineArgs populates info.Identity = "core-root/admin". - internal/exec/terraform_execute_helpers_auth_test.go::TestSetupTerraformAuth_IdentityFlagPropagatesToAuthCreator Builds a merged auth config containing both a `default: true` identity ("core-identity/devops") and a non-default identity ("core-root/admin") and asserts that setupTerraformAuth passes the explicit --identity value verbatim to the auth manager creator — not the profile default. This is the exact override that the issue described as silently happening at v1.216.0. - internal/exec/terraform_execute_helpers_auth_test.go::TestSetupTerraformAuth_EmptyIdentity_AllowsAutoDetection Inverse guard: with no --identity flag, info.Identity is empty and the creator receives the empty string so pkg/auth.resolveIdentityName can auto-detect the profile default. Prevents an over-eager fix from breaking the default-identity path. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * updates * test(auth): boost cmd/auth coverage from 33% to 47% Adds focused unit tests for the testable helpers exposed by the cmd/auth registry refactor. Targets the lowest-coverage files flagged by Codecov: cmd/auth/auth.go (CommandProvider getters): - TestAuthCommandProvider_OptionalMethods covers GetPositionalArgsBuilder, GetCompatibilityFlags, GetAliases, IsExperimental. - TestGetAuthCmd_ReturnsAuthCmd guards the public accessor used by cmd/ai to attach subcommands. cmd/auth/env.go (env command helpers): - TestResolveEnvOutputFile covers all four branches of the GitHub auto- detect: non-github pass-through, explicit output-file, $GITHUB_ENV detect, and the missing-GITHUB_ENV error sentinel. - TestResolveEnvOutputTarget exercises viper-backed format/output-file resolution including the bash default and the github auto-detect. - TestLoginIfNeeded covers cache-hit (no Authenticate), missing-cache (Authenticate triggered), ErrUserAborted unwrapping, and generic- error wrapping with ErrAuthenticationFailed. - TestResolveIdentityNameForEnv covers the explicit-flag, viper-fallback, default-auto-detect, and __SELECT__ interactive paths. cmd/auth/console.go (browser isolation helpers): - TestConsoleSessionDir asserts the deterministic XDG path is stable across calls, diverges by identity, and diverges by realm. - TestResolveConsoleIsolated covers default false, auth.console.isolated config honoured, flag overrides config in both directions. - TestPrintConsoleHelpers smoke-covers printConsoleURL and printConsoleInfo with full + showURL=true paths. - TestRetrieveCredentials_NoCredentialsAvailable covers the ErrAuthConsole sentinel for the no-credentials-anywhere branch. cmd/auth/login.go (provider-fallback helpers): - TestGetProviderForFallback covers ErrNoProvidersAvailable for empty list, single-provider auto-select (no prompt), and multi-provider non-interactive ErrNoDefaultProvider. - TestIsInteractive guards determinism (same env → same answer). cmd/auth/whoami.go (whoami helpers): - TestAddGCPReauthExplanation covers nil pass-through, unrelated-error pass-through, invalid_grant alone (no enrichment), and invalid_grant + invalid_rapt enrichment with gcloud reauth hint. - TestPrintWhoamiJSON_RedactsCredentials guards the contract that the JSON output never mutates the caller's Environment map. - TestPrintWhoamiHuman covers valid/invalid/no-expiration table render. cmd/auth/list.go (list command helpers): - TestParseFilterFlags covers the default, --providers (with comma list), --identities (with comma list), and the mutually-exclusive ErrMutuallyExclusiveFlags branch. - TestRenderOutput_InvalidFormatErrors guards the default-case ErrInvalidFlag path. - TestListFlagCompletions_NoConfig covers the no-atmos.yaml early-return path of listProvidersFlagCompletion / listIdentitiesFlagCompletion. cmd/auth/logout.go (new logout_test.go): - TestBuildKeychainDeletionMessage covers the pure prompt formatter. - TestConfirmKeychainDeletion_ForceShortCircuit covers --force bypass. - TestConfirmKeychainDeletion_NonTTYWithoutForceErrors covers the ErrKeychainDeletionRequiresConfirmation branch. - TestDetectExternalCredentials covers GCP/Azure/AWS env-var detection. - TestBuildLogoutOptions covers the empty-config, identities+providers, and provider-cascade-label branches. - TestExecuteLogoutOption_InvalidType covers ErrInvalidLogoutOption. - TestDiscoverRealms covers missing dir (no error), no-provider-subdir filter, and aws/azure realm reporting. cmd/auth/completion.go (completion helpers): - TestIdentityFlagCompletion covers the no-atmos.yaml branch. - TestAddIdentityCompletion_NoFlag covers the no-flag-registered no-op. cmd/auth/user/helpers.go (form-builder helper): - TestBuildCredentialFormField covers all six branches: YAML-managed Note, DefaultValue pre-fill, password mode, optional, custom ValidateFunc wired, description message wired. Infra: - New testmain_test.go initialises pkg/data and pkg/ui formatters once for the package so tests that exercise printWhoamiJSON/printWhoamiHuman don't panic with "data.InitWriter() must be called". - Snapshot tests/snapshots/TestCLICommands_atmos_auth_validate_--verbose picks up an env-dependent debug line drop (gh CLI not authenticated). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * style: apply gofumpt v0.10 multiline call wrapping CI's pre-commit hook installs `gofumpt@latest` which is now v0.10.0. That release tightened the multiline-call rule: when arguments span multiple lines, the function name + opening paren go on their own line and the closing paren goes on its own line as well. Local toolchain was on v0.9.1 which didn't enforce this, so the changes only surfaced on CI. Files affected (only PR-touched files reformatted): - cmd/auth/env.go: env.Output(...) call. - cmd/describe_dependents.go: getRunnableDescribeDependentsCmd(...) call. - cmd/describe_stacks.go: PersistentFlags().StringP(...) call. No behaviour change. Matches the auto-fix diff produced by the cloudposse/github-action-pre-commit job. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(auth): boost cmd/auth coverage from 47% to 69% Adds smoke and branch-coverage tests across the auth command tree to lift patch coverage from the post-merge baseline (47.8%) to ~69%. Orchestrator smoke tests (no-atmos.yaml tempdir, no-panic contract): - TestExecuteAuthLoginCommand_SmokeNoConfig - TestExecuteAuthWhoamiCommand_SmokeNoConfig - TestExecuteAuthListCommand_SmokeNoConfig - TestExecuteAuthValidateCommand_SmokeNoConfig - TestExecuteAuthConsoleCommand_SmokeNoConfig - TestExecuteAuthEnvCommand_SmokeNoConfig - TestExecuteAuthExecCommand_SmokeNoConfig - TestExecuteAuthShellCommand_SmokeNoConfig - TestExecuteAuthLogoutCommand_SmokeNoConfig - TestExecuteAuthUserConfigureCommand_SmokeNoConfig (cmd/auth/user) These cover the config-load error wrap path in each orchestrator and guard against panics for invalid setup. Where a specific error sentinel is documented (e.g. ErrInvalidAuthConfig, ErrFailedToInitializeAtmosConfig, ErrAuthConsole), the test asserts the wrap when an error surfaces. loadAuthManager* helpers (config init + auth manager init): - TestLoadAuthManager_SmokeFromEmptyTempDir (whoami) - TestLoadAuthManagerForEnv_SmokeFromEmptyTempDir (env) - TestLoadAuthManagerForList_SmokeFromEmptyTempDir (list) - TestInitializeAuthManager_SmokeFromEmptyTempDir (console) - TestSuggestProfilesForAuth_NoProfilesReturnsNil prepareShellEnvironment (cmd/auth/shell.go) — mocked-AuthManager test covering cache-hit, fresh-auth-success, ErrUserAborted, generic-error wrap with ErrAuthenticationFailed, PrepareShellEnvironment error, and atmosConfig.Env propagation through MergeGlobalEnv: - TestPrepareShellEnvironment (six subtests) prepareAuthenticatedEnv (cmd/auth/exec.go) — smoke from empty tempdir: - TestPrepareAuthenticatedEnv_SmokeNoConfig Display + handleBrowserOpen helpers (smoke): - TestDisplayExternalCredentialWarnings (with-warnings + clean branch) - TestDisplayBrowserWarning (first-call + cached-skip branches) - TestHandleBrowserOpen (skipOpen=true, nil opener, success, error) Logout perform helpers — mocked AuthManager branch coverage: - TestPerformIdentityLogout_NotFound (ErrIdentityNotInConfig) - TestPerformIdentityLogout_DryRun (no Logout call) - TestPerformProviderLogout_NotFound (missing provider in config) - TestPerformLogoutAll_DryRun (no LogoutAll call) - TestPerformLogoutAll_Success (happy path, two identities) renderOutput dispatcher coverage: - TestRenderOutput_AllValidFormats covers all 9 valid format branches (table/tree/json/yaml/graphviz/dot/mermaid/markdown/md). Coverage summary: - cmd/auth: 47.6% → 69.7% - cmd/auth/user: 51.0% → 58.7% - combined: 47.8% → 68.9% Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(auth): address review comments and push coverage from 47% to 78% Addresses CodeRabbit review comments and substantially increases test coverage of the cmd/auth package using a shared mock-auth fixture. Review fixes: - cmd/auth/completion.go, cmd/auth/list.go: all five shell-completion helpers now honour --base-path, --config, --config-path, and --profile by routing through BuildConfigAndStacksInfo(cmd, v) instead of using an empty ConfigAndStacksInfo{}. - cmd/auth/console.go: --print-only now propagates the data.Writeln write error so broken-pipe / stdout failures exit non-zero. The empty-identity branch of resolveIdentityName now uses multi-%w so ErrNoDefaultIdentity stays in the chain for the profile-fallback dispatcher. - cmd/auth/exec.go: prepareAuthenticatedEnv now returns []string directly (was []string → map → []string round-trip) so environment ordering is preserved and Windows drive-scoped vars (=C:=...) don't collide on the empty key. - cmd/auth/shell.go: fail fast when positional args are not preceded by `--` (`atmos auth shell bash` previously silently dropped "bash"). - cmd/auth/logout.go: add cobra.MaximumNArgs(1) so extra positional args are rejected up front. - cmd/auth/validate.go: wrap config-load failure with the static ErrFailedToInitializeAtmosConfig sentinel. - cmd/auth/login.go: introduce isInteractiveFn package var so getProviderForFallback tests can force the non-interactive branch deterministically. - cmd/auth/markdown/*.md: every opening fence now has the `shell` language identifier (MD040). The `$ ` prefix on commands is kept for consistency with the rest of cmd/markdown/. - cmd/auth/user/configure.go: switch user-facing messages from fmt.Fprintf(cmd.ErrOrStderr()) to ui.Writef / ui.Writeln per the I/O layer convention. - cmd/identity_helpers.go: add the missing perf.Track in CreateAuthManagerFromIdentityWithStackScan to match its sibling helpers. Coverage improvements (cmd/auth 47.6% → 79.3%; combined 47.8% → 77.8%): New shared helpers (helpers_test.go): - setupMockAuthFixture(t): writes a minimal atmos.yaml wired to the mock/aws provider and isolates keyring + XDG env to a tempdir, used by 9 deep-coverage tests to exercise the full orchestrator pipeline without touching the host's credential store. - runProfileFlagAppliedRegressionTest(t, commandName): shared table-driven driver for the issue #1973 regression so the exec_test.go and shell_test.go wrappers feed the same cases through one body (and dupl-lint stays clean). - newTestCommandWithGlobalParser: parser-returning variant of the global-flags helper so regression tests can drive the real Cobra → Viper binding path (cmd.ParseFlags → BindFlagsToViper). End-to-end orchestrator tests against the mock fixture: - TestExecuteAuthEnvCommand_WithMockAuth - TestExecuteAuthLoginCommand_WithMockAuth - TestExecuteAuthListCommand_WithMockAuth / _JSONFormat - TestExecuteAuthValidateCommand_WithMockAuth - TestExecuteAuthWhoamiCommand_WithMockAuth - TestExecuteAuthConsoleCommand_WithMockAuth (covers ErrProviderNotSupported for mock/aws) - TestExecuteAuthLogoutCommand_WithMockAuthDryRun - TestPrepareAuthenticatedEnv_WithMockAuth (asserts AWS_PROFILE + AWS_REGION injection) - TestExecuteAuthExecCommand_NoCommand (ErrNoCommandSpecified guard) Logout perform-helper branch coverage: - TestPerformIdentityLogout_Success / _PartialLogout / _LogoutError - TestPerformProviderLogout_Success / _DryRun - TestExecuteLogoutOption_DispatchAll / _DispatchIdentity / _DispatchProvider - TestPerformInteractiveLogout_NoIdentities (empty-identities branch) - TestPerformLogoutAllRealms_NoRealms / _DryRun / _RealRemove Pure / smoke helpers: - TestRetrieveCredentials_InlineCredentials (success path) - TestPromptForProvider_EmptyList (ErrNoProvidersAvailable guard) - TestExecuteCommandWithEnv_NonZeroExit (errUtils.ExitCodeError propagation via the test-binary subprocess pattern) - TestExecuteCommandWithEnv_WithValidCommand rewritten to use os.Executable() + _ATMOS_AUTH_TEST_EXIT_OK=1 (cross-platform, no PATH dependency on `go` / `true` / `false`). - Subprocess env-flag handlers added to TestMain. Test infrastructure: - TestAuthExec_ProfileFlagAppliedToConfig and the shell equivalent now use --profile=devops style CLI args + ParseFlags + BindFlagsToViper, exercising the full production binding chain rather than seeding viper directly. - Identity-resolution shared test table gains a "flag takes precedence over env" case so the precedence rule (flags > env > default) is regression-covered for both auth shell and auth exec. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(auth): address review comments — fence repair, helpers, sentinel Addresses six CodeRabbit review comments from the latest batch: 1. cmd/auth/console_test.go — drop tautological constant tests. TestConsoleLabelWidth and TestConsoleOutputFormat just asserted that a literal equals itself. The constants are exported but only used internally; not part of an external API contract. 2. cmd/auth/console_test.go — fragile test-name dispatch. TestResolveConsoleDuration's "invalid provider duration format" case keyed its assertion off tt.name. Renaming the case would silently skip the error check. Added an explicit `expectParseError bool` field; switch case now keys off that. 3. cmd/auth/exec_test.go — single-case table replaced. TestGetSeparatedArgsForExec was a one-case table that duplicated the sibling TestGetSeparatedArgsForExec_EmptyCommand. Rewrote with five meaningful cases (no separator, positional-without-sep, separator+ single command, separator+command+args, separator-only). Deleted the now-redundant sibling test. 4. cmd/auth/env_test.go — safe stdout-capture helper. Five sites used the same fragile capture pattern: if require.NoError aborted before restoration, os.Stdout stayed redirected and the read end of os.Pipe was never closed. Added captureStdout(t) helper in helpers_test.go that registers t.Cleanup to guarantee restoration and close both pipe ends even when intervening assertions abort. All five sites now use `read := captureStdout(t)` / `output := read()`. 5. cmd/auth/env_test.go — replace hardcoded Unix paths with t.TempDir(). Three paths (/tmp/out.sh, /explicit/path, /path/to/.env) replaced with filepath.Join(t.TempDir(), ...) so the tests don't bake in a Unix separator. 6. cmd/auth/markdown/atmos_auth_console_usage.md + cmd/auth/markdown/atmos_auth_logout_usage.md — fix malformed closers. An earlier awk script that added the shell language identifier to opening fences had a state-machine flaw on files that already mixed labeled/unlabeled fences; six closing fences across two files were rewritten as ```shell instead of plain ```. Repaired to keep the rest of the doc rendering correctly. Audited all four auth markdown files; the other two were already correctly paired. 7. cmd/auth/logout_test.go + sibling _WithMockAuth tests — add a cmd-state isolation helper. Tests that mutate package-level auth*Cmd via ParseFlags could leak flag.Changed / flag values to subsequent tests. cmd.NewTestKit isn't reachable from cmd/auth without a circular import, so added a local equivalent resetAuthCmdFlags(t, cmd) that snapshots and restores every flag's (Value, Changed) pair via t.Cleanup. Applied to all 8 tests that ParseFlags a shared cmd: TestExecuteAuthLogoutCommand_SmokeNoConfig and _WithMockAuthDryRun (the two reviewer-flagged sites), plus _WithMockAuth variants for console, env, exec (TestPrepareAuthenticatedEnv_WithMockAuth + TestExecuteAuthExecCommand_NoCommand), list (tree + JSON), login, validate, whoami. 8. cmd/auth/shell.go — tighten the pre-dash arg check and fix the hint. The previous check only caught the no-separator case; `atmos auth shell bash -- -lc env` had ArgsLenAtDash() == 1 and slipped through while getSeparatedArgs silently dropped "bash". Condition is now `dashIndex == -1 || dashIndex > 0` so positional args appearing before "--" are also rejected. The error message no longer suggests `-- bash` (which would just feed "bash" as the first arg to the default shell) — it points at `--shell` for the shell-binary override and "--" only for the shell args. Extracted into validateAuthShellArgs to keep executeAuthShellCommand under the 60-line revive limit. Added TestValidateAuthShellArgs with 5 subtests covering the three acceptable and two rejected arg shapes plus the ErrInvalidArguments sentinel contract. 9. cmd/auth/shell.go + cmd/auth/exec.go — wrap PrepareShellEnvironment failures with a static sentinel. Both helpers had a raw fmt.Errorf("failed to prepare ...: %w", err) that didn't satisfy the repo-wide "All errors MUST be wrapped using static errors defined in errors/errors.go" rule. Added new sentinel errUtils.ErrPrepareShellEnvironment and applied to both call sites. Updated TestPrepareShellEnvironment to assert the sentinel is in the chain (errors.Is) plus the original underlying error. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(auth): apply resetAuthCmdFlags across all smoke tests for state isolation Add `resetAuthCmdFlags(t, cmd)` to every test that uses a package-level `auth*Cmd` so flag/Changed state cannot leak across tests under shuffled runs. This is the in-package equivalent of `cmd.NewTestKit(t)` — `cmd/auth` cannot import `cmd` (cmd/root.go blank-imports cmd/auth) and `NewTestKit` is `_test.go`-scoped, so the local helper enforces the same auto-cleanup contract via t.Cleanup. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Matt Topper <matt.topper@gmail.com> Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com> Co-authored-by: Andriy Knysh <aknysh@users.noreply.github.com> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> Co-authored-by: aknysh <andriy.knysh@gmail.com> Co-authored-by: atmos-pro[bot] <atmos-pro[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
what
regiontokubeconfigcommandshelmfile-commandjobwhy