From 6cb44f9f50813b07cb2fd0c04a3cc65131c681a8 Mon Sep 17 00:00:00 2001 From: jdalton Date: Wed, 22 Apr 2026 11:34:51 -0400 Subject: [PATCH 1/2] fix(cli): align test/ error messages with 4-ingredient strategy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rewrites the Socket-JSON contract validator and auth-flow mocks under packages/cli/src/test/ and packages/cli/test/ to follow the What / Where / Saw vs. wanted / Fix strategy from CLAUDE.md. Sources: - src/test/json-output-validation.mts (6 throws): each violation now spells out the full Socket-JSON contract, the received value, and a concrete fix ("add ok:true", "return empty object", etc.). Long output payloads are truncated to 200 chars in the message so errors stay readable. - src/test/mocks/socket-auth.mts (2 throws): "Authentication failed" and "OAuth timeout" now call out that they come from a test fixture and point at the configuration flag to change. - test/json-output-validation.mts (2 non-throwing returns): message values now include the exit code / parse error and a stdout preview so failing tests diagnose themselves. - test/smoke.sh (6 labels): updated to mirror the TS validator so the bash and JS harnesses produce the same wording. Tests: full suite (343 files / 5225 tests) still passes. No assertions touched — the unrelated "Authentication failed" hits in other tests are test fixtures constructing their own Errors, not references to the mock. Follows strategy from #1254. Continues #1255-#1258. --- .../cli/src/test/json-output-validation.mts | 23 ++++++++++++------- packages/cli/src/test/mocks/socket-auth.mts | 8 +++++-- packages/cli/test/json-output-validation.mts | 14 ++++++++--- packages/cli/test/smoke.sh | 12 +++++----- 4 files changed, 38 insertions(+), 19 deletions(-) diff --git a/packages/cli/src/test/json-output-validation.mts b/packages/cli/src/test/json-output-validation.mts index 4e3e33fa2..156f232b6 100644 --- a/packages/cli/src/test/json-output-validation.mts +++ b/packages/cli/src/test/json-output-validation.mts @@ -37,17 +37,24 @@ export function validateSocketJson( ): SocketJsonResponse { let parsed: any + // Truncate to keep error messages readable; full payload goes in the message. + const preview = jsonString.length > 200 + ? `${jsonString.slice(0, 200)}...` + : jsonString + // Check if it's valid JSON. try { parsed = JSON.parse(jsonString) - } catch (_e) { - throw new Error(`Invalid JSON output: ${jsonString}`) + } catch (e) { + throw new Error( + `command output is not valid JSON (JSON.parse threw: ${(e as Error).message}); got: ${preview} — check for unclosed braces, trailing commas, or non-JSON text mixed into stdout`, + ) } // Check for required ok field. if (typeof parsed.ok !== 'boolean') { throw new Error( - `JSON output missing required 'ok' boolean field: ${jsonString}`, + `Socket JSON contract violation: missing boolean "ok" field (contract: {ok: boolean, data?: any, message?: string}); got: ${preview} — add ok:true for success, ok:false for failure in the output handler`, ) } @@ -55,31 +62,31 @@ export function validateSocketJson( if (expectedExitCode === 0) { if (parsed.ok !== true) { throw new Error( - `JSON output 'ok' should be true when exit code is 0: ${jsonString}`, + `Socket JSON contract violation: exit code is 0 but "ok" is ${JSON.stringify(parsed.ok)} (expected true); got: ${preview} — set ok:true when the command succeeds, or return a non-zero exit code`, ) } // Success response must have data field. if (parsed.data === undefined || parsed.data === null) { throw new Error( - `JSON output missing required 'data' field when ok is true: ${jsonString}`, + `Socket JSON contract violation: ok:true must include a non-null "data" field (got: ${JSON.stringify(parsed.data)}); full output: ${preview} — return an empty object or array instead of undefined/null`, ) } } else { if (parsed.ok !== false) { throw new Error( - `JSON output 'ok' should be false when exit code is non-zero: ${jsonString}`, + `Socket JSON contract violation: exit code is ${expectedExitCode} but "ok" is ${JSON.stringify(parsed.ok)} (expected false); got: ${preview} — set ok:false on failure, or exit 0 on success`, ) } // Error response must have message field. if (typeof parsed.message !== 'string' || !parsed.message.length) { throw new Error( - `JSON output missing required 'message' field when ok is false: ${jsonString}`, + `Socket JSON contract violation: ok:false must include a non-empty "message" string (got: ${JSON.stringify(parsed.message)}); full output: ${preview} — provide a user-facing error description`, ) } // If code exists, it must be a number. if (parsed.code !== undefined && typeof parsed.code !== 'number') { throw new Error( - `JSON output 'code' field must be a number: ${jsonString}`, + `Socket JSON contract violation: "code" field must be a number when present (got: ${typeof parsed.code} ${JSON.stringify(parsed.code)}); full output: ${preview} — drop the field or set it to an integer`, ) } } diff --git a/packages/cli/src/test/mocks/socket-auth.mts b/packages/cli/src/test/mocks/socket-auth.mts index e3b58867a..558d66745 100644 --- a/packages/cli/src/test/mocks/socket-auth.mts +++ b/packages/cli/src/test/mocks/socket-auth.mts @@ -40,7 +40,9 @@ export function mockInteractiveLogin(options?: { shouldSucceed?: boolean }) { }, } } - throw new Error('Authentication failed') + throw new Error( + `mock interactive-login rejected (configured with shouldSucceed:false); this is a test fixture — flip shouldSucceed to true when testing the happy path`, + ) }) } @@ -118,7 +120,9 @@ export function mockOAuthPoller(options?: { token: MOCK_API_TOKEN, } } - throw new Error('OAuth timeout') + throw new Error( + `mock OAuth poller rejected after ${pollCount} polls (configured with shouldSucceed:false); this is a test fixture — set shouldSucceed:true or adjust pollCount to test the happy path`, + ) }) } diff --git a/packages/cli/test/json-output-validation.mts b/packages/cli/test/json-output-validation.mts index f33f17e2e..5e8235673 100644 --- a/packages/cli/test/json-output-validation.mts +++ b/packages/cli/test/json-output-validation.mts @@ -41,10 +41,18 @@ export function validateSocketJson(output: string, exitCode: number) { } return { ok: false, - message: parsed.message || parsed.error || 'Unknown error', + message: + parsed.message || + parsed.error || + `command exited with code ${exitCode} but returned JSON had no .message or .error field`, } - } catch (_e) { + } catch (e) { // If not valid JSON, return error. - return { ok: false, message: 'Invalid JSON output' } + const preview = + output.length > 200 ? `${output.slice(0, 200)}...` : output + return { + ok: false, + message: `command output is not valid JSON (JSON.parse: ${(e as Error).message}); got: ${preview}`, + } } } diff --git a/packages/cli/test/smoke.sh b/packages/cli/test/smoke.sh index 1e7b0faa5..04c094b48 100755 --- a/packages/cli/test/smoke.sh +++ b/packages/cli/test/smoke.sh @@ -93,7 +93,7 @@ validate_json() { # First check if it's valid JSON if ! echo "$json_output" | jq . > /dev/null 2>&1; then - echo -e "${RED}✗ Invalid JSON output${NC}" + echo -e "${RED}✗ command output is not valid JSON (jq rejected it); stdout may contain progress text mixed with the payload${NC}" echo -e "Received:" echo -e "$json_output" return 1 @@ -115,13 +115,13 @@ validate_json() { # Check if ok field matches expected exit code if [ "$expected_exit" -eq 0 ] && [ "$ok_field" != "true" ]; then - echo -e "${RED}✗ JSON output 'ok' should be true when exit code is 0${NC}" + echo -e "${RED}✗ Socket JSON contract violation: exit code 0 but \"ok\" is \"$ok_field\" (expected true); set ok:true on success${NC}" echo -e "Received:" echo -e "$json_output" return 1 fi if [ "$expected_exit" -ne 0 ] && [ "$ok_field" != "false" ]; then - echo -e "${RED}✗ JSON output 'ok' should be false when exit code is non-zero${NC}" + echo -e "${RED}✗ Socket JSON contract violation: exit code $expected_exit but \"ok\" is \"$ok_field\" (expected false); set ok:false on failure${NC}" echo -e "Received:" echo -e "$json_output" return 1 @@ -129,7 +129,7 @@ validate_json() { # Check if data field exists (required when ok is true, optional when false) if [ "$ok_field" = "true" ] && [ "$data_field" = "null" ]; then - echo -e "${RED}✗ JSON output missing required 'data' field when ok is true${NC}" + echo -e "${RED}✗ Socket JSON contract violation: ok:true must include a non-null \"data\" field; return an empty object/array if there's no payload${NC}" echo -e "Received:" echo -e "$json_output" return 1 @@ -137,7 +137,7 @@ validate_json() { # If ok is false, message is required if [ "$ok_field" = "false" ] && [ -z "$message_field" ]; then - echo -e "${RED}✗ JSON output missing required 'message' field when ok is false${NC}" + echo -e "${RED}✗ Socket JSON contract violation: ok:false must include a non-empty \"message\" string; provide a user-facing error description${NC}" echo -e "Received:" echo -e "$json_output" return 1 @@ -145,7 +145,7 @@ validate_json() { # If code exists, it must be a number if [ -n "$code_field" ] && ! [[ "$code_field" =~ ^[0-9]+$ ]]; then - echo -e "${RED}✗ JSON output 'code' field must be a number${NC}" + echo -e "${RED}✗ Socket JSON contract violation: \"code\" field must be a number when present (got: \"$code_field\"); drop the field or set it to an integer${NC}" echo -e "Received:" echo -e "$json_output" return 1 From a8efd4f9d8472cc7b8c9762fbbabf7a824a48c2a Mon Sep 17 00:00:00 2001 From: jdalton Date: Wed, 22 Apr 2026 11:51:22 -0400 Subject: [PATCH 2/2] chore(cli): harden (e as Error) casts to safe stringify Switch `(e as Error).message` to `e instanceof Error ? e.message : String(e)` so that when a non-Error value is thrown (strings, objects, null) the error message stays informative instead of becoming 'undefined'. Same fix as applied to #1260 (iocraft.mts) after Cursor bugbot flagged the pattern on that PR. --- packages/cli/src/test/json-output-validation.mts | 2 +- packages/cli/test/json-output-validation.mts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/cli/src/test/json-output-validation.mts b/packages/cli/src/test/json-output-validation.mts index 156f232b6..198b08bcc 100644 --- a/packages/cli/src/test/json-output-validation.mts +++ b/packages/cli/src/test/json-output-validation.mts @@ -47,7 +47,7 @@ export function validateSocketJson( parsed = JSON.parse(jsonString) } catch (e) { throw new Error( - `command output is not valid JSON (JSON.parse threw: ${(e as Error).message}); got: ${preview} — check for unclosed braces, trailing commas, or non-JSON text mixed into stdout`, + `command output is not valid JSON (JSON.parse threw: ${e instanceof Error ? e.message : String(e)}); got: ${preview} — check for unclosed braces, trailing commas, or non-JSON text mixed into stdout`, ) } diff --git a/packages/cli/test/json-output-validation.mts b/packages/cli/test/json-output-validation.mts index 5e8235673..9ea32429a 100644 --- a/packages/cli/test/json-output-validation.mts +++ b/packages/cli/test/json-output-validation.mts @@ -52,7 +52,7 @@ export function validateSocketJson(output: string, exitCode: number) { output.length > 200 ? `${output.slice(0, 200)}...` : output return { ok: false, - message: `command output is not valid JSON (JSON.parse: ${(e as Error).message}); got: ${preview}`, + message: `command output is not valid JSON (JSON.parse: ${e instanceof Error ? e.message : String(e)}); got: ${preview}`, } } }