test: E2E tests for Go SDK by jiripetrlik · Pull Request #3338 · NVIDIA/OpenShell

jiripetrlik · 2026-09-15T10:02:05Z

Summary

This PR adds E2E tests for Go SDK. 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

This PR should replace: jiripetrlik#1

Related Issue

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

Changes

Add E2E tests for Go SDK
Update Github actions to run Go SDK E2E tests

Testing

[*] mise run pre-commit passes
[] Unit tests added/updated - issue is about adding E2E tests. No unit tests are needed
[*] E2E tests added/updated (if applicable)

Checklist

[*] Follows Conventional Commits
[*] Commits are signed off (DCO)
[*] Architecture docs updated (if applicable)

copy-pr-bot · 2026-09-15T10:02:08Z

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

jiripetrlik · 2026-09-15T14:23:01Z

Hello @rhuss , I've tried to fix your comments related to assertion depth from #3028 (comment) and rebased PR. Can you please take a look again?

rhuss

cc-review Summary

What Went Well

The response-contract helpers are the real fix from the last round. requireProviderResponse (providers_test.go:38-47) and requireWorkspaceResponse (workspace_test.go:18-25) assert the full field set, including that credentials come back empty, which is exactly what the earlier review asked for.
attachProviderRetry and detachProviderRetry (providers_test.go:92-131) handle optimistic-concurrency conflicts by re-fetching ResourceVersion, rather than assuming the first write wins.
Test isolation is sound: uniqueName (harness_test.go:128-130) plus consistent t.Cleanup registration makes t.Parallel() safe, and cleanup detaches providers before deleting sandboxes so no credential material is left attached.
TestProviderCRUDResponseContract (providers_test.go:259-311) exercises Create, Get, Update and ListAll in sequence and checks ResourceVersion monotonicity at line 298.
A correctness pass over every SDK call against the signatures in sdk/go/openshell/v1/ found no misuse: no context lifetime errors, no resource leaks on failure paths, correct cleanup ordering under LIFO t.Cleanup.
The credential assertion tests a real security boundary correctly. Checked against crates/openshell-core/src/secrets.rs, the suite genuinely fails if the gateway returns a raw secret, an empty value, or an encoded copy.

Findings

Severity	File	Description	Source
Important	e2e/go/harness_test.go:104	waitForPersistenceReady uses context.Background() with no deadline	production
Important	e2e/go/providers_test.go:238	AttachProviderResult.Sandbox and DetachProviderResult.Sandbox never asserted	test-quality
Important	e2e/go/sandbox_lifecycle_test.go:27	No sandbox error-path tests (NotFound, AlreadyExists)	test-quality, goal-alignment
Important	e2e/go/sandbox_lifecycle_test.go:30	Sandbox Create under-asserted vs the workspace and provider helpers	test-quality
Important	e2e/go/sandbox_lifecycle_test.go:51	No failing-command exec test	goal-alignment
Important	tasks/test.toml:64	e2e:go puts a Podman requirement into the Docker-only e2e aggregate	production, architecture, goal-alignment
Important	tasks/test.toml:64	PR claims GitHub Actions were updated, but no .github/ file is touched	goal-alignment, codex
Important	tasks/test.toml:140	No -timeout on go test	production, goal-alignment
Minor	(10 findings)	helper placement, poll-helper duplication, stale comments, retry diagnostics, stderr and placeholder-matching details	various
Notable	(4 findings)	Spec round-trip and uncovered SDK surfaces, stable-handle placeholder form, RFC 0015 divergence	various

Four of these were raised in the review of the predecessor implementation and are still open: the failing-exec test, the sandbox error-path tests, the Podman and Docker mismatch in the aggregate, and the missing -timeout. The one item that was fixed, discarded response values, was fixed thoroughly for providers and workspaces. The sandbox tests did not get the same treatment.

Not posted inline (outside the diff hunks):

tasks/test.toml:4 and the [test] description on line 7 still read "Rust + Python + TypeScript SDK" and do not mention Go, although the [e2e] description on line 64 was updated.
mise.lock removes the sccache linux-x64 platform entry (5 lines). This is unrelated to the Go E2E work and undeclared. It looks like lock file churn from running mise on an ARM host; please confirm it was intentional.

Review Details

Findings posted: 20 inline, 2 summary only
Gate outcome: FAIL (8 Important, 0 Critical)
Participating agents: correctness, architecture, security, production, test-quality, goal-alignment, codex, coderabbit
Meta-review: 22 findings verified by a skeptic pass; 0 false positives removed, 6 marked weak, 1 severity raised. CodeRabbit reported no findings.

rhuss · 2026-09-17T07:42:01Z

+func waitForPersistenceReady(client *v1.Client) error {
+	ctx := context.Background()
+	var lastErr error
+	for range 60 {
+		_, err := client.Sandboxes().ListAll(ctx, "default", v1.ListOptions{PageSize: 1})
+		if err == nil {
+			return nil
+		}
+		lastErr = err
+		if v1.IsUnavailable(err) {
+			time.Sleep(2 * time.Second)
+			continue
+		}
+		if strings.Contains(err.Error(), "no such table: objects") {
+			time.Sleep(1 * time.Second)
+			continue
+		}
+		return fmt.Errorf("unexpected error waiting for persistence: %w", err)
+	}
+	return fmt.Errorf("openshell-server persistence is not initialized after 60 attempts: %w", lastErr)
+}


Important: waitForPersistenceReady uses context.Background() with no deadline, so a hung gRPC call blocks TestMain indefinitely.

Why this matters: Line 105 creates ctx := context.Background(). The 60-iteration attempt cap on line 107 only guards against repeated short failures. If a single ListAll call hangs (TCP socket open but gateway process stuck), the loop blocks forever because the context has no deadline. This runs in TestMain before any test executes, so it blocks the whole binary with no diagnostics.

Suggested fix: ctx, cancel := context.WithTimeout(context.Background(), 3*time.Minute); defer cancel()

Source: production agent

Added a three-minute deadline to gateway persistence readiness checks.

Confirmed. harness_test.go:108 now wraps the readiness loop in context.WithTimeout(context.Background(), 3*time.Minute), so a hung ListAll can no longer block TestMain.

rhuss · 2026-09-17T07:42:01Z

+	attachResult := attachProviderRetry(ctx, t, client, "default", sandboxName, providerName)
+	require.True(t, attachResult.Attached)
+
+	value = pollUntil(t, ctx, func() string {
+		v, err := readEnvVar(ctx, client, "default", sandboxName, "NVIDIA_API_KEY")
+		require.NoError(t, err)
+		return v
+	}, func(v string) bool { return v != "NOT_SET" })
+	require.True(t, isPlaceholderForEnvKey(value, "NVIDIA_API_KEY"), "expected a placeholder, got %q", value)
+
+	detachResult := detachProviderRetry(ctx, t, client, "default", sandboxName, providerName)
+	require.True(t, detachResult.Detached)


Important: AttachProviderResult.Sandbox and DetachProviderResult.Sandbox are never asserted. Both tests check only the boolean flag (Attached, Detached).

Why this matters: A gateway returning Attached: true with a nil or zero-valued Sandbox would pass. The embedded Sandbox is the primary mechanism for clients to observe post-mutation state (updated provider list, new resource version), so serialization bugs in the attach and detach response path cannot be detected.

Suggested fix: After attachProviderRetry assert NotNil(attachResult.Sandbox), Equal(sandbox.ID, attachResult.Sandbox.ID) and that the provider list contains the attached provider. Make the symmetric assertions after detach.

Source: test-quality agent

Added assertions for the returned sandbox identity and provider attachment state.

Confirmed, and you went further than the comment asked. Both results now run through requireSandboxResponse, assert the sandbox identity against the created ID, and check Spec.Providers membership with Contains after attach and NotContains after detach. That covers the post-mutation state the boolean flag was hiding.

rhuss · 2026-09-17T07:42:01Z

+	sb, err := client.Sandboxes().Create(ctx, "default", name, &v1.SandboxSpec{
+		Policy: defaultPolicy(),
+	}, nil)
+	require.NoError(t, err)
+	t.Cleanup(func() {
+		cleanupCtx, cleanupCancel := context.WithTimeout(context.Background(), 2*time.Minute)
+		defer cleanupCancel()
+		require.NoError(t, client.Sandboxes().Delete(cleanupCtx, "default", name))
+	})
+	require.NotEmpty(t, sb.ID)
+	require.Equal(t, name, sb.Name)
+
+	ready, err := client.Sandboxes().WaitReady(ctx, "default", name)
+	require.NoError(t, err)
+	requireReadySandbox(t, ready, sb.ID)
+
+	fetched, err := client.Sandboxes().Get(ctx, "default", name)
+	require.NoError(t, err)
+	require.Equal(t, sb.ID, fetched.ID)
+
+	sandboxes, err := client.Sandboxes().ListAll(ctx, "default", v1.ListOptions{PageSize: 100})
+	require.NoError(t, err)
+	require.True(t, containsID(sandboxes, sb.ID))
+
+	result, err := client.Exec().Run(ctx, "default", name, []string{"sh", "-c", "printf sandbox-ok"}, v1.ExecOptions{})
+	require.NoError(t, err)
+	require.Equal(t, 0, result.ExitCode)
+	require.Equal(t, "sandbox-ok", string(result.Stdout))
+
+	// Exec launches share the same sandbox filesystem across calls.
+	writeResult, err := client.Exec().Run(ctx, "default", name,
+		[]string{"sh", "-c", "echo persisted > /sandbox/exec-persistence.txt"}, v1.ExecOptions{})
+	require.NoError(t, err)
+	require.Equal(t, 0, writeResult.ExitCode)
+
+	readResult, err := client.Exec().Run(ctx, "default", name,
+		[]string{"cat", "/sandbox/exec-persistence.txt"}, v1.ExecOptions{})
+	require.NoError(t, err)
+	require.Equal(t, 0, readResult.ExitCode)
+	require.Equal(t, "persisted\n", string(readResult.Stdout))


Important: No error-path tests for sandbox operations: Get on a nonexistent sandbox (NotFound), Create with a duplicate name (AlreadyExists), Delete on a nonexistent sandbox. The workspace tests cover both NotFound cases.

Why this matters: The SDK error helpers IsNotFound and IsAlreadyExists depend on correct gRPC status mapping through the converter layer. If the gateway returns a different status for a sandbox 404 than for a workspace 404, the current suite would not catch it. Flagged as R3 in the prior review and unresolved.

Suggested fix: Add TestSandboxGetNonexistentNotFound and TestSandboxDeleteNonexistentNotFound mirroring the workspace tests; optionally TestSandboxCreateDuplicateAlreadyExists.

Source: test-quality agent (also flagged by: goal-alignment)

Added NotFound coverage for get/delete and AlreadyExists coverage for duplicate creation.

Confirmed. TestSandboxGetNonexistentNotFound, TestSandboxDeleteNonexistentNotFound and TestSandboxCreateDuplicateAlreadyExists now give the sandbox path the same error-classification coverage the workspace tests had.

rhuss · 2026-09-17T07:42:01Z

+	require.NoError(t, err)
+	t.Cleanup(func() {
+		cleanupCtx, cleanupCancel := context.WithTimeout(context.Background(), 2*time.Minute)
+		defer cleanupCancel()
+		require.NoError(t, client.Sandboxes().Delete(cleanupCtx, "default", name))
+	})
+	require.NotEmpty(t, sb.ID)
+	require.Equal(t, name, sb.Name)


Important: Sandbox Create response is under-asserted relative to the workspace and provider equivalents. Only ID (non-empty) and Name are checked; Workspace, CreatedAt and ResourceVersion are not.

Why this matters: A gateway returning an empty Workspace, zero CreatedAt or zero ResourceVersion on sandbox Create would pass every test. For a suite whose stated purpose is catching protocol schema mismatches, leaving the primary resource type less validated than the secondary ones is a real gap. requireProviderResponse and requireWorkspaceResponse prove the pattern already exists in this same suite.

Suggested fix: Add requireSandboxResponse(t, sandbox, workspace, name) asserting NotNil, NotEmpty(ID), Equal(workspace, Workspace), Equal(name, Name), False(CreatedAt.IsZero()), NotZero(ResourceVersion). Use it after Create and Get in TestSandboxCRUDAndExec and after each Create in TestSandboxListScopedAndAllWorkspaces.

Source: test-quality agent

Added shared assertions for ID, workspace, name, creation time, and resource version.

Confirmed. requireSandboxResponse in harness_test.go:137 asserts ID, workspace, name, CreatedAt and ResourceVersion, and it is applied after both Create and Get. The sandbox response contract now matches the provider and workspace helpers.

rhuss · 2026-09-17T07:42:01Z

+	result, err := client.Exec().Run(ctx, "default", name, []string{"sh", "-c", "printf sandbox-ok"}, v1.ExecOptions{})
+	require.NoError(t, err)
+	require.Equal(t, 0, result.ExitCode)
+	require.Equal(t, "sandbox-ok", string(result.Stdout))


Important: Exec is tested only on the happy path. No test runs a failing command and asserts a non-zero ExitCode or stderr content.

Why this matters: Without a failing-command test the suite cannot detect regressions where the gateway swallows non-zero exit codes or drops stderr. Flagged as R2 in the prior review and unresolved.

Suggested fix: Add a case running sh -c 'echo err >&2; exit 42' asserting ExitCode == 42 and that stderr contains the sentinel.

Source: goal-alignment agent

Added a nonzero-exit execution test. I still need to require exact stderr and enable NoLoginShell for RFC-0015 compliance.

Confirmed for the exit code and stdout: the new case asserts exit 42, empty stdout and the stderr sentinel. Agreed on your note that exact stderr equality and NoLoginShell should wait for the RFC 0015 exec definition, which now pins tty=false and no_login_shell=true. Worth doing in the same change that aligns the adapter.

rhuss · 2026-09-17T07:42:01Z

+	result, err := client.Exec().Run(ctx, "default", name, []string{"sh", "-c", "printf sandbox-ok"}, v1.ExecOptions{})
+	require.NoError(t, err)
+	require.Equal(t, 0, result.ExitCode)
+	require.Equal(t, "sandbox-ok", string(result.Stdout))


Minor: Exec success results never assert that Stderr is empty. All exec calls check ExitCode and Stdout but discard Stderr (lines 51-54, 58-60, 62-66).

Why this matters: A gateway that erroneously includes error output in a successful execution would pass undetected. Note that this assertion is only well defined for non-TTY exec, since a PTY merges the two streams.

Suggested fix: Add require.Empty(t, result.Stderr) after each successful exec assertion, and confirm the exec runs without a TTY.

Source: test-quality agent

Added empty-stderr assertions to successful execution paths.

Confirmed. require.Empty on Stderr is present on all three successful exec paths.

rhuss · 2026-09-17T07:42:01Z

+func elementsMatch(a, b []string) bool {
+	if len(a) != len(b) {
+		return false
+	}
+	counts := make(map[string]int, len(a))
+	for _, v := range a {
+		counts[v]++
+	}
+	for _, v := range b {
+		counts[v]--
+	}
+	for _, c := range counts {
+		if c != 0 {
+			return false
+		}
+	}
+	return true
+}


Minor: elementsMatch hand-rolls unordered set equality by map counting.

Why this matters: Go 1.25 provides slices.Sorted and slices.Equal in the standard library, which would replace the counting logic with a one-liner and remove the surface for off-by-one bugs. It exists because the poll helper needs a boolean predicate rather than testify's fatal require.ElementsMatch.

Suggested fix: Use slices.Equal(slices.Sorted(slices.Values(a)), slices.Sorted(slices.Values(b))), or fold it into the generic pollUntil[T].

Source: architecture agent

Replaced the counting implementation with sorted slice comparison.

Confirmed. slices.Equal(slices.Sorted(slices.Values(a)), slices.Sorted(slices.Values(b))) replaces the map counting.

rhuss · 2026-09-17T07:42:01Z

+func uniqueName(prefix string) string {
+	return fmt.Sprintf("%s-%09d", prefix, time.Now().UnixNano()%1_000_000_000)
+}


Notable: Draft RFC 0015 (PR #3238) proposes shared scenario identifiers across SDK E2E suites and a task name of the form e2e:sdk:go. The current task is e2e:go and the tests carry no scenario identifiers.

Why this matters: The RFC is not normative yet so no action is required. The file-per-domain structure (sandbox, workspace, providers) aligns with the RFC's proposed groupings, which would ease a later migration.

Suggested fix: No fix now. If RFC 0015 is adopted, rename the task and annotate each test with its scenario identifier.

Source: architecture agent

Agreed. The current task split predates the RFC contract; it still needs e2e:sdk:go:adapter, Docker-backed e2e:sdk:go, and the e2e:go alias.

Acknowledged, and your read matches mine. Nothing to do here until RFC 0015 is accepted; the task split and scenario identifiers belong in the same change that introduces the adapter contract.

rhuss · 2026-09-17T07:42:01Z

+func isPlaceholderForEnvKey(value, key string) bool {
+	const prefix = "openshell:resolve:env:"
+	if value == prefix+key {
+		return true
+	}
+	token, ok := strings.CutPrefix(value, prefix)
+	if !ok {
+		return false
+	}
+	return strings.HasPrefix(token, "v") && strings.HasSuffix(token, "_"+key)
+}


Notable: isPlaceholderForEnvKey does not recognize the stable-handle placeholder variant openshell:resolve:env:s<64 lowercase hex>_KEY defined in crates/openshell-core/src/secrets.rs (split_stable_env_key, lines 802-815).

Why this matters: The security invariant is preserved: a stable-handle placeholder makes the test fail rather than silently pass, so a leaked secret is still caught. But if the gateway begins issuing stable handles for refresh-managed credentials, these tests produce false failures with a misleading diagnostic that does not indicate the value is a valid placeholder in another format.

Suggested fix: Extend isPlaceholderForEnvKey to accept the stable-handle form, ideally with one compiled regexp covering all three variants: ^openshell:resolve:env:(?:KEY|v[0-9]+_KEY|s[0-9a-f]{64}_KEY)$

Source: security agent

Added strict support for the s<64-lowercase-hex>_ placeholder form.

Confirmed. The s<64 lowercase hex>_KEY form is now accepted with a strict length and hex check, which matches split_stable_env_key in crates/openshell-core/src/secrets.rs. The separate TestIsPlaceholderForEnvKey unit test is a good addition, since the matcher is now the security assertion's load-bearing piece.

rhuss · 2026-09-17T07:42:01Z

+	sb, err := client.Sandboxes().Create(ctx, "default", name, &v1.SandboxSpec{
+		Policy: defaultPolicy(),
+	}, nil)
+	require.NoError(t, err)
+	t.Cleanup(func() {
+		cleanupCtx, cleanupCancel := context.WithTimeout(context.Background(), 2*time.Minute)
+		defer cleanupCancel()
+		require.NoError(t, client.Sandboxes().Delete(cleanupCtx, "default", name))
+	})
+	require.NotEmpty(t, sb.ID)
+	require.Equal(t, name, sb.Name)


Notable: Sandbox Spec round-trip is never verified, and several SDK surfaces are uncovered: Stop, Start, WaitStopped, Watch, GetLogs, ListProviders, Exec Stream and Interactive, and the paginated List path.

Why this matters: Sandboxes are created with defaultPolicy() containing filesystem, landlock and process policy, but the returned or fetched Spec.Policy is never compared. Policy serialization across the gRPC boundary is exactly the kind of schema mismatch this suite exists to catch. These are scope decisions rather than bugs and are reasonable for a first E2E PR.

Suggested fix: Consider a TestSandboxSpecRoundTrip that creates with a detailed policy, fetches via Get and deep-compares the Spec. Track the remaining interface methods and paginated List as follow-up coverage.

Source: test-quality agent

Added policy round-trip assertions and stronger CRUD coverage. Scenario IDs, coverage reporting, scavenging, and remaining RFC error cases are still outstanding.

Confirmed for the round trip: the policy is compared against Spec.Policy after both Create and Get, which is the schema-mismatch case I cared about most. The remaining surfaces (Stop, Start, Watch, GetLogs, paginated List, Exec streaming) stay as follow-up coverage, and scenario IDs, coverage reporting and scavenging belong with the RFC 0015 alignment.

jiripetrlik · 2026-09-18T17:16:24Z

Hello @rhuss , thank you for your review. I've tried to fix all the issues. See comments for individual problems. Here is also how I've fixed additional comments:

Test tasks. [test] remains unchanged because it does not run Go; go:ci is a separate dependency of ci.)
Updated the file-level header. I left the [test] description unchanged because that task does not execute Go tests; the full ci task runs go:ci separately.
The removal was unintended ARM-host lockfile churn. I restored the Linux x64 sccache entry.

rhuss · 2026-09-20T06:04:09Z

Second pass on 83132744. All 20 inline findings are addressed and verified against the code, and both summary-only items are handled: mise.lock no longer differs from the base, and the tasks/test.toml header is updated with a reasonable argument for leaving the [test] description alone.

Three of the fixes are worth calling out because they went past what was asked.

The Podman and Docker split landed in the right shape: e2e:go:docker joins the aggregate while e2e:go stays a standalone Podman lane, mirroring how e2e:podman already sits outside it. The CI wiring actually takes effect, which was the part most likely to go wrong silently. branch-e2e.yml:202-213 calls the Docker workflow without overriding suite-matrix, so the new go entry is used, and Go 1.26.7 is locked for linux-arm64 in mise.lock to match the runner. And the placeholder matcher is now the strict parser the credential assertion deserves, with its own unit test.

Beyond the findings, the attach and detach assertions now check Spec.Providers membership in both directions, and the policy round trip is compared after both Create and Get. Those close the schema-mismatch gap the suite exists for.

The three items you deferred are the right ones to defer, and I agree with your reasoning on each: exact stderr with NoLoginShell, removing the conflict retries, and scenario identifiers with coverage reporting. All three are RFC 0015 alignment rather than defects in this PR, and RFC 0015 now pins tty = false and no_login_shell = true for exec scenarios, so the exec item has a definition to code against once that lands.

Nothing blocking remains from my side. I cannot resolve review threads on this repository, so the 20 threads need resolving on your end. After that this needs a vetter to comment /ok to test so the gated lanes, including the new Go suite, actually run.

rhuss · 2026-09-20T06:08:19Z

@mrunalp @sjenning @krishicks I think we are good to go with this PR (along with the associated RFC), to at least run it through the CI pipeline. It really would help to keep the Golang SDK stable over the long run and detect regressions earlier. (i'm off next week, therefor this eager ping :-)

Signed-off-by: Jiri Petrlik <jpetrlik@redhat.com>

jiripetrlik · 2026-10-02T12:19:38Z

Hello @mrunalp , @sjenning , @krishicks , can you please take a look at this PR? I've rebased it with current main. It will help us to better test Go SDK using E2E tests.

rhuss · 2026-10-02T14:07:24Z

would be really great if we could integrate the tests, either now or when we moved to the CNFC repos (under another GitHub org then, I guess). @jiripetrlik wdyt, would it be better to wait to this move (should happen quite soon, now that OpenShell has been accepted for the CNCF sandbox) or does it not really matter?

jiripetrlik requested review from a team, derekwaynecarr, mrunalp and sjenning as code owners September 15, 2026 10:02

jiripetrlik mentioned this pull request Sep 15, 2026

test: E2E tests for Go SDK jiripetrlik/OpenShell#1

Closed

rhuss suggested changes Sep 17, 2026

View reviewed changes

jiripetrlik force-pushed the go-sdk-e2e-tests branch from f938393 to 8313274 Compare September 18, 2026 16:32

test: E2E tests for Go SDK

00cd9ae

Signed-off-by: Jiri Petrlik <jpetrlik@redhat.com>

jiripetrlik force-pushed the go-sdk-e2e-tests branch from 8313274 to 00cd9ae Compare October 2, 2026 10:56

Conversation

jiripetrlik commented Sep 15, 2026

Summary

Related Issue

Changes

Testing

Checklist

Uh oh!

copy-pr-bot Bot commented Sep 15, 2026

Uh oh!

jiripetrlik commented Sep 15, 2026

Uh oh!

rhuss left a comment

Choose a reason for hiding this comment

cc-review Summary

What Went Well

Findings

Review Details

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jiripetrlik commented Sep 18, 2026