[rhuss]

Originally NVIDIA/OpenShell#3662 by @letv1nnn. Thanks for the contribution!

Motivation

SDK clients that monitor a sandbox (operators, CLIs, dashboards) lose log and platform events when a stream drops or lags, and today they can only restart observation from scratch. GetLogs is a one-shot read and Watch only reports status snapshots. WatchLogs resumes from the last delivered cursor, so a reconnect neither loses nor duplicates events, and unrecoverable gaps fail loudly with ErrorOutOfRange (NVIDIA/OpenShell#3055).

Summary

Add WatchLogs to the Go SDK, a loss-aware, resumable log and platform event stream helper that mirrors the Rust SDK's reconnect, backoff, and cursor-resume semantics. Recoverable loss surfaces as WatchLogKindWarning events; unrecoverable loss (trimmed cursor) terminates with ErrorOutOfRange.

Related Issue

Refs NVIDIA/OpenShell#3055 (partial, item B of NVIDIA/OpenShell#3209 TODO).

Changes

• New API: SandboxLogsWatchInterface.WatchLogs(ctx, workspace, name, opts...) (via client.SandboxLogsWatch(), keeping SandboxInterface unchanged) returns WatchInterface[*WatchLogEvent] with transparent reconnect on Unavailable, capped exponential backoff (100ms→2s) reset on each delivered event, and cursor-aware replay from the highest observed cursor.
• Domain types: PlatformEvent, WatchLogEvent{Kind, Log, Event, Warning, Cursor}, WatchLogKind{Log,Event,Warning}, WatchLogsOptions{FollowLogs, FollowEvents, LogSources, LogMinLevel, ResumeAfterCursor, LogTailLines, EventTail}.
• Error code: New ErrorOutOfRange to distinguish terminal gaps (cursor trimmed/from dead cursor space) from invalid arguments. Remapped gRPC OutOfRange from ErrorInvalidArgument → ErrorOutOfRange.
• Converter: PlatformEventFromProto, WatchLogEventFromProto with deep-copy semantics; returns nil for non-resumable payloads (status snapshots, draft policy updates).
• Fake client: Returns ErrorUnimplemented (matching GetLogs, no replayable buffer contract).
• Tests: 13 integration tests covering payload delivery, reconnect-no-loss/no-dup, high-water-mark cursor semantics, warning passthrough, OUT_OF_RANGE terminal (no retry), backoff reset, Stop/context cancellation, and error cases. 8 converter tests + error mapping + code string tests.
• Docs: WatchLogs section in docs/src/api/sandboxes.md with usage examples, loss semantics, and cursor contract. Error-handling doc updated with IsOutOfRange predicate.

Testing

• mise run pre-commit passes
• Unit tests added/updated
• E2E tests added/updated (if applicable)

Checklist

• Follows Conventional Commits
• Commits are signed off (DCO)
• Architecture docs updated (if applicable)

---

Port note

Ported from NVIDIA/OpenShell#3662 by @letv1nnn after the Go SDK moved out of NVIDIA/OpenShell at 8ed8bf0c907fd06572d616d64f22ede915d0c10f. The commits are the original ones from the core PR at 6186bba777cb6b9af835441a26f6ea292d8f0a87, with the same authors and sign-offs; the only change is the module path (github.com/NVIDIA/OpenShell/sdk/go is now github.com/ai-openshell/sdk-go). The core PR changed only sdk/go, so nothing stays in core.

• 99c3e8f feat(go-sdk): WatchLogs
• 03b5219 fix(go-sdk): wait the WatchLogs reconnect backoff once and keep it cancellable
• cfb737a fix(go-sdk): reset the WatchLogs backoff only after a delivered item

Review fixes by @letv1nnn, taken from letv1nnn/sdk-go@port/3662 and rebased onto main (the ported commits above now have new SHAs):

• 9865349 fix(sandbox): move WatchLogs to an optional SandboxLogsWatchInterface
• 00cce03 refactor(sandbox): inject the WatchLogs backoff wait and tighten its tests
• 704c984 docs(sandbox): document WatchLogs ordering, retry and OutOfRange semantics
• f6e2e0d test(fake): cover the WatchLogs stub

The two follow-up commits came after the description above was written. The Testing section refers to core's tasks.

[letv1nnn]

thanks for porting this! I'll follow up on any review here.

[rhuss]

cc-review Summary

What Went Well

• openshell/v1/sandbox_watch_logs.go runWatchLogs (lines 145-147) keeps the resume cursor as a high-water mark with a comment explaining out-of-order live delivery across the log and platform sources, so a later lower cursor cannot rewind the resume point. TestWatchLogs_CursorIsHighWaterMark sends cursor 5 then 2 before a drop and asserts the redial resumes from 5.
• openshell/v1/sandbox_watch_logs.go retryWatchLogs and isRetryableWatchLogsError isolate the retry decision in one place and retry only on the raw gRPC Unavailable code. ResourceExhausted (also mapped to ErrorUnavailable), PermissionDenied and Unauthenticated stay terminal, and OUT_OF_RANGE is surfaced as a terminal EventError as the proto contract requires.
• openshell/v1/watch.go emit and sleep (lines 38-62) select on the result channel, w.done and ctx.Done(), and sleep stops its timer with defer. Stop therefore interrupts both a blocked send and a backoff wait, verified by TestWatchLogs_StopInterruptsBackoff.
• openshell/v1/sandbox_watch_logs.go runWatchLogs (lines 90-91) defers close(w.result) and w.cancel(), so every exit path releases the gRPC stream and the single producer closes the channel exactly once.
• openshell/v1/sandbox_watch_logs.go line 139 resets the backoff only after a converted item is delivered, not after the skipped status snapshot, which delivers the stated fix commit. TestWatchLogs_BackoffGrowsWhenOnlySnapshotsArrive pins the 100/200/400ms sequence.
• openshell/v1/internal/converter/watch_log.go WatchLogEventFromProto keeps all proto oneof dispatch in the converter and returns nil for non-stream payloads, with table coverage in TestWatchLogEventFromProtoSkipsNonStreamPayloads. PlatformEventFromProto copies the metadata map, and the request copies LogSources, so caller and SDK share no mutable state.
• openshell/v1/sandbox_watch_logs.go WatchLogs resolves the sandbox with Get under the caller's workspace before opening the stream and passes namedWorkspaceScope(workspace) on every redial, consistent with the other workspace-scoped clients.
• openshell/v1/types/errors.go appends ErrorOutOfRange at the end of the iota block, so existing ErrorCode values are unchanged, and adds a matching String case and IsOutOfRange helper.
• openshell/v1/types/watch_log.go and openshell/v1/sandbox.go follow the established types package plus v1 alias pattern, and openshell/v1/fake/sandbox.go WatchLogs mirrors the fake GetLogs convention (closed client returns ErrorUnavailable, otherwise ErrorUnimplemented).
• openshell/v1/sandbox_client_test.go mockSandboxServer.watchFunc/watchRequests clone every request per attempt, so tests can script each reconnect, and TestWatchLogs_OutOfRangeIsTerminal asserts IsOutOfRange, not IsInvalidArgument, and exactly one dial.

Findings

Severity	File	Description	Source
Important	openshell/v1/sandbox_watch_logs.go:145	Avoid advancing the resume cursor past undelivered events.	codex
Minor	openshell/v1/fake/sandbox.go:904	The new fake WatchLogs (Unimplemented, or Unavailable when the client is closed) has no tests...	test-quality
Minor	openshell/v1/sandbox.go:121	WatchLogs is added directly to the exported SandboxInterface, breaking every external...	goal-alignment
Minor	openshell/v1/sandbox_watch_logs.go:34	A package-level mutable function variable (testHookWatchLogsSleep) is added to production code...	architecture
Minor	openshell/v1/sandbox_watch_logs.go:120	The transparent-reconnect guarantee depends on stream.Recv returning Unavailable, but the client...	production
Minor	openshell/v1/sandbox_watch_logs.go:176	WatchLogs retries Unavailable forever with no signal to the consumer, so a gateway that stays...	gator
Minor	openshell/v1/sandbox_watch_logs.go:192	The status.Code fallback in isRetryableWatchLogsError is unreachable, because its only caller...	architecture
Minor	openshell/v1/sandbox_watch_logs_test.go:174	TestWatchLogs_RequestFields never asserts the request's Sandbox name or WorkspaceScope, and no...	test-quality
Minor	openshell/v1/sandbox_watch_logs_test.go:348	No test covers ResourceExhausted, the one code where the SDK classification (ErrorUnavailable)...	test-quality
Minor	openshell/v1/sandbox_watch_logs_test.go:414	The backoff cap (watchLogsMaxBackoff via min()) is never exercised: the backoff tests stop at...	test-quality
Minor	openshell/v1/sandbox_watch_logs_test.go:579	TestWatchLogs_ContextCancelClosesChannel throws away the drained events, so it cannot detect a...	test-quality
Minor	openshell/v1/types/errors.go:28	ErrorOutOfRange and IsOutOfRange are documented as meaning only 'a stream resume cursor is...	architecture
Notable	openshell/v1/sandbox_watch_logs.go:54	Need is UNCLEAR: the PR description only states that the change is ported from...	goal-alignment
Notable	openshell/v1/watch.go:36	The new watcher.emit/sleep/stopped helpers centralize the send-or-stop select, but the existing...	architecture

All findings are posted as inline comments. The one that matters most is the Important resume-cursor issue in sandbox_watch_logs.go (lines 145-146): a high-water cursor can skip an undelivered lower-cursor event after a drop. Everything else is nonblocking polish, documentation and test coverage.

Review Details

• Review type: initial review, full scope (no earlier review on this PR)
• Findings posted: 14
• Findings reviewed and not posted: 0
• Gate outcome: FAIL (one Important finding)
• Participating reviewers: architecture, goal-alignment, production, test-quality, gator, codex
• Coverage: incomplete. The correctness and security reviewers ran and found nothing, but the CodeRabbit reviewer failed to run, so this review does not count as complete coverage.

Coverage limitation: required reviewer coderabbit failed, so this run cannot establish a completed review baseline.

[letv1nnn]

Hey @rhuss, thanks for the thorough review! I can't push to this branch, so the fixes are on my fork: https://github.com/letv1nnn/sdk-go/tree/port/3662

It's rebased onto main, with four commits on top of the ported ones. Feel free to force-push it here or cherry-pick them. CI passes.

The cursor issue turned out to be a wrong comment on our side. The gateway already delivers events in cursor order (it merges both sources behind a watermark in grpc/sandbox.rs), so I fixed the comment and documented the guarantee. I also moved WatchLogs into an optional SandboxLogsWatchInterface, so SandboxInterface stays unchanged.

Everything else is covered except keepalive and the Watch helper cleanup, which I'll open as follow-ups if it's ok.

[rhuss]

cc-review Summary

Follow-up progress

Follow-up verification at head cfb737ab, unchanged since the previous review. Addressed sources: 0. Open material sources: 2 (the resume-cursor thread and the prior review summary). New material findings: 0. Prior nonblocking suggestions: 13, all still present. Pending questions: 0. Limitations: the coderabbit reviewer failed this round, so it was not possible to confirm completed coverage; the seven internal reviewers and codex covered the full delta. The author's fix commits sit on their fork branch and have not been pushed to this PR.

What Went Well

• runWatchLogs (openshell/v1/sandbox_watch_logs.go:90-91) defers close(w.result) and w.cancel(), so every exit path closes the channel exactly once and the gRPC stream is always released.
• watcher.emit/sleep/stopped (openshell/v1/watch.go:38-72) put the cancellable send-or-stop select in one place, with deferred timer teardown, so Stop can interrupt a blocked send as well as a backoff wait.
• Authorization stays consistent across the retry loop because WatchLogs (openshell/v1/sandbox_watch_logs.go:55-76) resolves the sandbox under the caller's workspace before opening the stream and passes that same scope on every redial.
• WatchLogEventFromProto (openshell/v1/internal/converter/watch_log.go:29-59) keeps proto oneof dispatch inside the converter, copies mutable state, and returns nil for non-resumable payloads. Warnings carry no cursor, so they can never advance a resume point.
• The test suite pins the exact invariants: the out-of-order cursor high-water mark, an OUT_OF_RANGE terminal with exactly one dial, and backoff reset only on delivered items.

Findings

No findings are posted in this review.

Review Details

These findings were produced by cc-review's reviewer agents and posted without a per-finding review.

• Findings posted: 0
• Findings reviewed and not posted: 0
• Gate outcome: FAIL
• Participating reviewers: codex

[rhuss]

Follow-up verification of the previous review: the PR head is unchanged (cfb737a) since that review, and the fixes described in the author's latest comment exist only on the fork branch, so all 14 findings from that review remain present in the code. The one Important finding (resume-cursor handling and the wrong out-of-order comment, openshell/v1/sandbox_watch_logs.go:140-146) is still open in its thread, along with 13 Minor and Notable findings. This round added no new findings: an independent tool pass (codex) raised two issues, both duplicates of existing threads. Coverage note: the coderabbit reviewer failed this round, so completed coverage could not be established; the seven internal reviewers and codex covered the full delta.

[rhuss]

I've pushed all the review fixes from @letv1nnn's fork to this branch (rebased onto main, CI is green), and all 14 review threads are resolved. Keepalive and the Watch helper cleanup are tracked as follow-ups in #35 and #36.

@grkumbach @SDAChess could you take another look? I'd approve it myself, but since I ported this PR from the core repo, GitHub counts me as the author, so my approval doesn't count here.