pkg/client/clientimplementation/workspace_client.go (1)

559-588: ⚠️ Potential issue | 🟠 Major

Validate machine provider status before returning or using it.

The machineClient.Status() path returns the raw provider output without normalizing it through ParseStatus(), while getContainerStatus() correctly validates its output. This creates an inconsistency: StartWait() has a default case (line 896) that treats any unrecognized status value as success, so a typo or backend-specific string in the machine provider's status could incorrectly mark the workspace as ready.

Parse the machine status before the comparison and return:

		status, err := machineClient.Status(ctx, options)
		if err != nil {
			return client.StatusNotFound, err
		}
+		status, err = client.ParseStatus(status)
+		if err != nil {
+			return client.StatusNotFound, err
+		}
 
 		// try to check container status and if that fails check workspace folder
 		if status == client.StatusRunning && options.ContainerStatus {
 			return s.getContainerStatus(ctx)
 		}
 
 		return status, nil

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/client/clientimplementation/workspace_client.go` around lines 559 - 588,
The machine provider path returns raw status from machineClient.Status() without
normalizing it via ParseStatus(), risking unrecognized strings being treated as
success; after calling machineClient.Status(ctx, options) pass the returned
status through ParseStatus() (e.g. ParseStatus(status)) and use the parsed value
for comparisons and returns: if parsed == client.StatusRunning and
options.ContainerStatus then call s.getContainerStatus(ctx), otherwise return
the parsed status (and propagate any ParseStatus error if it returns one)
instead of the raw provider string so StartWait's default handling can't
misinterpret provider-specific/typoed values.

pkg/client/client.go (1)

184-200: ⚠️ Potential issue | 🟠 Major

Return the unrecognized status value rather than StatusNotFound on parse failure.

The current implementation converts any unrecognized status to StatusNotFound with an error. This masks new backend states, making them indistinguishable from genuinely missing resources. When callers receive a parse error (in workspace_client.go and machine_client.go), they return StatusNotFound anyway, silencing the actual status value. Return the raw trimmed input in the default case to preserve information and allow proper diagnosis of unexpected statuses.

Suggested fix

 func ParseStatus(in string) (string, error) {
-	in = strings.ToUpper(strings.TrimSpace(in))
-	switch in {
+	raw := strings.TrimSpace(in)
+	switch strings.ToUpper(raw) {
 	case "RUNNING":
 		return StatusRunning, nil
 	case "BUSY":
 		return StatusBusy, nil
 	case "STOPPED":
 		return StatusStopped, nil
 	case "NOTFOUND":
 		return StatusNotFound, nil
 	default:
-		return StatusNotFound, fmt.Errorf(
+		return raw, fmt.Errorf(
 			"error parsing status: '%s' unrecognized status, needs to be one of: %s",
-			in,
+			raw,
 			[]string{StatusRunning, StatusBusy, StatusStopped, StatusNotFound},
 		)
 	}
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/client/client.go` around lines 184 - 200, The ParseStatus function
currently returns StatusNotFound on unrecognized inputs which hides unknown
backend states; update the default case in ParseStatus to return the raw trimmed
input value (the local variable "in") along with the error instead of
StatusNotFound so callers (e.g., workspace_client.go and machine_client.go) can
see the actual unrecognized status; keep the existing error message but replace
the returned status constant with the "in" variable.

pkg/client/clientimplementation/workspace_client.go (2)

603-609: ⚠️ Potential issue | 🟠 Major

Don't treat every os.Stat failure as StatusNotFound.

Line 604 currently maps permission and I/O errors to NotFound. In StartWait, that can send callers down the create path even though the workspace folder still exists, which turns a transient filesystem problem into the wrong lifecycle action.

Suggested fix

-	_, err = os.Stat(workspaceFolder)
-	if err == nil {
-		return client.StatusRunning, nil
-	}
-
-	return client.StatusNotFound, nil
+	if _, err := os.Stat(workspaceFolder); err == nil {
+		return client.StatusRunning, nil
+	} else if os.IsNotExist(err) {
+		return client.StatusNotFound, nil
+	} else {
+		return "", fmt.Errorf("stat workspace folder: %w", err)
+	}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/client/clientimplementation/workspace_client.go` around lines 603 - 609,
The current os.Stat(workspaceFolder) treats all errors as StatusNotFound; change
the logic in the function that checks workspaceFolder (used by StartWait) to
only return client.StatusNotFound when os.IsNotExist(err) is true, return
client.StatusRunning when err == nil, and for any other error propagate the
underlying error (do not map permission or I/O errors to NotFound) so callers
like StartWait can handle transient filesystem errors correctly.

---

710-714: ⚠️ Potential issue | 🟡 Minor

Fix the mislabeled container-status debug fields.

Line 711 logs the merged buffer as stdout, and Line 712 logs the stdout buffer as stderr. When this path misbehaves, the debug record points at the wrong stream.

Suggested fix

 	s.log.WithFields(logrus.Fields{
-		"stdout": buf.String(),
-		"stderr": stdout.String(),
-		"parsed": parsed,
+		"stdout":         stdout.String(),
+		"combinedOutput": buf.String(),
+		"parsed":         parsed,
 	}).Debug("container status command output")

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/client/clientimplementation/workspace_client.go` around lines 710 - 714,
The debug log in s.log.WithFields is labeling the wrong buffers: it currently
sets "stdout" to buf.String() and "stderr" to stdout.String(); swap these so
"stdout" maps to stdout.String() and "stderr" maps to buf.String() (retain
"parsed" and the Debug message). Update the call in the containing
function/method where s.log.WithFields is used (referencing variables buf,
stdout, parsed and the s.log.WithFields invocation) to correct the labels.