Repository navigation
Speed up container discovery and harden emulator listings - #2828
Conversation
|
Tip Atmos Pro
No affected stacks workflow was detected for this pull request. |
Dependency Review✅ No vulnerabilities or license issues found.Scanned Files
|
|
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughContainer runtime resolution now supports automatic-selection caching, invalidation, explicit recovery outcomes, and bulk status queries. File-cache deletion is idempotent. Emulator listing treats missing imports or stack manifests as empty successful results. Website dependency overrides and dependency-review exceptions are updated. ChangesContainer runtime resolution
Emulator empty-result handling
Website dependency overrides
Estimated code review effort: 4 (Complex) | ~45 minutes 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
pkg/component/emulator/executor_test.go (1)
476-522: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse a table-driven test for the sentinel cases.
The two new subtests share the same setup and assertions; table-drive them with the sentinel error as the test-case input. As per coding guidelines, Go tests should use table-driven tests for multiple scenarios.
🤖 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 `@pkg/component/emulator/executor_test.go` around lines 476 - 522, Convert the two sentinel-error subtests around emulatorStatuses into one table-driven test, using each sentinel error (errUtils.ErrFailedToFindImport and errUtils.ErrNoStacksFound) as the test-case input. Keep the shared stub setup, describeEmulatorStacks seam, emulatorStatuses invocation, and no-error/empty-status assertions unchanged for every case.Source: Coding guidelines
🤖 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 `@pkg/component/container/runtime_discovery.go`:
- Around line 77-80: In the cached-runtime branch of runtime discovery, set
resolution.cached to true before assigning resolution.invalidate to
invalidatingRuntime. Ensure the captured invalidation method observes the cached
state so failures remove stale cache entries, while preserving the existing
runtime return flow.
---
Nitpick comments:
In `@pkg/component/emulator/executor_test.go`:
- Around line 476-522: Convert the two sentinel-error subtests around
emulatorStatuses into one table-driven test, using each sentinel error
(errUtils.ErrFailedToFindImport and errUtils.ErrNoStacksFound) as the test-case
input. Keep the shared stub setup, describeEmulatorStacks seam, emulatorStatuses
invocation, and no-error/empty-status assertions unchanged for every case.
🪄 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 Plus
Run ID: 5887254f-320c-449d-9410-28067ecd15b9
📒 Files selected for processing (13)
pkg/cache/file_cache.gopkg/cache/file_cache_test.gopkg/component/container/executor.gopkg/component/container/list.gopkg/component/container/list_test.gopkg/component/container/runtime_discovery.gopkg/component/container/runtime_discovery_test.gopkg/component/emulator/executor.gopkg/component/emulator/executor_test.gopkg/container/detector.gopkg/container/detector_autostart_test.gopkg/container/identity.gopkg/container/identity_test.go
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2828 +/- ##
==========================================
+ Coverage 81.89% 81.90% +0.01%
==========================================
Files 1796 1797 +1
Lines 173711 173901 +190
==========================================
+ Hits 142255 142428 +173
- Misses 23674 23690 +16
- Partials 7782 7783 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Covers the Delete() error branch in FileCache, the invalidatingRuntime wrapper's delegate-and-invalidate-on-error behavior, the cached-runtime decode switch, resolved.runtime's error/env-forwarding branches, and DetectRuntimeWithPreferenceAndRecovery's new provider-selection switch. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Bumps the brace-expansion pnpm overrides from 1.1.13/2.0.3 to 1.1.17/2.1.3 (both within the existing major-version ranges, so not blocked by dependabot.yml's major-bump ignore policy; pnpm resolved 1.1.18/2.1.4, the latest patch releases satisfying those ranges). Verified by unpacking the tarballs that 1.1.18/2.1.4 contain the CVE-2026-14257 fix (EXPANSION_MAX_LENGTH bounding) while keeping the same `module.exports = expandTop` callable export shape that minimatch@3.1.5/9.0.9 require() against — unlike the full 5.0.8 bump, which changes that export shape and was reverted elsewhere for breaking those consumers. Fixes GHSA-mh99-v99m-4gvg / alert #261. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The "Review Dependency Licenses" check still flags brace-expansion
1.1.18/2.1.4 (the backported patch versions from the prior commit)
because the advisory's vulnerable_version_range ("<=5.0.7") predates
the maintainer's backport of the same fix into the 1.x/2.x lines --
GitHub's advisory data hasn't caught up to it. Bumping further to the
"official" first-patched version 5.0.8 isn't an option: its CommonJS
export shape change breaks minimatch@3.1.5/9.0.9's require() usage
(already tried and reverted elsewhere in this repo's history).
Mirrors the existing GHSA-fxhp-mv3v-67qp temporary-allowlist pattern
in this same file, with the verification and removal condition
documented inline.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Important Cloud Posse Engineering Team Review RequiredThis pull request modifies files that require Cloud Posse's review. Please be patient, and a core maintainer will review your changes. To expedite this process, reach out to us on Slack in the |
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 @.github/workflows/dependency-review.yml:
- Around line 72-74: Clarify the dependency-version wording in the workflow
comment near the brace-expansion verification: the website/package.json
overrides are semver ranges, so either change them to exact versions or state
that verification used the versions resolved in the lockfile rather than
“pinned” tarballs.
- Around line 68-80: Scope the GHSA-mh99-v99m-4gvg exception in the
dependency-review configuration to the verified brace-expansion versions
recorded in website/pnpm-lock.yaml, rather than allowing the advisory for every
resolved version. Preserve the existing GHSA-fxhp-mv3v-67qp exception and ensure
vulnerable versions such as <=5.0.7 cannot bypass review; remove the exception
instead if advisory metadata has been corrected.
🪄 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 Plus
Run ID: 11bee48d-e31e-449a-8236-7255b114bee3
📒 Files selected for processing (1)
.github/workflows/dependency-review.yml
Addresses two CodeRabbit findings on the prior commit: - website/package.json's pnpm overrides used ^1.1.17/^2.1.3 (semver ranges), but the workflow comment described them as "pinned" tarball versions. Changed to exact pins (1.1.18/2.1.4, the same versions already verified by unpacking their tarballs) so the wording is accurate and a future `pnpm install` can't silently drift to an unverified patch release within the range. - allow-ghsas suppresses GHSA-mh99-v99m-4gvg for every resolved brace-expansion version, not just 1.1.18/2.1.4 -- the action has no package/version-scoped vulnerability exception (only allow-dependencies-licenses is purl-scoped, and that's license-only, confirmed against the action's README). Documented this limitation inline and noted the exact override pins are what keeps the exception scoped in practice. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Tip Atmos Pro
No affected stacks workflow was detected for this pull request. |
|
These changes were released in v1.225.0-rc.3. |
what
why
references
Summary by CodeRabbit
New Features
Bug Fixes