feat(aws): clear expired-credential errors and improved AWS identity banner#384
Merged
Conversation
Make the AWS identity line print first and colored, and surface expired/invalid credentials as a clear, actionable error across the CLI and all SDKs instead of masking them as ParameterNotFound. Identity banner (CLI/GHA): - Emit the identity line via a dispatcher preflight (new optional ISecretProvider.logIdentity) so it prints before parallel secret resolution instead of mid-stream. - Colorize it with picocolors; account/region render red when unknown. Expired credentials (CLI core, Node.js, Python, .NET SDKs): - Detect expired/invalid credentials (expired SSO token, expired security token, unrecognized client, etc.) and throw a typed, actionable ExpiredCredentialsError/Exception telling the user to refresh credentials (e.g. aws sso login). - CLI pull handler now fails fast on expired credentials and reports the real error message instead of a hardcoded ParameterNotFound. - Genuine ParameterNotFound and unrelated errors keep their behavior.
Release the AWS identity banner and expired-credential error work: CLI/GHA 0.13.0, Node.js SDK 0.4.0, Python SDK 0.5.0, .NET SDK 0.5.0. Adds changelog entries for the colorized identity banner, its earlier ordering, and the new ExpiredCredentialsError/Exception across the CLI and SDKs.
github-actions
Bot
added
documentation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe PR adds expired AWS credential and SSO-session handling across the core app and SDKs, updates AWS identity banner logging, bumps SDK versions, and adds release notes and an ADR for local override map files. ChangesExpired AWS credential handling
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces explicit handling for expired or invalid AWS credentials across the CLI, GitHub Action, and SDKs (.NET, Node.js, Python) by raising a dedicated ExpiredCredentialsError (or ExpiredCredentialsException in .NET) with actionable remediation steps. It also updates the CLI to display a colorized AWS identity banner before resolving secrets. The review feedback highlights several important improvements: adding a null check and a regex match timeout in the .NET ExpiredCredentialsDetector to prevent runtime exceptions and ReDoS vulnerabilities, implementing standard exception constructors in .NET, and explicitly declaring the cause property in TypeScript error classes to avoid type assertions.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
src/sdks/dotnet/Infrastructure/Aws/ExpiredCredentialsDetector.cs (2)
20-23: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value
expiredandsso sessionpatterns are broad and may misclassify unrelated errors.A bare
expired(orsso session) substring will match any message containing those words—including, for example, a parameter whose value or description mentions "expired"—converting an unrelated failure intoExpiredCredentialsExceptionand masking the true cause. Consider tightening to phrases liketoken has expired/session has expiredrather than the standaloneexpiredalternative.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/sdks/dotnet/Infrastructure/Aws/ExpiredCredentialsDetector.cs` around lines 20 - 23, The Regex in ExpiredCredentialsDetector.MessagePattern is too broad because standalone alternatives like expired and sso session can match unrelated errors and misclassify them as ExpiredCredentialsException. Tighten the pattern to more specific credential-expiration phrases by removing the bare substrings and using explicit matches such as token has expired or session has expired, while keeping the existing credential-related alternatives in place.
25-35: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winConsider inspecting inner exceptions, not just the top-level message.
IsExpiredCredentialsonly checksexception.Messageand the top-levelErrorCode. AWS SDK credential/SSO failures are frequently surfaced as a generic wrapper whose real cause (the expired-token detail) lives inInnerException. In those cases the detector returnsfalseand the original error propagates instead of being mapped toExpiredCredentialsException. Walking the inner-exception chain would make detection more robust.♻️ Walk the inner-exception chain
public static bool IsExpiredCredentials(Exception exception) { - if (exception is AmazonServiceException serviceException - && serviceException.ErrorCode is not null - && ExpiredErrorCodes.Contains(serviceException.ErrorCode)) - { - return true; - } - - return MessagePattern.IsMatch(exception.Message); + for (Exception? current = exception; current is not null; current = current.InnerException) + { + if (current is AmazonServiceException serviceException + && serviceException.ErrorCode is not null + && ExpiredErrorCodes.Contains(serviceException.ErrorCode)) + { + return true; + } + + if (MessagePattern.IsMatch(current.Message)) + { + return true; + } + } + + return false; }Please confirm whether real expired-credential failures (e.g. expired SSO sessions) reach this provider as a top-level
AmazonClientException/AmazonServiceExceptionor as a wrapper with the detail inInnerException.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/sdks/dotnet/Infrastructure/Aws/ExpiredCredentialsDetector.cs` around lines 25 - 35, The expired-credentials detector only checks the top-level exception message and AWS error code, so it can miss wrapped AWS/SSO failures where the real expired-token detail is in InnerException. Update ExpiredCredentialsDetector.IsExpiredCredentials to walk the full inner-exception chain and apply the existing ErrorCode/MessagePattern checks to each exception in the chain, keeping the current top-level AmazonServiceException handling intact. Use the IsExpiredCredentials method and MessagePattern/ExpiredErrorCodes as the main lookup points.tests/sdks/dotnet/Infrastructure/Aws/ExpiredCredentialsTests.cs (1)
11-87: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the synchronous
GetSecretexpired-credentials path.All four tests exercise
GetSecretAsync. The synchronousGetSecretmethod has the same expired-credentials catch filter (AwsSsmSecretProvider.csLines 71-74), but no test asserts it maps toExpiredCredentialsException. A regression in the sync branch would go undetected.Want me to draft a
Should_ThrowExpiredCredentialsException_When_GetSecretFailsWithExpiredTokentest mirroring the async case for the syncGetSecretpath?🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/sdks/dotnet/Infrastructure/Aws/ExpiredCredentialsTests.cs` around lines 11 - 87, Add a synchronous coverage test for the expired-credentials mapping in AwsSsmSecretProvider by mirroring the existing async expired-token case with GetSecret instead of GetSecretAsync; use the same expired token setup and assert that GetSecret("/p") throws ExpiredCredentialsException with the expected aws sso login message so the GetSecret catch filter is exercised too.src/sdks/nodejs/src/domain/expired-credentials-error.ts (1)
30-49: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueBroad message regex may misclassify unrelated failures as expired credentials.
The bare
expiredandrefresh.*failedalternatives can match non-credential errors (e.g., a transient message that happens to contain "expired"), converting them intoExpiredCredentialsErrorand masking the real cause — the very symptom this PR aims to eliminate. Thename-set check already covers the well-defined AWS cases; the message fallback is the loose part. If this breadth is intentional for cross-SDK parity, no change is needed.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/sdks/nodejs/src/domain/expired-credentials-error.ts` around lines 30 - 49, The message-based matching in isExpiredCredentialsError is too broad and can misclassify unrelated failures as expired credentials. Tighten EXPIRED_CREDENTIAL_MESSAGE_PATTERN in expired-credentials-error.ts by removing or narrowing the loose alternatives like expired and refresh.*failed, while keeping the explicit AWS name checks in EXPIRED_CREDENTIAL_ERROR_NAMES as the primary signal. Preserve the message fallback only for well-scoped credential-expiration phrases so ExpiredCredentialsError is created only for clearly credential-related cases.src/envilder/core/infrastructure/aws/isExpiredCredentialsError.ts (1)
1-11: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAllow-list diverges from the Node.js SDK copy.
This core set includes
'UnrecognizedClient'(Line 4) in addition to'UnrecognizedClientException', but the Node.js SDK'sisExpiredCredentialsErrorallow-list omits'UnrecognizedClient'. AWS surfaces this condition asUnrecognizedClientException, so the bare'UnrecognizedClient'entry is effectively dead and the two detectors classify the same error differently. Consider aligning the lists (dropping the redundant bare entry or adding it to Node.js) so expired-credential detection behaves consistently across surfaces.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/envilder/core/infrastructure/aws/isExpiredCredentialsError.ts` around lines 1 - 11, The expired-credentials allow-list in isExpiredCredentialsError is out of sync with the Node.js SDK copy because it includes the redundant UnrecognizedClient entry alongside UnrecognizedClientException. Align the two detectors by updating EXPIRED_CREDENTIAL_ERROR_NAMES to match the Node.js list (or explicitly adding the same entry there) so both surfaces classify expired credential errors consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/sdks/python/envilder/__init__.py`:
- Line 9: The new import in envilder.__init__ is breaking the package import
order and causing the isort check to fail. Re-run isort for the envilder package
and adjust the import grouping/order in __init__.py so the
ExpiredCredentialsError import is placed in the correct position relative to the
other envilder.* imports and any standard/third-party imports. Keep the final
ordering consistent with isort output.
In `@tests/sdks/python/infrastructure/aws/test_expired_credentials.py`:
- Around line 1-9: The new test module has an import block that is not sorted
according to isort, causing the tests/sdks/python CI check to fail. Update the
imports in test_expired_credentials.py to match standard isort grouping/order,
keeping the existing symbols like Mock, pytest, ClientError,
TokenRetrievalError, ExpiredCredentialsError, and AwsSsmSecretProvider in the
normalized order. Focus only on the import section at the top of the file and
ensure the final ordering passes isort --check-only.
---
Nitpick comments:
In `@src/envilder/core/infrastructure/aws/isExpiredCredentialsError.ts`:
- Around line 1-11: The expired-credentials allow-list in
isExpiredCredentialsError is out of sync with the Node.js SDK copy because it
includes the redundant UnrecognizedClient entry alongside
UnrecognizedClientException. Align the two detectors by updating
EXPIRED_CREDENTIAL_ERROR_NAMES to match the Node.js list (or explicitly adding
the same entry there) so both surfaces classify expired credential errors
consistently.
In `@src/sdks/dotnet/Infrastructure/Aws/ExpiredCredentialsDetector.cs`:
- Around line 20-23: The Regex in ExpiredCredentialsDetector.MessagePattern is
too broad because standalone alternatives like expired and sso session can match
unrelated errors and misclassify them as ExpiredCredentialsException. Tighten
the pattern to more specific credential-expiration phrases by removing the bare
substrings and using explicit matches such as token has expired or session has
expired, while keeping the existing credential-related alternatives in place.
- Around line 25-35: The expired-credentials detector only checks the top-level
exception message and AWS error code, so it can miss wrapped AWS/SSO failures
where the real expired-token detail is in InnerException. Update
ExpiredCredentialsDetector.IsExpiredCredentials to walk the full inner-exception
chain and apply the existing ErrorCode/MessagePattern checks to each exception
in the chain, keeping the current top-level AmazonServiceException handling
intact. Use the IsExpiredCredentials method and MessagePattern/ExpiredErrorCodes
as the main lookup points.
In `@src/sdks/nodejs/src/domain/expired-credentials-error.ts`:
- Around line 30-49: The message-based matching in isExpiredCredentialsError is
too broad and can misclassify unrelated failures as expired credentials. Tighten
EXPIRED_CREDENTIAL_MESSAGE_PATTERN in expired-credentials-error.ts by removing
or narrowing the loose alternatives like expired and refresh.*failed, while
keeping the explicit AWS name checks in EXPIRED_CREDENTIAL_ERROR_NAMES as the
primary signal. Preserve the message fallback only for well-scoped
credential-expiration phrases so ExpiredCredentialsError is created only for
clearly credential-related cases.
In `@tests/sdks/dotnet/Infrastructure/Aws/ExpiredCredentialsTests.cs`:
- Around line 11-87: Add a synchronous coverage test for the expired-credentials
mapping in AwsSsmSecretProvider by mirroring the existing async expired-token
case with GetSecret instead of GetSecretAsync; use the same expired token setup
and assert that GetSecret("/p") throws ExpiredCredentialsException with the
expected aws sso login message so the GetSecret catch filter is exercised too.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a4d697d4-5148-4168-b699-b87529705447
⛔ Files ignored due to path filters (2)
github-action/dist/index.jsis excluded by!**/dist/**,!github-action/dist/**and included by nonepackage.jsonis excluded by none and included by none
📒 Files selected for processing (29)
docs/changelogs/cli.mddocs/changelogs/gha.mddocs/changelogs/sdk-dotnet.mddocs/changelogs/sdk-nodejs.mddocs/changelogs/sdk-python.mdsrc/envilder/core/application/dispatch/DispatchActionCommandHandler.tssrc/envilder/core/application/pullSecretsToEnv/PullSecretsToEnvCommandHandler.tssrc/envilder/core/domain/errors/DomainErrors.tssrc/envilder/core/domain/ports/ISecretProvider.tssrc/envilder/core/infrastructure/aws/AwsSsmSecretProvider.tssrc/envilder/core/infrastructure/aws/isExpiredCredentialsError.tssrc/sdks/dotnet/Envilder.csprojsrc/sdks/dotnet/ExpiredCredentialsException.cssrc/sdks/dotnet/Infrastructure/Aws/AwsSsmSecretProvider.cssrc/sdks/dotnet/Infrastructure/Aws/ExpiredCredentialsDetector.cssrc/sdks/nodejs/package.jsonsrc/sdks/nodejs/src/domain/expired-credentials-error.tssrc/sdks/nodejs/src/index.tssrc/sdks/nodejs/src/infrastructure/aws/aws-ssm-secret-provider.tssrc/sdks/python/envilder/__init__.pysrc/sdks/python/envilder/domain/expired_credentials_error.pysrc/sdks/python/envilder/infrastructure/aws/aws_ssm_secret_provider.pysrc/sdks/python/pyproject.tomltests/envilder/core/application/dispatch/DispatchActionCommandHandler.test.tstests/envilder/core/application/pullSecretsToEnv/PullSecretsToEnvCommandHandler.test.tstests/envilder/core/infrastructure/aws/AwsSsmSecretProvider.test.tstests/sdks/dotnet/Infrastructure/Aws/ExpiredCredentialsTests.cstests/sdks/nodejs/infrastructure/aws/expired-credentials.test.tstests/sdks/python/infrastructure/aws/test_expired_credentials.py
Address PR #384 review (Gemini, CodeRabbit). Stop matching AWS error message text -- it is fragile and locale/version dependent -- and rely only on structured signals (AWS error name/code and exception type). - CLI core + Node.js: detect solely via AWS SDK v3 `error.name` allow-list (now identical across both); message regex removed. Declare `readonly cause` on the error classes instead of a cast. - Python: detect via `ClientError` error code set + botocore credential exception class names; message regex removed. - .NET: detect via `AmazonServiceException.ErrorCode` + exception type name, walking `InnerException`; Regex removed (no ReDoS risk), null guard added, standard exception constructors added, plus a test for the synchronous GetSecret path.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/sdks/nodejs/src/domain/expired-credentials-error.ts`:
- Around line 25-29: The expired-credentials allowlist in
expired-credentials-error should be narrowed so only explicit token-expiration
names remain. Update EXPIRED_CREDENTIAL_ERROR_NAMES to remove RequestExpired,
UnrecognizedClientException, InvalidClientTokenId, InvalidSignatureException,
CredentialsProviderError, ProviderError, and similar generic provider/error
names, and keep only true expiration cases like ExpiredToken and
ExpiredTokenException. Make the change in the allowlist used by
ExpiredCredentialsError so the SDK stops mapping clock-skew or
credential-configuration failures to credential-expiration remediation.
In `@src/sdks/python/envilder/domain/expired_credentials_error.py`:
- Around line 11-22: Update the expired-credentials classification so missing
credentials and clock-skew cases are not treated as session expiration. In the
logic around is_expired_credentials_error, remove NoCredentialsError,
PartialCredentialsError, and RequestExpired from the expired-session match path,
and keep only true token/session expiration exceptions in
_EXPIRED_EXCEPTION_NAMES or the equivalent matching set. Make sure
aws_ssm_secret_provider.py no longer maps those distinct failures to
ExpiredCredentialsError so the remediation path stays accurate.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a08c6eb1-35cc-4570-b50b-edd60b8448d4
⛔ Files ignored due to path filters (3)
CONTEXT.mdis excluded by none and included by nonegithub-action/dist/index.jsis excluded by!**/dist/**,!github-action/dist/**and included by nonesrc/sdks/python/uv.lockis excluded by!**/*.lockand included bysrc/**
📒 Files selected for processing (7)
src/envilder/core/domain/errors/DomainErrors.tssrc/envilder/core/infrastructure/aws/isExpiredCredentialsError.tssrc/sdks/dotnet/ExpiredCredentialsException.cssrc/sdks/dotnet/Infrastructure/Aws/ExpiredCredentialsDetector.cssrc/sdks/nodejs/src/domain/expired-credentials-error.tssrc/sdks/python/envilder/domain/expired_credentials_error.pytests/sdks/dotnet/Infrastructure/Aws/ExpiredCredentialsTests.cs
✅ Files skipped from review due to trivial changes (1)
- src/envilder/core/domain/errors/DomainErrors.ts
15 tasks
Address PR #384 review (CodeRabbit, Major). The detectors classified clock-skew, missing-config, bad-key and generic provider errors as expired credentials, giving wrong remediation (run aws sso login). Keep only true session-expiry signals across CLI core, Node.js, Python, .NET. Still catches expired SSO sessions via token-provider/token-retrieval signals, without misclassifying config or clock issues.
Startup.ts was the only file in bootstrap/ for both cli and gha app
folders. With entry/, errors/, and recovery/ already extracted, Startup.ts
is unambiguous by elimination at the app root, so the extra subfolder was
over-categorization.
- Move src/envilder/apps/{cli,gha}/bootstrap/Startup.ts to the app root
- Move mirrored tests/envilder/apps/{cli,gha}/bootstrap/Startup.test.ts
to the test root
- Fix import paths in Cli.ts, Gha.ts, Startup.test.ts, Cli.test.ts,
Gha.test.ts, e2e/cli.test.ts, e2e/gha.test.ts
- Rebuild github-action/dist/index.js
Validated: pnpm build clean, pnpm lint clean, 64/64 unit tests passing.
SecretOperationError messages from the AWS/Azure providers already embed a masked path (\<maskedPath>: <reason>\). PullSecretsToEnvCommandHandler was storing the raw, unmasked path in SecretFetchFailure.path while also keeping the full error message (including the duplicated masked path) as reason. CLI and GHA error presenters then printed the raw path directly, leaking real SSM/Key Vault paths in logs. - Mask the path once at the source, in processSecret(), so SecretsFetchError.failures[].path is always masked by construction - Strip the duplicated \<maskedPath>: \ prefix from reason when present - Add a regression test for the prefix-stripping behavior - Update the existing SecretsFetchError test to expect a masked path - Rebuild github-action/dist/index.js Addresses 3 PR review comments (PullSecretsToEnvCommandHandler.ts, CliErrorPresenter.ts, GhaErrorPresenter.ts) with a single root-cause fix instead of patching masking logic into both presenters separately. Validated: pnpm build clean, pnpm lint clean, 348/348 unit tests passing.
AzureKeyVaultSecretProvider.getSecret() masked and printed the normalized Key Vault name (after casing/slash/prefix transforms), not the original mapping key. When normalization changed the name, users could not correlate the error back to their map file. - Use the original name param for the masked path in SecretOperationError instead of the normalized secretName - Add regression test with a name that normalizes differently from its original form Validated: 26/26 unit tests passing.
Fallback error rendering in both presenters used error.message directly, which is often empty for AggregateError (AWS SDK v3 wraps connection failures this way), producing a blank reason line in CLI/action output. - Reuse describeError() (already used by AWS/Azure providers) in both CliErrorPresenter.renderFallback() and GhaErrorPresenter.renderFallback() - It digs into AggregateError.errors and Error.cause so the reason is never blank, falling back to error.name then unknown error - Add regression tests for both presenters with an AggregateError that has an empty top-level message Validated: 14/14 unit tests passing.
AwsSsmSecretProvider.setSecret() rethrew non-credential failures raw (throw error), unlike getSecret() which wraps them as SecretOperationError with a masked path and extracted reason. This was inconsistent and could leak the full parameter name/path in error messages and stack traces on push. - Wrap non-credential setSecret failures the same way getSecret does - Rename the affected test to reflect the new wrapping behavior Validated: 20/20 unit tests passing.
describeErrorReason() read error.message directly, which can be empty for AggregateError (wrapped AWS SDK v3 connection failures), producing a blank WHY/reason line in SecretsFetchError presentation. - Reuse describeError() instead of raw error.message, mirroring how AWS/Azure providers already build the SecretOperationError message it strips, keeping both ends of the duplicated-prefix logic symmetric - Add regression test with an AggregateError with an empty top-level message - Fix a stray biome formatting nit left over in AzureKeyVaultSecretProvider.test.ts from the previous commit Validated: 11/11 unit tests passing, pnpm lint clean.
Replace all em dash (U+2014) characters in the per-component changelogs
with colon, semicolon, or parentheses depending on context. No wording
or content changes, punctuation only.
Files: docs/changelogs/{cli,gha,sdk-dotnet,sdk-nodejs,sdk-python}.md
Replace all em dash (U+2014) characters across .github/ (agents, instructions, skills, prompts), docs/ (ADRs, architecture, security, ai-workflows), README.md, ROADMAP.md, CONTEXT.md, CONTRIBUTING.md, examples/sdk/*/README.md, and src/sdks/*/README.md with colon, semicolon, comma, or parentheses depending on context. Also fixed table cells where a standalone em dash meant N/A: replaced with 'n/a' instead of a bare colon. Left unchanged: 4 occurrences in workflow-pr-sync skill docs that literally document the em-dash character itself as an example of what to avoid in PR bodies (removing them would break the example). No wording or content changes beyond punctuation. Validated: pnpm lint clean.
…lback DispatchActionCommandHandler.handleCommand()'s switch default branch silently called handlePull() for any unhandled OperationMode. Since OperationMode is an enum, this could silently run the wrong operation if a new mode is added without updating this switch, or if an invalid value slips through. - Throw InvalidArgumentError in the default branch instead of falling back to pull - Add regression test asserting the pull handler is never invoked for an unsupported mode Validated: pnpm build clean, pnpm lint clean, 353/353 unit tests passing, github-action/dist/index.js rebuilt.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 159 out of 162 changed files in this pull request and generated 7 comments.
Comments suppressed due to low confidence (2)
src/envilder/core/application/dispatch/DispatchActionCommandHandler.ts:75
- Avoid
as stringcasts here: they bypass type safety and can hide future regressions if validation changes. Use non-null assertions (or convert the validation helpers intoassertsfunctions) so the compiler keeps helping you.
src/envilder/core/application/dispatch/DispatchActionCommandHandler.ts:84 - Avoid
as stringcasts here: they bypass type safety and can hide future regressions if validation changes. Use non-null assertions (or convert the validation helpers intoassertsfunctions) so the compiler keeps helping you.
…mocks - DispatchActionCommandHandler: replace 'as string' casts with TS asserts type-predicate functions, and use typeof === 'function' guard for optional logIdentity port method - test_expired_credentials.py: use Mock(spec=SSMClient) per python-test-doubles skill - Rebuild github-action/dist/index.js bundle
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
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.
Summary
Improves AWS identity visibility and credential-error clarity. The
AWS identitybanner now prints first (before parallel secret resolution) and is colorized. And when AWS credentials or the security token are expired/invalid (e.g. an expired SSO session), Envilder now raises a clear, actionable error across the CLI and all SDKs instead of masking it as a misleadingParameterNotFound.Changes
AWS identitybanner (account/regionshown red whenunknown), emitted via a dispatcher preflight so it always prints before secrets are resolved (CLI + GitHub Action).ExpiredCredentialsError(CLI core, Node.js, Python) /ExpiredCredentialsException(.NET) -- guiding the user to refresh credentials (e.g.aws sso login).ParameterNotFound.Testing
pnpm lintpasses (secretlint + biome + tsc)Related
N/A