feat(page): print the eval-js value bare and use exception_detail for failures

internal/cmd/execution_failure_test.go

@@ -0,0 +1,70 @@
+package cmd
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/nottelabs/notte-cli/internal/api"
+)
+
+func strPtr(s string) *string { return &s }
+
+// The API sends the failure twice: `exception` is a bare string rendered in the
+// server's ErrorConfig mode (often the generic user-facing sentence), while
+// `exception_detail` carries the concrete type and the per-audience messages.
+// The CLI must report the structured one when it is there.
+func TestExecutionFailureError_PrefersStructuredDetail(t *testing.T) {
+	detail := &api.SerializedError{
+		ErrorType:   "ActionExecutionError",
+		DevMessage:  "Failed to execute action: evaluate_js on https://x. Reason: ReferenceError: foo is not defined",
+		UserMessage: "Sorry, this action cannot be executed at the moment.",
+	}
+
+	err := executionFailureError(detail, strPtr("Sorry, this action cannot be executed at the moment."), "boom")
+
+	got := err.Error()
+	if !strings.Contains(got, "ActionExecutionError") {
+		t.Fatalf("expected the concrete error type, got %q", got)
+	}
+	if !strings.Contains(got, "ReferenceError: foo is not defined") {
+		t.Fatalf("expected the actual reason, got %q", got)
+	}
+	if strings.Contains(got, "Sorry, this action cannot be executed") {
+		t.Fatalf("expected the generic user message to be dropped, got %q", got)
+	}
+}
+
+func TestExecutionFailureError_FallsBackToUserMessage(t *testing.T) {
+	detail := &api.SerializedError{ErrorType: "BrowserError", UserMessage: "Something unexpected happened."}
+
+	err := executionFailureError(detail, nil, "")
+
+	if got := err.Error(); got != "BrowserError: Something unexpected happened." {
+		t.Fatalf("unexpected error: %q", got)
+	}
+}
+
+// API builds that predate exception_detail send only the legacy string.
+func TestExecutionFailureError_LegacyExceptionString(t *testing.T) {
+	err := executionFailureError(nil, strPtr("boom"), "click failed")
+
+	if got := err.Error(); got != "boom: click failed" {
+		t.Fatalf("unexpected error: %q", got)
+	}
+}
+
+func TestExecutionFailureError_MessageOnly(t *testing.T) {
+	err := executionFailureError(nil, nil, "click failed")
+
+	if got := err.Error(); got != "action failed: click failed" {
+		t.Fatalf("unexpected error: %q", got)
+	}
+}
+
+func TestExecutionFailureError_NothingToGoOn(t *testing.T) {
+	err := executionFailureError(nil, nil, "")
+
+	if got := err.Error(); got != "action failed" {
+		t.Fatalf("unexpected error: %q", got)
+	}
+}

internal/cmd/output_helpers.go

@@ -227,3 +227,41 @@ func printSessionStatus(resp *api.SessionResponse) error {
 
 	return nil
 }
+
+// executionFailureError builds the error for a failed page action.
+//
+// The API serializes failures twice: `exception` is a bare string rendered in
+// whatever ErrorConfig mode the server ran in (often the generic user-facing
+// sentence), while `exception_detail` carries the concrete error type, the
+// per-audience messages and the retry/notify flags. Prefer the structured
+// field so the CLI reports what actually went wrong, and fall back to the
+// legacy string for API builds that predate it.
+func executionFailureError(detail *api.SerializedError, exception *string, message string) error {
+	if detail != nil {
+		reason := detail.DevMessage
+		if reason == "" {
+			reason = detail.UserMessage
+		}
+		if reason == "" {
+			reason = message
+		}
+		if reason == "" {
+			return fmt.Errorf("%s", detail.ErrorType)
+		}
+		if detail.ErrorType != "" {
+			return fmt.Errorf("%s: %s", detail.ErrorType, reason)
+		}
+		return fmt.Errorf("%s", reason)
+	}
+
+	if exception != nil && *exception != "" {
+		if message != "" && *exception != message {
+			return fmt.Errorf("%s: %s", *exception, message)
+		}
+		return fmt.Errorf("%s", *exception)
+	}
+	if message != "" {
+		return fmt.Errorf("action failed: %s", message)
+	}
+	return fmt.Errorf("action failed")
+}

internal/cmd/page.go

@@ -55,19 +55,7 @@ func printExecuteResponse(resp *api.ApiExecutionResponse) error {
 	}
 
 	if !resp.Success {
-		// Build error message with available context
-		if resp.Exception != nil {
-			// Include exception and message if both present and different
-			if resp.Message != "" && *resp.Exception != resp.Message {
-				return fmt.Errorf("%s: %s", *resp.Exception, resp.Message)
-			}
-			return fmt.Errorf("%s", *resp.Exception)
-		}
-		// No exception - use message or generic fallback
-		if resp.Message != "" {
-			return fmt.Errorf("action failed: %s", resp.Message)
-		}
-		return fmt.Errorf("action failed")
+		return executionFailureError(resp.ExceptionDetail, resp.Exception, resp.Message)
 	}
 
 	// Print message
@@ -718,10 +706,15 @@ var pageEvalJsCmd = &cobra.Command{
 
 The JavaScript code is executed in the context of the page's main frame.
 
+The evaluated value is printed alone on stdout (objects and arrays as JSON, a JS
+null as "null"), so it pipes straight into jq or a shell variable; the status
+line goes to stderr. Use -o json for the full execution result.
+
 Examples:
   notte page eval-js "document.title"
   notte page eval-js "window.location.href"
-  notte page eval-js "document.querySelectorAll('a').length"`,
+  notte page eval-js "document.querySelectorAll('a').length"
+  notte page eval-js "JSON.stringify([...document.links].map(a => a.href))" | jq length`,
 	Args: cobra.ExactArgs(1),
 	RunE: runPageEvalJs,
 }
@@ -765,21 +758,16 @@ func runPageEvalJs(cmd *cobra.Command, args []string) error {
 	}
 
 	if !result.Success {
-		if result.Exception != nil {
-			if result.Message != "" && *result.Exception != result.Message {
-				return fmt.Errorf("%s: %s", *result.Exception, result.Message)
-			}
-			return fmt.Errorf("%s", *result.Exception)
-		}
-		if result.Message != "" {
-			return fmt.Errorf("eval-js failed: %s", result.Message)
-		}
-		return fmt.Errorf("eval-js failed")
+		return executionFailureError(result.ExceptionDetail, result.Exception, result.Message)
 	}
 
-	fmt.Println(result.Message)
-	if result.Data != nil && result.Data.Markdown != "" {
-		fmt.Printf("Result:  %s\n", result.Data.Markdown)
+	// The evaluated value is the point of the command, so print it bare: it pipes
+	// into jq or a shell variable without post-processing, and a JS value that is
+	// legitimately empty still prints an (empty) line instead of vanishing behind
+	// a success banner. The status line goes to stderr so stdout stays the value.
+	_, _ = fmt.Fprintln(os.Stderr, result.Message)
+	if result.Data != nil {
+		fmt.Println(result.Data.Markdown)
 	}
 	return nil
 }

tests/integration/page_evaljs_test.go

@@ -0,0 +1,158 @@
+//go:build integration
+
+package integration
+
+import (
+	"bytes"
+	"context"
+	"encoding/json"
+	"os"
+	"os/exec"
+	"strings"
+	"testing"
+	"time"
+)
+
+// runCLIText runs the CLI in its default (human) output mode. The shared
+// helpers prepend `-o json`, which is exactly what these tests must not do:
+// the point is what a shell sees on stdout without --output json.
+func runCLIText(t *testing.T, args ...string) CLIResult {
+	t.Helper()
+
+	ctx, cancel := context.WithTimeout(context.Background(), 120*time.Second)
+	defer cancel()
+
+	cmd := exec.CommandContext(ctx, "go", append([]string{"run", "./cmd/notte", "--yes"}, args...)...)
+	cmd.Dir = getProjectRoot()
+	cmd.Env = append(os.Environ(), "NOTTE_API_KEY="+os.Getenv("NOTTE_API_KEY"))
+	if apiURL := os.Getenv("NOTTE_API_URL"); apiURL != "" {
+		cmd.Env = append(cmd.Env, "NOTTE_API_URL="+apiURL)
+	}
+
+	var stdout, stderr bytes.Buffer
+	cmd.Stdout = &stdout
+	cmd.Stderr = &stderr
+
+	exitCode := 0
+	if err := cmd.Run(); err != nil {
+		if exitErr, ok := err.(*exec.ExitError); ok {
+			exitCode = exitErr.ExitCode()
+		} else {
+			exitCode = -1
+		}
+	}
+	return CLIResult{Stdout: stdout.String(), Stderr: stderr.String(), ExitCode: exitCode}
+}
+
+// evalJs runs `notte page eval-js` in text mode, the way a shell would.
+func evalJs(t *testing.T, sessionID string, code string) CLIResult {
+	t.Helper()
+	return runCLIText(t, "page", "eval-js", code, "--session-id", sessionID)
+}
+
+// The evaluated value is the point of the command: it must land on stdout with
+// nothing else, so `title=$(notte page eval-js "document.title")` captures the
+// title alone and a pipe into jq needs no post-processing. The status line goes
+// to stderr.
+func TestPageEvalJsPrintsOnlyTheValue(t *testing.T) {
+	sessionID := startTestSession(t)
+	defer cleanupSession(t, sessionID)
+
+	requireSuccess(t, runCLIWithTimeout(t, 120*time.Second, "page", "goto", "https://example.com", "--session-id", sessionID))
+
+	result := evalJs(t, sessionID, "document.title")
+	requireSuccess(t, result)
+
+	// what `title=$(...)` would capture
+	title := strings.TrimSpace(result.Stdout)
+	if title != "Example Domain" {
+		t.Fatalf("expected the title alone on stdout, got %q (stderr: %q)", result.Stdout, result.Stderr)
+	}
+	if strings.Contains(result.Stdout, "Result:") || strings.Contains(result.Stdout, "Successfully executed") {
+		t.Errorf("stdout must carry the value only, got %q", result.Stdout)
+	}
+}
+
+// `notte page eval-js "JSON.stringify(...)" | jq length` — stdout has to be a
+// standalone JSON document for the pipe to work.
+func TestPageEvalJsStdoutIsPipeableJSON(t *testing.T) {
+	sessionID := startTestSession(t)
+	defer cleanupSession(t, sessionID)
+
+	requireSuccess(t, runCLIWithTimeout(t, 120*time.Second, "page", "goto", "https://example.com", "--session-id", sessionID))
+
+	result := evalJs(t, sessionID, "JSON.stringify([...document.links].map(a => a.href))")
+	requireSuccess(t, result)
+
+	var links []string
+	if err := json.Unmarshal([]byte(strings.TrimSpace(result.Stdout)), &links); err != nil {
+		t.Fatalf("stdout is not parseable JSON (%v): %q", err, result.Stdout)
+	}
+	if len(links) == 0 {
+		t.Errorf("expected at least one link on example.com, got %v", links)
+	}
+}
+
+// A JS null is a successful evaluation and must arrive as the string "null",
+// not as an empty stdout that a caller would read as "no result".
+func TestPageEvalJsNullPrintsNull(t *testing.T) {
+	sessionID := startTestSession(t)
+	defer cleanupSession(t, sessionID)
+
+	requireSuccess(t, runCLIWithTimeout(t, 120*time.Second, "page", "goto", "https://example.com", "--session-id", sessionID))
+
+	result := evalJs(t, sessionID, "null")
+	requireSuccess(t, result)
+
+	if got := strings.TrimSpace(result.Stdout); got != "null" {
+		t.Fatalf("expected \"null\" on stdout, got %q", got)
+	}
+}
+
+// -o json still emits the whole execution result, so machine consumers that
+// want the envelope keep working.
+func TestPageEvalJsJSONOutputKeepsTheEnvelope(t *testing.T) {
+	sessionID := startTestSession(t)
+	defer cleanupSession(t, sessionID)
+
+	requireSuccess(t, runCLIWithTimeout(t, 120*time.Second, "page", "goto", "https://example.com", "--session-id", sessionID))
+
+	result := runCLIWithTimeout(t, 120*time.Second, "page", "eval-js", "document.title", "--session-id", sessionID)
+	requireSuccess(t, result)
+
+	var envelope struct {
+		Success bool `json:"success"`
+		Data    *struct {
+			Markdown string `json:"markdown"`
+		} `json:"data"`
+	}
+	if err := json.Unmarshal([]byte(result.Stdout), &envelope); err != nil {
+		t.Fatalf("expected a JSON envelope on stdout (%v): %q", err, result.Stdout)
+	}
+	if !envelope.Success {
+		t.Errorf("expected success=true, got %+v", envelope)
+	}
+	if envelope.Data == nil || envelope.Data.Markdown != "Example Domain" {
+		t.Errorf("expected the title in data.markdown, got %+v", envelope.Data)
+	}
+}
+
+// A failing script must exit non-zero and name the actual JavaScript error,
+// rather than the generic user-facing sentence the API serializes by default.
+func TestPageEvalJsFailureReportsTheJavaScriptError(t *testing.T) {
+	sessionID := startTestSession(t)
+	defer cleanupSession(t, sessionID)
+
+	requireSuccess(t, runCLIWithTimeout(t, 120*time.Second, "page", "goto", "https://example.com", "--session-id", sessionID))
+
+	result := evalJs(t, sessionID, "notAFunction()")
+	requireFailure(t, result)
+
+	combined := result.Stdout + result.Stderr
+	if !containsString(combined, "notAFunction") {
+		t.Errorf("expected the JavaScript error in the output, got stdout=%q stderr=%q", result.Stdout, result.Stderr)
+	}
+	if containsString(combined, "Sorry, this action cannot be executed") {
+		t.Errorf("generic user message leaked instead of the real reason: %q", combined)
+	}
+}