pkg/gcloud/gcloud.go (1)

215-225: ⚠️ Potential issue | 🟠 Major

Handle SUSPENDED as a valid VM state.

compute.instances.get can return SUSPENDED. Right now that falls through to unexpected status, so status fails for a legitimate instance state instead of reporting a provider status. Mapping it to client.StatusStopped would keep the command usable.

Suggested fix

  switch status {
  case "RUNNING":
  	return client.StatusRunning, nil
  case "STOPPING", "SUSPENDING", "REPAIRING", "PROVISIONING", "STAGING":
  	return client.StatusBusy, nil
- case "TERMINATED":
+ case "TERMINATED", "SUSPENDED":
  	return client.StatusStopped, nil
  }

What are the documented Google Compute Engine VM instance status values, and does that list include SUSPENDED?

🤖 Prompt for AI Agents

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

In `@pkg/gcloud/gcloud.go` around lines 215 - 225, The switch on the normalized
GCE instance status currently treats "SUSPENDED" as unexpected; update the
mapping in the status-mapping block (the code that sets status :=
strings.TrimSpace(strings.ToUpper(*instance.Status)) and switches on status) to
include "SUSPENDED" as a recognized VM state and return client.StatusStopped for
it (i.e., add "SUSPENDED" to the case that returns client.StatusStopped
alongside "TERMINATED"). Keep the existing default fallback to return
client.StatusNotFound and the error for any truly unexpected values.

cmd/stop.go (1)

83-90: ⚠️ Potential issue | 🟠 Major

Bug: Error is swallowed when reading error response body succeeds.

Line 86 wraps err (which is nil after a successful io.ReadAll) instead of creating a new error. When the status code indicates failure (≥400) but the body is read successfully, this returns nil, silently ignoring the HTTP error.

🐛 Proposed fix

 	if resp.StatusCode >= 400 {
 		out, err := io.ReadAll(resp.Body)
-		if err == nil {
-			return errors.Wrapf(err, "Error stopping vm: %s", string(out))
+		if err != nil {
+			return errors.Wrap(err, "reading error response body")
 		}
-
-		return err
+		return fmt.Errorf("error stopping vm: %s", string(out))
 	}

🤖 Prompt for AI Agents

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

In `@cmd/stop.go` around lines 83 - 90, The code handling HTTP error responses
misuses the local err variable: when io.ReadAll(resp.Body) succeeds err is nil
but the code calls errors.Wrapf(err, ...) and ends up returning nil, swallowing
the HTTP error; update the error path in the block that checks resp.StatusCode
>= 400 (using resp and out variables shown in the diff) so that when ReadAll
succeeds you create and return a new error (e.g., errors.Errorf or errors.New
with a descriptive message including the response body string/out and status)
instead of wrapping the nil err, and keep returning the original err when
ReadAll fails.

cmd/command.go (1)

40-105: ⚠️ Potential issue | 🟠 Major

Propagate tunnel startup cancellation instead of falling through.

The context plumbing stops short in the IAP branch: Line 86 still launches gcloud with exec.Command, and line 105 ignores whether waitForPort exited due to timeout or caller cancellation. If the context times out or is canceled, waitForPort returns silently and execution continues to SSH connection setup against a tunnel that may never be ready.

Proposed fix

-		gcloudIAPcommand := exec.Command("gcloud", []string{
+		gcloudIAPcommand := exec.CommandContext(ctx, "gcloud", []string{
 			"compute",
 			"start-iap-tunnel",
 			*instance.Name,
 			"22",
 			"--local-host-port=localhost:" + port,
 			"--zone=" + *instance.Zone,
 		}...)
 
 		// open tunnel in background
 		if err = gcloudIAPcommand.Start(); err != nil {
 			return fmt.Errorf("start tunnel: %w", err)
 		}
 		defer func() {
 			err = gcloudIAPcommand.Process.Kill()
 		}()
 
 		timeoutCtx, cancelFn := context.WithTimeout(ctx, 30*time.Second)
 		defer cancelFn()
-		waitForPort(timeoutCtx, port)
+		if err := waitForPort(timeoutCtx, port); err != nil {
+			return fmt.Errorf("wait for IAP tunnel on port %s: %w", port, err)
+		}
 	}
 
 	sshClient, err := ssh.NewSSHClient("devpod", target+":"+port, privateKey)
 	if err != nil {
 		return errors.Wrap(err, "create ssh client")
 	}
 	defer func() { _ = sshClient.Close() }()
 
 	// run command
 	return ssh.Run(ctx, ssh.RunOptions{
 		Client:  sshClient,
 		Command: command,
 		Stdin:   os.Stdin,
 		Stdout:  os.Stdout,
 		Stderr:  os.Stderr,
 		EnvVars: nil,
 	})
 }
 
-func waitForPort(ctx context.Context, port string) {
+func waitForPort(ctx context.Context, port string) error {
 	for {
 		select {
 		case <-ctx.Done():
-			return
+			return ctx.Err()
 		default:
 			l, err := net.Listen("tcp", "localhost:"+port)
 			if err != nil {
 				// port is taken
-				return
+				return nil
 			}
 			_ = l.Close()
 			time.Sleep(1 * time.Second)
 		}
 	}
 }

🤖 Prompt for AI Agents

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

In `@cmd/command.go` around lines 40 - 105, In Run, the IAP tunnel branch must not
ignore cancellation/timeouts from waitForPort: call waitForPort with timeoutCtx
and check its returned error (or context.Done) and if it indicates
timeout/cancellation return that error (or wrap it) before proceeding to SSH;
ensure gcloudIAPcommand is terminated on that early return (use the existing
deferred kill or call Process.Kill on error) and propagate the context error
instead of continuing to SSH, referencing Run, gcloudIAPcommand, timeoutCtx,
waitForPort and the IAP branch logic around findAvailablePort.