feat(e2e/go): E2E tests for the Go SDK

[jiripetrlik]

User Story

As a contributor working on OpenShell Go SDK I want mise run e2e:go to run E2E tests against a real
gateway. Regressions in sandbox lifecycle, exec, provider credential injection, and workspace management will be caught before they reach SDK consumers.

Problem Statement

The Go SDK (sdk/go/) is tested only by unit tests and sdk/go/openshell/v1/integration_test.go, which is only a very basic smoke E2E test. New E2E Go SDK test suite should be able to catch issues with:

• protocol schema mismatches between the Go SDK and the actual gateway build
• sandbox lifecycle can be managed by the Go SDK
• provider credentials management using the Go SDK
• workspace management

Impact / Why This Matters

The Go SDK (sdk/go/) is tested mostly by unit tests. These tests mock the transport and never talk to a real openshell-gateway. Which means that currently:

• A gateway change can silently break every Go SDK consumer
• Contributors have no mise run entry point to test Go SDK changes end-to-end before opening PR

Proposed Design

• Separate module: E2E tests live in e2e/go/
• No SDK changes: sdk/go/ is untouched

Possible design can be found in draft PR: jiripetrlik#1

Acceptance Criteria

• e2e/go/ is a separate Go module
• All e2e files has //go:build e2e
• Sandbox tests cover CRUD
• Provider tests assert placeholders (not raw secrets)
• Workspace tests cover CRUD
• AGENTS.md documents the e2e/go/ conventions
• mise run pre-commit passes; commits are DCO-signed and follow Conventional Commits

Alternatives Considered

• Rely on unit tests - can be faster to test, but many possible bugs can be missed
• Put the e2e tests inside `sdk/go/ - put E2E tests to a separated directory is much more clear

Agent Investigation

No response

Checklist

• I've reviewed existing issues and the architecture docs
• This is a design proposal, not a "please build this" request

[jiripetrlik]

Hello @rhuss , could you please take a look if you think it is fine or if something is missing in this issue?

[rhuss]

I will but i'm still on PTO, back on Wed with no access to a computer

[rhuss]

Agent review of this proposal and of the reference implementation in jiripetrlik#1 (commit 6d771e49).

I ran a set of specialized review agents over it (correctness, architecture, security, production readiness, test quality, goal alignment) along with CodeRabbit, then put the merged findings through a skeptic pass to drop false positives. All 16 findings below survived that pass with line level evidence. Treat the severities as a starting point for discussion rather than a verdict, and push back where the agents got it wrong. Happy to go deeper on any of them.

What already works

The implementation is a solid first cut, and all seven acceptance criteria in this issue are met: separate module with a replace directive to ../../sdk/go, //go:build e2e on every file, sandbox and workspace CRUD, placeholder assertions, and the AGENTS.md note.

A few things stood out:

• The credential assertions test a real security boundary correctly. providers_test.go asserts both that the sandbox sees a placeholder and that the raw secret is not returned, plus NOT_SET for profileless providers. isPlaceholderForEnvKey tolerating both the bare and versioned placeholder formats is a nice touch, since it will survive a format bump.
• Deletion is treated as asynchronous. pollUntilNamesMatch and waitForWorkspaceSandboxesGone poll with bounded deadlines rather than assuming immediate consistency, and the comments explain why.
• Parallel isolation is clean. uniqueName plus consistent t.Cleanup registration means t.Parallel() is safe throughout, and cleanup ordering (workspace registered before sandboxes, detach before delete) is correct.
• SDK API usage is correct throughout. The correctness pass checked every call against the SDK signatures and found no mismatches, no context lifetime errors, and no resource leaks on failure paths.

The branch needs a rebase first

It is 108 commits behind main, and both non-test files it touches have moved underneath it:

• .github/workflows/e2e-test.yml no longer exists. It was split into e2e-docker-test.yml, e2e-podman-test.yml, e2e-vm-test.yml, e2e-kubernetes-test.yml and e2e-gpu-test.yaml. The new e2e-podman-go job needs to land in e2e-podman-test.yml (which takes a suite-matrix input) or in a new e2e-go-test.yml.
• tasks/test.toml has since gained e2e:conformance:build, e2e:cli-conformance and e2e:websocket-conformance, and changed e2e:rust and e2e:podman. The added e2e:go section will conflict across several hunks.

Worth doing before the review goes much further, since the workflow job has to be restructured anyway.

Assertion depth is the one substantive gap

The stated purpose is catching protocol schema mismatches between the SDK and a real gateway build. As written, the suite would miss a good number of them, because response values are mostly discarded:

• All three Providers().Create() calls use _, err :=. The returned Provider has ten fields and none is asserted, so a gateway returning a zero-valued Provider would pass all three tests.
• WaitReady()'s return value is discarded at all four call sites, so Status.Phase after readiness is never checked. Create asserts only that ID is non-empty and Name matches.
• Providers().Get(), .List() and .Update() are never called anywhere in e2e/go/.
• Workspace tests assert Name, Phase and Labels, but never ID, CreatedAt or ResourceVersion.

Capturing those return values and asserting on the fields would close most of the gap without adding new tests. Adding a provider Get/List/Update round trip would close the rest.

Smaller items

Ten lower-severity findings

CI

• e2e-podman-go duplicates roughly 135 lines from e2e-podman-rootless (env, checkout, mise install, Podman install, rootless config, capability probe, GHCR login). A fourth matrix entry would inherit all of it. Worth reconsidering during the rebase anyway.
• That duplication has already diverged: the Go job omits the apparmor package and both AppArmor steps ("Allow pasta to receive Podman stop signals" and the SIGTERM denial check). Without the profile patch, Podman falls back to SIGKILL and sandbox cleanup blocks until the timeout, so the Go job will be slower and flakier than the Rust one. Also flagged independently by CodeRabbit.
• No gateway log capture on failure. The only if: always() step prints AppArmor denials, so a future failure shows the Go assertion with no server side context. The existing Podman job has the same gap, so this is inherited rather than new, and probably better fixed repo wide.

Task wiring

• The [e2e] aggregator now depends on e2e:go, which uses with-podman-gateway.sh, while e2e:rust, e2e:python and e2e:mcp all use with-docker-gateway.sh. That makes mise run e2e silently require both Docker and Podman. The deliberately Podman specific suites are kept out of the aggregator for this reason. Either switch e2e:go to Docker and add a separate e2e:go:podman for the CI lane, or leave it out of the aggregator.
• go test runs without -timeout, so Go's 10 minute default applies. The per test contexts already add up close to that: five minutes in TestSandboxCRUDAndExec, three minutes each for the provider tests, and up to two minutes in waitForPersistenceReady. On a cold runner the Go timeout fires first and produces a goroutine dump instead of a readable failure.

Coverage

• Exec is only tested on the happy path. Nothing runs a failing command and asserts on a non-zero ExitCode or on Stderr content.
• No sandbox error path tests. The workspace tests cover NotFound for both Get and Delete; the sandbox tests have no equivalent.

Test code

• pollUntilNamesMatch and pollUntil are the same poll with deadline pattern differing only in value type. go.mod declares Go 1.25, so one generic helper would cover both.
• attachProviderRetry and detachProviderRetry poll at a fixed 200ms with no backoff and no attempt cap, bounded only by the 3 minute context. Low impact in practice, but a persistent conflict would mean roughly 900 requests and a lot of log noise.
• waitForPersistenceReady sleeps on its final loop iteration before the loop exits. Costs a second or two on the timeout path only.

Before this gets marked validated

The proposal itself is sound and the premise checks out, but a few things are open:

1. How is the gateway bootstrapped? This is the important one. The Python and Rust suites use e2e/with-docker-gateway.sh, which exports OPENSHELL_GATEWAY_ADDRESS. The issue does not say, and the implementation picked Podman, which is what causes the aggregator problem above.
2. The User Story names exec as a regression surface, but no acceptance criterion covers it. Should it be added?
3. Should e2e:go join the [e2e] aggregator, given it is currently Docker only?
4. Why a separate module rather than extending integration_test.go, which already connects to a real gateway? A short rationale in the issue would help, since it is the main structural decision here.
5. How does this relate to openshell-conformance? As far as the review could tell, the conformance suite is CLI driven and therefore cannot validate SDK level API contracts, which would justify a separate Go suite, but the issue should say so.
6. The Impact section covers the consequences of the current behavior but not the current workaround or why it is insufficient.

One small correction on the problem statement: there are 82 test files under sdk/go/, so "only unit tests" undersells the existing coverage. The substantive point holds though. Every one of the 15 client test files uses bufconn, so nothing currently talks to a real gateway, and while integration_test.go does read OPENSHELL_GATEWAY_ADDRESS, there is no mise task or shell driver to bring a gateway up for it. That gap is real and worth closing.

[krishicks]

I think we should do this, but it needs to be applicable to all SDKs. This one may benefit from an RFC, because I think we need an SDK conformance test suite that we can define in some DSL and set expectations (what does, doesn't work) for each SDK.

[jiripetrlik]

@rhuss @krishicks
Hello and thank you for your feedback! I've tried to create a draft of the RFC here: #3238