fix(lifecycle): apply explicit resume and connect timeouts

[zyl1121]

Motivation

Resume and Connect both accept an optional idle timeout, but explicit values do not currently become the sandbox's effective lifecycle timeout in all supported paths.

There are two gaps:

• CubeAPI serializes a Resume timeout into the CubeMaster update request, but CubeMaster's request type does not include that field, so JSON decoding silently drops it.
• Connect skips Resume when the sandbox is already running, but it also does not call the existing set-timeout path.

As a result, callers can successfully send timeout while the sandbox continues using its previous lifecycle policy. This also diverges from E2B-compatible Connect behavior, where an explicit timeout resets the sandbox timeout.

What Changed

• Preserve the optional timeout across the CubeAPI-to-CubeMaster Resume request boundary.
• Refresh lifecycle timeout metadata only after Resume succeeds.
• Apply paused Connect timeout through the Resume path without issuing a duplicate timeout update.
• Use the existing set-timeout path when Connect targets an already-running sandbox.
• Preserve the current effective timeout when callers omit the field.
• Keep the existing -1, 0, and positive timeout semantics, and reject values below -1 at both API layers.
• Add the E2B-compatible optional timeout argument to Python Sandbox.connect.
• Add SDK compatibility E2E coverage for running Connect and paused Connect/Resume.
• Update lifecycle and Python SDK documentation.

Validation

• CubeMaster lifecycle and sandbox service package tests passed.
• CubeAPI test suite passed.
• Python SDK tests passed.
• SDK compatibility E2E passed for both CubeSandbox and E2B backends.
• Runtime tests confirmed that changing an existing timeout takes effect for running Connect, paused Connect/Resume, and Resume.

Scope

This PR only makes explicit Resume and Connect timeout values effective and adds the corresponding Python SDK compatibility surface.

[cubesandboxbot]

Review — PR #1352: fix(lifecycle): apply explicit resume and connect timeouts

AI-generated review. This review is automated and does not constitute human approval.

Verdict

The PR correctly identifies and fixes a real defect: explicit idle timeouts on Resume/Connect were silently dropped at the CubeAPI→CubeMaster boundary (UpdateRequest had no timeout field, so JSON decoding discarded it), and Connect never applied a timeout to an already-running sandbox. The fix is well-structured: it threads timeout through the wire type, applies it via the existing refreshTimeoutMeta only after a successful resume, reuses the existing set-timeout path for running sandboxes, validates >= -1 at both layers, and adds solid unit tests plus SDK-compat E2E coverage. No blocking issues found. Two code-level concerns are posted inline; a few non-blocking observations follow.

Findings

1. Connect still silently drops an explicit timeout for statuses other than Paused/Running (medium, CubeAPI/src/services/sandboxes.rs:384)
The new else if d.status == SandboxStatus::Running branch means the timeout is only applied when the fetched status is exactly Running or Paused. Any other status — including SandboxStatus::Unknown, which CubeMaster reports for a freshly created sandbox (CONTAINER_CREATED, status 0 → Unknown per cubemaster/mod.rs:1351), plus Pausing/Stopping/Error — falls through with no timeout applied and no error. Sandbox.connect(id, timeout=300) issued immediately after create therefore races the status transition and can return success while the sandbox keeps its old lifecycle policy — the same silent-drop behavior this PR is meant to eliminate. Recommend applying the timeout for any non-Paused status (resume carries the timeout), or explicitly rejecting when it cannot be applied.

2. A timeout-refresh failure now fails the whole Connect on a running sandbox (low, CubeAPI/src/services/sandboxes.rs:389)
self.set_timeout(...).await? propagates errors (e.g. transient NotFound while CubeMaster's localcache is cold after a restart, or a backend error) and turns "connect to a running sandbox" — previously always successful — into a hard failure even though the connection info could still be returned. Consider best-effort handling (log + continue) to match the claimed E2B semantics, or document the new failure mode.

Non-blocking observations

• Semantic change for timeout=0 on resume/connect. The field doc on SandboxUpdateRequest changed from "0 = keep original" to "0 = immediate". Before this PR the field was dropped at the CubeMaster boundary, so a client sending 0 kept its original timeout; now it gets an immediate expiry (refreshTimeoutMeta(0) → EndAt = now). This matches the API-wide 0 = immediate convention (create, set_timeout), so it is a correction, but it is a real behavior change and worth a changelog/API note — a legacy caller relying on the old documented "keep original" meaning will now expire their sandbox.
• pause + timeout is silently accepted and ignored in CubeMaster's Update. The new doc comment says timeout is "omitted for pause", but a pause request carrying a timeout >= -1 passes validation and is silently ignored. Consider rejecting the combination explicitly for clarity.
• refreshTimeoutMeta return value is discarded in Update. If the lifecycle store errors (or the meta entry is absent), resume reports success but the timeout is silently not applied. This is consistent with the existing SetTimeout swallow-errors design ("a Redis hiccup cannot fail a sandbox create/destroy"), but it is the same class of silent-drop the PR targets; a log line or explicit note would help operators.
• E2B divergence on shorter timeouts for running sandboxes. Per the E2E test comment, E2B keeps the existing timeout when a running sandbox receives a shorter value, while Cube applies the shorter value via set_timeout. The tests work around it by using a longer value, but this divergence should be documented in docs/guide/lifecycle.md so SDK users aren't surprised.
• Test-only: sandbox_update_test.go's TestMain starts miniredis but only wires it into config.GetConfig().RedisConf when the config is already initialized — which it is not at TestMain time — so the miniredis setup is effectively dead code. The tests pass because they use MockUpdateAction plus a recording TimeoutProvider. Harmless, but misleading; consider removing the Redis scaffolding or actually wiring it.

Scope / coverage notes

• The CubeAPI and CubeMaster unit tests are focused and exercise the important branches: invalid timeout rejection at both layers, resume-applies-timeout, omitted/pause-does-not-change-timeout, and running-vs-paused connect behavior (no duplicate timeout update on the paused path).
• The Python SDK connect(timeout=...) change is backward-compatible (None omits the field) and matches E2B's signature; the E2E assertions (endAt/timeout visibility) are coherent with how both backends expose the effective deadline.
• The deprecated resume endpoint on an already-running sandbox still returns its pre-existing error and does not apply the timeout (refresh fires only on success). This is consistent with the documented "resume is for paused sandboxes" contract; worth keeping in mind that only connect resets a running sandbox's timeout.

[cubesandboxbot]

The else if only handles Paused and Running. Any other status — most notably SandboxStatus::Unknown (CubeMaster status 0 = CONTAINER_CREATED, which is what a freshly created sandbox reports until it reaches running) as well as Pausing/Stopping/Error — falls through with no timeout applied, silently dropping the explicit value. That is the exact class of silent-drop this PR is meant to fix. Concretely, Sandbox.connect(id, timeout=300) issued immediately after create races the status transition and can return success while the sandbox keeps its old lifecycle policy. Consider applying the timeout for any status that is not Paused (the resume-with-timeout path), or at minimum rejecting/warning when the requested timeout cannot be applied.

[cubesandboxbot]

self.set_timeout(...).await? propagates any failure and fails the entire Connect request, even though the sandbox is running and the connection info could still be returned. The timeout is an optional side effect here, so a transient error (e.g. NotFound while CubeMaster's localcache is cold after a restart, or a backend error) turns a previously-always-successful "connect to running sandbox" into a hard failure. Consider treating the timeout refresh as best-effort (log and continue) to match the E2B semantics the PR claims, or at least document that Connect now fails when the timeout cannot be applied.