Skip to content

chore(deps): take go-container v0.13.0 and drop the ExecResult workaround - #19

Merged
Snider merged 2 commits into
mainfrom
chore/container-v0.13.0
Aug 8, 2026
Merged

chore(deps): take go-container v0.13.0 and drop the ExecResult workaround#19
Snider merged 2 commits into
mainfrom
chore/container-v0.13.0

Conversation

@Snider

@Snider Snider commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Follow-up to #18. 73 deletions against 25 insertions — the receipt here is the diff shrinking.

Why

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's a plain {Stdout, Stderr, Exit} struct with nothing VZ-specific about it. #18 only ever worked around its placement.

What goes

  • the local vzExec type
  • the vzDecodeExec darwin/!darwin pair, which 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.

What stays

The platform split. IsVZAvailable and NewVZProvider are still behind //go:build darwin in vz.go (verified against v0.13.0, not assumed), so vzHostAvailable and newVZProviderImpl keep their pair, and the non-darwin provider still fails every verb rather than pretending to work.

Verification

check result
go build ./... (darwin) pass
go vet ./... (darwin) pass
go test ./... -count=1 (darwin) pass
gofmt -l pkg/agentic/ clean
go mod tidy leaves go.mod untouched
go build ./pkg/agentic/ (non-darwin half) pass
go test ./pkg/agentic/ (non-darwin half) pass

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 !darwin file 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

    • Improved consistency when handling execution results in supported virtualisation environments.
    • Maintained existing behaviour for unsupported platforms.
  • Refactor

    • Streamlined platform-specific virtualisation handling for clearer, more reliable execution output.
  • Chores

    • Updated underlying Go components to newer versions, improving compatibility and maintenance.

…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>
@coderabbitai

coderabbitai Bot commented Aug 8, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The VZ dispatcher now uses container.ExecResult directly. The platform-specific decoding helpers were removed, dependency versions were updated, and dispatcher tests now construct the container result type.

Changes

VZ execution result migration

Layer / File(s) Summary
Direct container result contract
go/go.mod, go/pkg/agentic/vz_platform_*.go, go/pkg/agentic/dispatch_vz.go
The container dependencies were updated. The Darwin and non-Darwin vzDecodeExec adapters were removed.
Dispatcher integration and validation
go/pkg/agentic/dispatch_vz.go, go/pkg/agentic/dispatch_vz_test.go
Completion handling and output formatting now use container.ExecResult. Tests preserve success, error, output, and exit-code assertions.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the dependency update and removal of the ExecResult workaround, which are the main changes in the pull request.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
go/pkg/agentic/dispatch_vz.go (1)

91-93: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Use 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 an ExecResult("vzfake01", "sh", "-c", "true") call and access to the returned container.ExecResult fields.
  • go/pkg/agentic/vz_platform_darwin.go#L11-L14: Show a container.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

📥 Commits

Reviewing files that changed from the base of the PR and between 1c68633 and 7e826f1.

⛔ Files ignored due to path filters (1)
  • go/go.sum is excluded by !**/*.sum
📒 Files selected for processing (5)
  • go/go.mod
  • go/pkg/agentic/dispatch_vz.go
  • go/pkg/agentic/dispatch_vz_test.go
  • go/pkg/agentic/vz_platform_darwin.go
  • go/pkg/agentic/vz_platform_other.go
💤 Files with no reviewable changes (1)
  • go/pkg/agentic/vz_platform_other.go

@codecov

codecov Bot commented Aug 8, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@Snider
Snider merged commit 1fd5971 into main Aug 8, 2026
6 of 8 checks passed
@Snider
Snider deleted the chore/container-v0.13.0 branch August 8, 2026 10:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant