fix: container/volume listing, Files for stopped containers, chart tooltips - #66
Conversation
Skip corrupt engine leftovers in lists, browse stopped-container files via archive, read volume paths from the guest on macOS, and fix chart tooltips / Stats styling. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Warning Review limit reached
Next review available in: 37 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughRuntime handling now distinguishes missing and stopped containers, supports browsing stopped-container files and guest volumes, filters invalid container entries, and makes volume inspection resilient. Build-history and stats charts use improved tooltips, while stopped-container and log presentation were adjusted. ChangesRuntime container handling
UI chart and state presentation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant _BuildHistoryChart
participant OverlayPortal
participant _BuildHistoryTooltip
User->>_BuildHistoryChart: Touch or hover a history point
_BuildHistoryChart->>OverlayPortal: Show or update tooltip overlay
OverlayPortal->>_BuildHistoryTooltip: Render selected build metrics
_BuildHistoryTooltip-->>User: Display duration, steps, and cached status
Possibly related PRs
🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
backend/internal/runtime/container_detail.go (1)
277-321: 🎯 Functional Correctness | 🔵 Trivial | ⚖️ Poor tradeoffField-count-based
tar -tvparsing is fragile across tar variants.Size/modified-time extraction relies on fixed field offsets (
fields[4],fields[5:8]/fields[5:7]). BusyBox tar (common in Alpine-based images), GNU tar, and BSD tar format their-tvcolumns differently (e.g., combined vs. separate owner/group fields), so this could silently mis-parse size or modified time depending on the container's base image.🤖 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 `@backend/internal/runtime/container_detail.go` around lines 277 - 321, Replace the fixed-index size and modified-time parsing in the tar listing parser with variant-tolerant extraction that handles BusyBox, GNU, and BSD tar column layouts. Update the logic surrounding mode, name, size, and modified assignment while preserving symlink handling, directory detection, and child deduplication.backend/internal/runtime/volume_detail.go (1)
254-289: 🚀 Performance & Scalability | 🔵 Trivial | ⚖️ Poor tradeoffPer-container inspect is now sequential — consider bounded concurrency for hosts with many containers.
The resiliency fix (continuing past a bad container instead of failing the whole batch) is correct, but it replaces one batched inspect with N sequential subprocess calls, adding latency to every volume-list request proportional to container count.
🤖 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 `@backend/internal/runtime/volume_detail.go` around lines 254 - 289, The volumeNamesInUse function now inspects containers sequentially, causing latency to scale linearly with container count. Add bounded concurrency around inspectContainer and parseContainerMounts, limiting the number of workers or in-flight subprocesses while preserving the current behavior of skipping uninspectable containers and returning the collected volume names.
🤖 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 `@backend/internal/runtime/container_detail.go`:
- Around line 63-67: Update the JSON parsing flow around json.Unmarshal in the
container detail retrieval function to handle errors separately: return or wrap
the actual unmarshal error when parsing fails, and retain ErrContainerNotFound
only for a successfully parsed empty rows result. Preserve the existing
successful path for non-empty rows.
- Around line 146-147: Update the command construction in the container detail
flow to pass dirPath through the existing shellQuote helper instead of
fmt.Sprintf("%q") before executing via sh -c. Preserve the existing shellQuote
usage for cp arguments in the archive helper.
In `@backend/internal/runtime/volume_detail.go`:
- Around line 301-305: Update enrichVolumesInUse to preserve the error returned
by volumeNamesInUse and emit a diagnostic log containing the failure details
before applying the existing empty inUse fallback; keep the volume enrichment
flow unchanged after logging.
---
Nitpick comments:
In `@backend/internal/runtime/container_detail.go`:
- Around line 277-321: Replace the fixed-index size and modified-time parsing in
the tar listing parser with variant-tolerant extraction that handles BusyBox,
GNU, and BSD tar column layouts. Update the logic surrounding mode, name, size,
and modified assignment while preserving symlink handling, directory detection,
and child deduplication.
In `@backend/internal/runtime/volume_detail.go`:
- Around line 254-289: The volumeNamesInUse function now inspects containers
sequentially, causing latency to scale linearly with container count. Add
bounded concurrency around inspectContainer and parseContainerMounts, limiting
the number of workers or in-flight subprocesses while preserving the current
behavior of skipping uninspectable containers and returning the collected volume
names.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 8490601f-3649-4886-aed2-10e86095ef88
📒 Files selected for processing (13)
CHANGELOG.mdbackend/internal/httpkit/runtime_errors.gobackend/internal/runtime/command_error.gobackend/internal/runtime/container_detail.gobackend/internal/runtime/errors.gobackend/internal/runtime/guest_darwin.gobackend/internal/runtime/nerdctl.gobackend/internal/runtime/volume_detail.gobackend/test/runtime/container_files_test.gobackend/test/runtime/nerdctl_test.goui/lib/screens/build_detail_screen.dartui/lib/screens/container_detail_screen.dartui/lib/widgets/logs_panel.dart
…and concurrency - Added detailed error messages for container inspection failures to aid debugging. - Implemented bounded concurrency for volume inspection to improve performance and prevent overwhelming the system. - Introduced new utility functions for parsing tar output, enhancing the handling of size and modification date extraction. - Added comprehensive tests for the new parsing logic to ensure correctness across different tar output formats.
1 participant
Summary
psbut cannot be inspected so Volumes and Containers lists no longer fail or show broken rows.cp+tar); map missing/not-running containers to clear HTTP errors.Test plan
Summary by CodeRabbit