chore(deps): take go-container v0.13.0 and drop the ExecResult workaround - #19
Conversation
…ound
v0.13.0 moves ExecResult out of the darwin-tagged vz.go into provider.go with
no build constraint, which is where it always belonged — it is a plain
{Stdout, Stderr, Exit} struct with nothing VZ-specific about it, and #18 only
worked around its placement.
So the shim goes: the local vzExec type, and the vzDecodeExec pair that
existed solely to convert into it because the real type could not be named on
Linux. dispatch_vz.go asserts execResult.Value.(container.ExecResult) directly
again, vzExecOutput takes the real type, and the unit tests script the fake
dispatcher with container.ExecResult rather than a stand-in.
The platform split itself stays. IsVZAvailable and NewVZProvider are still
declared behind //go:build darwin in vz.go, so vzHostAvailable and
newVZProviderImpl keep their darwin/!darwin pair, and the non-darwin provider
still fails every verb rather than pretending.
Net 73 deletions against 25 insertions.
Verified on darwin: go build ./..., go vet ./... and go test ./... all pass,
gofmt clean, go mod tidy leaves go.mod untouched. The non-darwin half is
verified by temporarily inverting the two constraints so it compiles here —
build and the full pkg/agentic suite both pass under that inversion too. The
real Linux build is CI's to confirm, as this machine has no CGO toolchain for
the duckdb bindings.
Co-Authored-By: Virgil <virgil@lethean.io>
📝 WalkthroughWalkthroughThe VZ dispatcher now uses ChangesVZ execution result migration
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
Warning Billing warning: we have not been able to collect payment for this subscription for more than 72 hours. Please update the payment method or pay any pending invoices in Billing to avoid service interruption. 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.
🧹 Nitpick comments (1)
go/pkg/agentic/dispatch_vz.go (1)
91-93: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse usage examples in both changed Go comment blocks.
The comments describe implementation details instead of showing how to use the code. Replace or supplement each block with a concrete example using real values.
go/pkg/agentic/dispatch_vz.go#L91-L93: Show anExecResult("vzfake01", "sh", "-c", "true")call and access to the returnedcontainer.ExecResultfields.go/pkg/agentic/vz_platform_darwin.go#L11-L14: Show acontainer.IsVZAvailable()check before creating the VZ provider.🤖 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 `@go/pkg/agentic/dispatch_vz.go` around lines 91 - 93, Replace the implementation-focused comment at go/pkg/agentic/dispatch_vz.go lines 91-93 with a concrete ExecResult("vzfake01", "sh", "-c", "true") usage example that demonstrates accessing the returned container.ExecResult fields. Also update the comment at go/pkg/agentic/vz_platform_darwin.go lines 11-14 with an example checking container.IsVZAvailable() before creating the VZ provider.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.
Nitpick comments:
In `@go/pkg/agentic/dispatch_vz.go`:
- Around line 91-93: Replace the implementation-focused comment at
go/pkg/agentic/dispatch_vz.go lines 91-93 with a concrete ExecResult("vzfake01",
"sh", "-c", "true") usage example that demonstrates accessing the returned
container.ExecResult fields. Also update the comment at
go/pkg/agentic/vz_platform_darwin.go lines 11-14 with an example checking
container.IsVZAvailable() before creating the VZ provider.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 1026f9ba-2fa9-4f9f-8f7f-b73b5e580686
⛔ Files ignored due to path filters (1)
go/go.sumis excluded by!**/*.sum
📒 Files selected for processing (5)
go/go.modgo/pkg/agentic/dispatch_vz.gogo/pkg/agentic/dispatch_vz_test.gogo/pkg/agentic/vz_platform_darwin.gogo/pkg/agentic/vz_platform_other.go
💤 Files with no reviewable changes (1)
- go/pkg/agentic/vz_platform_other.go
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Follow-up to #18. 73 deletions against 25 insertions — the receipt here is the diff shrinking.
Why
v0.13.0 moves
ExecResultout of the darwin-taggedvz.gointoprovider.gowith no build constraint, which is where it always belonged: it's a plain{Stdout, Stderr, Exit}struct with nothing VZ-specific about it. #18 only ever worked around its placement.What goes
vzExectypevzDecodeExecdarwin/!darwinpair, which existed solely to convert into it because the real type could not be named on Linuxdispatch_vz.goassertsexecResult.Value.(container.ExecResult)directly again,vzExecOutputtakes the real type, and the unit tests script the fake dispatcher withcontainer.ExecResultrather than a stand-in.What stays
The platform split.
IsVZAvailableandNewVZProviderare still behind//go:build darwininvz.go(verified against v0.13.0, not assumed), sovzHostAvailableandnewVZProviderImplkeep their pair, and the non-darwin provider still fails every verb rather than pretending to work.Verification
go build ./...(darwin)go vet ./...(darwin)go test ./... -count=1(darwin)gofmt -l pkg/agentic/go mod tidygo.moduntouchedgo build ./pkg/agentic/(non-darwin half)go test ./pkg/agentic/(non-darwin half)The non-darwin half is checked by temporarily inverting the two build constraints so it compiles on this machine — the same technique added in #18 after a
!darwinfile shipped broken precisely because it never compiles on a Mac. The real Linux build is CI's to confirm; this machine has no CGO toolchain for the duckdb bindings, so a local Linux green would be a guess rather than a receipt.🤖 Generated with Claude Code
Co-Authored-By: Virgil virgil@lethean.io
Summary by CodeRabbit
Bug Fixes
Refactor
Chores