Skip to content

fix(cubeops): preserve unknown sandbox states - #1350

Open
luzhixing12345 wants to merge 1 commit into
TencentCloud:masterfrom
luzhixing12345:fix/cubeops-unknown-sandbox-state
Open

fix(cubeops): preserve unknown sandbox states#1350
luzhixing12345 wants to merge 1 commit into
TencentCloud:masterfrom
luzhixing12345:fix/cubeops-unknown-sandbox-state

Conversation

@luzhixing12345

Copy link
Copy Markdown
Collaborator

sandbox may fail to pause or resume, and its state will goto unknown, current webui has display error

before

Clipboard_Screenshot_1786693632

after

Clipboard_Screenshot_1786693747

Comment thread CubeAPI/src/models/mod.rs Outdated
Comment thread CubeOps/internal/translator/translator.go
@cubesandboxbot

cubesandboxbot Bot commented Aug 14, 2026

Copy link
Copy Markdown

Review: fix(cubeops): preserve unknown sandbox states (#1350)

Summary

This PR fixes a display bug in CubeOps: SandboxStateFromInt/SandboxStateFromRaw returned "running" for any status that wasn't explicitly 4/5, so CubeMaster statuses 0 (created), 2 (exited/stopped), and 3 (unknown) were all reported to the WebUI as running. A sandbox whose pause/resume had failed (status 3) therefore looked healthy. The change makes the fallback "unknown", adds an explicit case 1 → running, and adds unknown to the SandboxState enum in openapi.yml plus the generated web/src/api/generated/schema.ts.

The fix is correct and internally consistent:

  • SandboxStateFromInt now maps 1→running, 4→pausing, 5→paused, everything else → unknown.
  • SandboxStateFromRaw's string branch keeps "1"→running, drops the incorrect "2"→running, and falls back to unknown — consistent with the integer path for every value.
  • The new tests (TestSandboxStateFromInt, TestSandboxStateFromRaw, TestTransformSandboxListPreservesUnknownState, and the handler-level TestSDK_GetSandbox_UnknownStateIsNotReportedAsRunning) cover both mapping functions and the end-to-end list/detail translation.
  • openapi.yml and the regenerated schema.ts are in lock-step, and the WebUI already renders non-running/paused states with a muted badge tone, so "unknown" displays gracefully everywhere (Sandboxes.tsx:216, SandboxDetail.tsx:115, NodeDetail.tsx:413, Overview.tsx:140). No existing tests assert the old default, so nothing breaks.

Findings

1. [Low] Test coverage gap — string-form statuses (translator_test.go)

CMSandboxListItem.Status is json.RawMessage and "may be string or int". The previous code had an explicit case "2": return "running" for the string form; this PR removes that case, so string "2" now reaches the default → unknown. The new tests cover the int forms (1, 2, 3, …) but not the string forms "1" and "2". Given the whole point of the fix is status correctness across the string/int ambiguity, adding {json.RawMessage("1"), "running"} and {json.RawMessage("2"), "unknown"} would lock in the new behavior and guard against silently reintroducing the old "2"→running mapping.

2. [Low] Behavior change for status 0/2 — worth confirming (translator.go:155)

The default-case change also affects statuses the PR doesn't target: a freshly created sandbox (status 0) will now briefly display as "unknown" (grey/muted badge) during the creation window, and an exited/stopped sandbox (status 2) shows "unknown" too. These are more honest than the old "running", and the test deliberately asserts {0, "unknown"}, so this is likely intended — but it's a user-visible side effect beyond the "status 3" fix, so flagging it for confirmation. If created should read as active/neutral during the transient creation phase, an explicit case 0 would make the intent clearer than relying on the default.

3. [Suggestion, follow-up] WebUI actions don't account for unknown state

Now that unknown is a legal state, note that the pause/resume controls still branch only on state === 'paused': Sandboxes.tsx:239 and SandboxDetail.tsx:222 render a pause button for every non-paused state, including unknown. Since an unknown state typically means a pause/resume failed, offering "pause" again (and not "resume") is likely to fail or be the wrong action. Consider disabling both actions (or showing the sandbox as read-only) when state is unknown. This is a natural completion of the PR's goal rather than a regression introduced by it.


AI-generated review — based on the PR diff and the base-branch workspace; not a human approval.

Return unknown instead of reporting unsupported CubeMaster states as running.
Synchronize the SandboxState OpenAPI schema and generated frontend types.

Signed-off-by: kamilu <kamilu@tencent.com>
@luzhixing12345
luzhixing12345 force-pushed the fix/cubeops-unknown-sandbox-state branch from e69d59e to 6a4a44f Compare August 14, 2026 08:07
Comment thread CubeOps/internal/translator/translator_test.go
Comment thread CubeOps/internal/translator/translator.go
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.

2 participants