From 0227840a75bb18f4de7174b70bfc75e2f82ce792 Mon Sep 17 00:00:00 2001 From: Andy Ford Date: Wed, 1 Apr 2026 11:38:24 +0100 Subject: [PATCH 01/15] enable @typescript-eslint/no-unused-vars for react-web-cli Remove dead code: unused imports (safeFit, CONTROL_MESSAGE_PREFIX, isHijackMetaChunk, colour), unused state (sessionIdInitialized, secondarySocket value, secondaryConnectionStatus value, secondarySpinnerPrefixReference, secondaryReconnectScheduledThisCycleReference), unused locals (colour object, wasReconnecting, wasConnecting, latestStatus, TERMINAL_PROMPT_IDENTIFIER), and unused variables in terminal-shared.ts (bufferLength, baseY, viewportY, cursorX). Also fixes a lonely-if lint error exposed by the cleanup. Co-Authored-By: Claude Opus 4.6 --- eslint.config.js | 9 +- .../src/AblyCliTerminal.resize.test.tsx | 2 +- .../src/AblyCliTerminal.test.tsx | 14 +- .../react-web-cli/src/AblyCliTerminal.tsx | 120 ++++++------------ packages/react-web-cli/src/terminal-shared.ts | 15 --- 5 files changed, 53 insertions(+), 107 deletions(-) diff --git a/eslint.config.js b/eslint.config.js index 170a65b4..720ea22d 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -153,7 +153,14 @@ export default [ "unicorn/prefer-code-point": "off", "unicorn/prevent-abbreviations": "off", "unicorn/no-array-reduce": "off", - "@typescript-eslint/no-unused-vars": "off", + "@typescript-eslint/no-unused-vars": [ + "error", + { + argsIgnorePattern: "^_", + varsIgnorePattern: "^_", + caughtErrorsIgnorePattern: "^_", + }, + ], "@typescript-eslint/no-explicit-any": "off", "@typescript-eslint/no-unsafe-assignment": "off", "@typescript-eslint/no-unsafe-call": "off", diff --git a/packages/react-web-cli/src/AblyCliTerminal.resize.test.tsx b/packages/react-web-cli/src/AblyCliTerminal.resize.test.tsx index d69f2f2d..ae5dcbd5 100644 --- a/packages/react-web-cli/src/AblyCliTerminal.resize.test.tsx +++ b/packages/react-web-cli/src/AblyCliTerminal.resize.test.tsx @@ -1,5 +1,5 @@ import React from "react"; -import { render, act, waitFor } from "@testing-library/react"; +import { render, act } from "@testing-library/react"; import { describe, vi, test, expect, beforeEach } from "vitest"; import { AblyCliTerminal } from "./AblyCliTerminal"; diff --git a/packages/react-web-cli/src/AblyCliTerminal.test.tsx b/packages/react-web-cli/src/AblyCliTerminal.test.tsx index 6ccbc0f3..e580a1a9 100644 --- a/packages/react-web-cli/src/AblyCliTerminal.test.tsx +++ b/packages/react-web-cli/src/AblyCliTerminal.test.tsx @@ -9,11 +9,7 @@ import { } from "@testing-library/react"; import "@testing-library/jest-dom"; import { vi, describe, test, expect, beforeEach, afterEach } from "vitest"; -import { - getAttempts, - getMaxAttempts, - isMaxAttemptsReached, -} from "./global-reconnect"; +import "./global-reconnect"; // Mock the GlobalReconnect module AT THE TOP LEVEL using a factory function. vi.mock("./global-reconnect", () => ({ @@ -121,8 +117,6 @@ vi.mock("@xterm/addon-fit", () => ({ import * as GlobalReconnect from "./global-reconnect"; import { AblyCliTerminal } from "./AblyCliTerminal"; // Import now import type { AblyCliTerminalHandle } from "./AblyCliTerminal"; -import * as TerminalBoxModule from "./terminal-box"; // Import to access mocked colours if needed for assertions - // Mock use-terminal-visibility vi.mock("./use-terminal-visibility", () => ({ useTerminalVisibility: () => true, @@ -130,8 +124,8 @@ vi.mock("./use-terminal-visibility", () => ({ // Mock lucide-react icons to simple stubs to avoid SVG complexity in tests vi.mock("lucide-react", () => ({ - SplitSquareHorizontal: (properties: any) => null, - X: (properties: any) => null, + SplitSquareHorizontal: (_properties: any) => null, + X: (_properties: any) => null, })); // Mock the crypto utility @@ -1013,7 +1007,7 @@ describe("AblyCliTerminal - Connection Status and Animation", () => { const consoleLogSpy = vi.spyOn(console, "log"); // Render component with resumeOnReload enabled so it reads the stored sessionId - const { container } = renderTerminal({ resumeOnReload: true }); + renderTerminal({ resumeOnReload: true }); // Wait for the session to be restored await waitFor(() => { diff --git a/packages/react-web-cli/src/AblyCliTerminal.tsx b/packages/react-web-cli/src/AblyCliTerminal.tsx index 3edc5777..460c6b9c 100644 --- a/packages/react-web-cli/src/AblyCliTerminal.tsx +++ b/packages/react-web-cli/src/AblyCliTerminal.tsx @@ -15,7 +15,6 @@ import { clearBox, updateLine, colour as boxColour, - colour, type TerminalBox, } from "./terminal-box"; import { @@ -41,14 +40,12 @@ import { filterDockerHandshake, createTerminal, createFitAddon, - safeFit, createAuthPayload, parseControlMessage, messageDataToUint8Array, clearConnectingMessage, showConnectingMessage, debugLog as sharedDebugLog, - CONTROL_MESSAGE_PREFIX, MAX_PTY_BUFFER_LENGTH, TERMINAL_PROMPT_PATTERN, } from "./terminal-shared"; @@ -81,9 +78,6 @@ export type ConnectionStatus = | "reconnecting" | "error"; -// Prompts that indicate the terminal is ready for input -const TERMINAL_PROMPT_IDENTIFIER = "ably> "; // Basic prompt - // Shared CLI installation tip const CLI_INSTALL_TIP = { lines: [ @@ -161,9 +155,6 @@ if (globalThis.window !== undefined) { } } -// Import isHijackMetaChunk from shared module for backward compatibility -import { isHijackMetaChunk } from "./terminal-shared"; - const AblyCliTerminalInner = ( { websocketUrl, @@ -505,7 +496,6 @@ const AblyCliTerminalInner = ( const [sessionId, setSessionId] = useState(null); const [credentialHash, setCredentialHash] = useState(null); const [credentialsInitialized, setCredentialsInitialized] = useState(false); - const [sessionIdInitialized, setSessionIdInitialized] = useState(false); // Track the second terminal's sessionId const [secondarySessionId, setSecondarySessionId] = useState( @@ -560,19 +550,6 @@ const AblyCliTerminalInner = ( const spinnerPrefixReference = useRef(""); const statusBoxReference = useRef(null); - // ANSI colour / style helpers - const colour = { - reset: "\u001B[0m", - bold: "\u001B[1m", - dim: "\u001B[2m", - red: "\u001B[31m", - green: "\u001B[32m", - yellow: "\u001B[33m", - blue: "\u001B[34m", - magenta: "\u001B[35m", - cyan: "\u001B[36m", - } as const; - const clearStatusDisplay = useCallback(() => { if (spinnerIntervalReference.current) { clearInterval(spinnerIntervalReference.current); @@ -833,9 +810,7 @@ const AblyCliTerminalInner = ( const secondaryTermCleanupReference = useRef<() => void>(() => {}); // Secondary terminal state - const [secondarySocket, setSecondarySocket] = useState( - null, - ); + const [, setSecondarySocket] = useState(null); const [isSecondarySessionActive, setIsSecondarySessionActive] = useState(false); const [secondaryOverlay, setSecondaryOverlay] = useState("initial"); - const [secondaryConnectionStatus, setSecondaryConnectionStatus] = + const [, setSecondaryConnectionStatus] = useState("initial"); - const secondarySpinnerPrefixReference = useRef(""); const secondarySpinnerIndexReference = useRef(0); const secondaryStatusBoxReference = useRef(null); const secondarySpinnerIntervalReference = useRef | null>(null); const secondaryShowManualReconnectPromptReference = useRef(false); - const [ - secondaryShowManualReconnectPrompt, - setSecondaryShowManualReconnectPrompt, - ] = useState(false); - const secondaryReconnectScheduledThisCycleReference = useRef(false); + const [, setSecondaryShowManualReconnectPrompt] = useState(false); // Secondary terminal: pending initial command and timeout refs const pendingSecondaryInitialCommandReference = useRef(null); @@ -874,10 +844,6 @@ const AblyCliTerminalInner = ( debugLog( `⚠️ DIAGNOSTIC: Received hello message with sessionId=${message.sessionId}`, ); - const wasReconnecting = - connectionStatusReference.current === "reconnecting"; - const wasConnecting = - connectionStatusReference.current === "connecting"; setSessionId(message.sessionId); if (onSessionId) onSessionId(message.sessionId); debugLog( @@ -2704,7 +2670,6 @@ const AblyCliTerminalInner = ( } setCredentialsInitialized(true); - setSessionIdInitialized(true); debugLog( `⚠️ DIAGNOSTIC: Credentials initialized, restored sessionId: ${storedSessionId || "none"}`, ); @@ -3393,54 +3358,49 @@ const AblyCliTerminalInner = ( // Handle data input in secondary terminal secondaryTerm.current.onData((data: string) => { - // Special handling for Enter key - if (data === "\r") { - const latestStatus = secondaryConnectionStatusReference.current; - - // Manual prompt visible: attempt manual reconnect even if an old socket is open - if (secondaryShowManualReconnectPromptReference.current) { - debugLog( - "[AblyCLITerminal] Secondary terminal: Enter pressed for manual reconnect.", - ); + // Special handling for Enter key when manual reconnect prompt is visible + if ( + data === "\r" && + secondaryShowManualReconnectPromptReference.current + ) { + debugLog( + "[AblyCLITerminal] Secondary terminal: Enter pressed for manual reconnect.", + ); - // Clear overlay and status displays - clearSecondaryStatusDisplay(); - secondaryShowManualReconnectPromptReference.current = false; - setSecondaryShowManualReconnectPrompt(false); + // Clear overlay and status displays + clearSecondaryStatusDisplay(); + secondaryShowManualReconnectPromptReference.current = false; + setSecondaryShowManualReconnectPrompt(false); - // Forget previous session completely so no resume is attempted - if (!resumeOnReload) { - setSecondarySessionId(null); - } + // Forget previous session completely so no resume is attempted + if (!resumeOnReload) { + setSecondarySessionId(null); + } - // Ensure any lingering socket is fully closed before opening a new one - if ( - secondarySocketReference.current && - secondarySocketReference.current.readyState !== WebSocket.CLOSED - ) { - try { - secondarySocketReference.current.close(1000, "manual-reconnect"); - } catch { - /* ignore */ - } - secondarySocketReference.current = null; // Make sure the reference is cleared + // Ensure any lingering socket is fully closed before opening a new one + if ( + secondarySocketReference.current && + secondarySocketReference.current.readyState !== WebSocket.CLOSED + ) { + try { + secondarySocketReference.current.close(1000, "manual-reconnect"); + } catch { + /* ignore */ } - - // Give the browser a micro-task to mark socket CLOSED before reconnect - setTimeout(() => { - debugLog( - "[AblyCLITerminal] [Secondary] Starting fresh reconnect sequence", - ); - secondaryPtyBuffer.current = ""; // Clear buffer - secondaryHandshakeFilterState.current = - createHandshakeFilterState(); // Reset handshake filter - connectSecondaryWebSocketReference.current?.(); - }, 20); - - return; + secondarySocketReference.current = null; // Make sure the reference is cleared } - // Handle other special cases like primary terminal if needed + // Give the browser a micro-task to mark socket CLOSED before reconnect + setTimeout(() => { + debugLog( + "[AblyCLITerminal] [Secondary] Starting fresh reconnect sequence", + ); + secondaryPtyBuffer.current = ""; // Clear buffer + secondaryHandshakeFilterState.current = createHandshakeFilterState(); + connectSecondaryWebSocketReference.current?.(); + }, 20); + + return; } // Track input line for autocomplete (simple tracking - reset on Enter) diff --git a/packages/react-web-cli/src/terminal-shared.ts b/packages/react-web-cli/src/terminal-shared.ts index dc30f2c5..43006402 100644 --- a/packages/react-web-cli/src/terminal-shared.ts +++ b/packages/react-web-cli/src/terminal-shared.ts @@ -97,14 +97,6 @@ export function filterDockerHandshake( return ""; } -/** - * Checks if a string contains Docker handshake markers (legacy function for compatibility) - * Note: This only works for complete chunks and is kept for backward compatibility - */ -export function isHijackMetaChunk(txt: string): boolean { - return /"stream"\s*:\s*true/.test(txt) || /"hijack"\s*:\s*true/.test(txt); -} - /** * Terminal configuration shared between primary and secondary terminals */ @@ -296,9 +288,6 @@ export function clearConnectingMessage(term: Terminal): void { const currentY = term.buffer.active.cursorY; const currentX = term.buffer.active.cursorX; const connectingLine = termAny._connectingLine; - const bufferLength = term.buffer.active.length; - const baseY = term.buffer.active.baseY; - const viewportY = term.buffer.active.viewportY; // Move to the connecting line and clear it term.write(`\u001B[${connectingLine + 1};1H`); // Move to line @@ -324,10 +313,6 @@ export function showConnectingMessage( ): void { try { const cursorY = term.buffer.active.cursorY; - const cursorX = term.buffer.active.cursorX; - const bufferLength = term.buffer.active.length; - const baseY = term.buffer.active.baseY; - const viewportY = term.buffer.active.viewportY; term.writeln(message); From 97630b93d7f1640eacf3aa5687ec1cb236362490 Mon Sep 17 00:00:00 2001 From: Andy Ford Date: Wed, 1 Apr 2026 11:39:20 +0100 Subject: [PATCH 02/15] enable unicorn/prefer-string-slice for react-web-cli Remove the "off" override and replace two substring() calls with slice() in the control message parsing logic. Co-Authored-By: Claude Opus 4.6 --- eslint.config.js | 1 - packages/react-web-cli/src/AblyCliTerminal.tsx | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/eslint.config.js b/eslint.config.js index 720ea22d..e7a4546c 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -149,7 +149,6 @@ export default [ "unicorn/prefer-module": "off", "unicorn/no-negated-condition": "off", "unicorn/filename-case": "off", - "unicorn/prefer-string-slice": "off", "unicorn/prefer-code-point": "off", "unicorn/prevent-abbreviations": "off", "unicorn/no-array-reduce": "off", diff --git a/packages/react-web-cli/src/AblyCliTerminal.tsx b/packages/react-web-cli/src/AblyCliTerminal.tsx index 460c6b9c..bda6246b 100644 --- a/packages/react-web-cli/src/AblyCliTerminal.tsx +++ b/packages/react-web-cli/src/AblyCliTerminal.tsx @@ -1550,7 +1550,7 @@ const AblyCliTerminalInner = ( // Append any text before the control message if (ctrlIndex > currentPos) { - regularOutput += filteredData.substring(currentPos, ctrlIndex); + regularOutput += filteredData.slice(currentPos, ctrlIndex); } // Find the end of the control message (either newline or end of string) @@ -1560,7 +1560,7 @@ const AblyCliTerminalInner = ( if (messageEnd === -1) messageEnd = filteredData.length; // Extract the control message - const ctrlLine = filteredData.substring(ctrlIndex, messageEnd); + const ctrlLine = filteredData.slice(ctrlIndex, messageEnd); try { const jsonString = ctrlLine.slice("ABLY_CTRL:".length); From ef30656f947a03768db044c743f2fd1c779aa745 Mon Sep 17 00:00:00 2001 From: Andy Ford Date: Wed, 1 Apr 2026 11:40:25 +0100 Subject: [PATCH 03/15] enable unicorn/prefer-code-point for react-web-cli Replace all charCodeAt() calls with codePointAt() for proper surrogate pair handling. Fixes potential bugs with emoji/wide characters in TerminalOverlay.tsx and AblyCliTerminal.tsx. Co-Authored-By: Claude Opus 4.6 --- eslint.config.js | 1 - packages/react-web-cli/src/AblyCliTerminal.tsx | 10 +++++----- packages/react-web-cli/src/TerminalOverlay.tsx | 8 +++++--- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/eslint.config.js b/eslint.config.js index e7a4546c..0170d554 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -149,7 +149,6 @@ export default [ "unicorn/prefer-module": "off", "unicorn/no-negated-condition": "off", "unicorn/filename-case": "off", - "unicorn/prefer-code-point": "off", "unicorn/prevent-abbreviations": "off", "unicorn/no-array-reduce": "off", "@typescript-eslint/no-unused-vars": [ diff --git a/packages/react-web-cli/src/AblyCliTerminal.tsx b/packages/react-web-cli/src/AblyCliTerminal.tsx index bda6246b..24f821af 100644 --- a/packages/react-web-cli/src/AblyCliTerminal.tsx +++ b/packages/react-web-cli/src/AblyCliTerminal.tsx @@ -2045,7 +2045,7 @@ const AblyCliTerminalInner = ( // Enhanced logging for special keys const bytes = [...data] .map((char) => { - const code = char.charCodeAt(0); + const code = char.codePointAt(0)!; if (code < 32 || code > 126) { return `\\x${code.toString(16).padStart(2, "0")}`; } @@ -2217,8 +2217,8 @@ const AblyCliTerminalInner = ( } } else if ( data.length === 1 && - data.charCodeAt(0) >= 32 && - data.charCodeAt(0) <= 126 + data.codePointAt(0)! >= 32 && + data.codePointAt(0)! <= 126 ) { // Regular printable character currentInputLine = @@ -3417,8 +3417,8 @@ const AblyCliTerminalInner = ( } } else if ( data.length === 1 && - data.charCodeAt(0) >= 32 && - data.charCodeAt(0) <= 126 + data.codePointAt(0)! >= 32 && + data.codePointAt(0)! <= 126 ) { // Regular printable character secondaryCurrentInputLine = diff --git a/packages/react-web-cli/src/TerminalOverlay.tsx b/packages/react-web-cli/src/TerminalOverlay.tsx index ccc16a14..242da990 100644 --- a/packages/react-web-cli/src/TerminalOverlay.tsx +++ b/packages/react-web-cli/src/TerminalOverlay.tsx @@ -274,7 +274,7 @@ function stripHtml(string_: string): string { function getStringWidth(string_: string): number { let width = 0; for (let index = 0; index < string_.length; index++) { - const code = string_.charCodeAt(index); + const code = string_.codePointAt(index)!; // Check for emoji and wide characters if ( code >= 0x1_f0_00 || // Emoji blocks @@ -288,11 +288,13 @@ function getStringWidth(string_: string): number { ) { // Supplemental Symbols and Pictographs width += 2; + // Skip low surrogate for supplementary-plane characters (code point > 0xFFFF) + if (code > 0xff_ff) index++; // Skip next char if it's a combining character if ( index + 1 < string_.length && - string_.charCodeAt(index + 1) >= 0xfe_00 && - string_.charCodeAt(index + 1) <= 0xfe_0f + string_.codePointAt(index + 1)! >= 0xfe_00 && + string_.codePointAt(index + 1)! <= 0xfe_0f ) { index++; } From 17e05c5f1990d98cce07eac0066fbf1584ba04f0 Mon Sep 17 00:00:00 2001 From: Andy Ford Date: Wed, 1 Apr 2026 11:42:12 +0100 Subject: [PATCH 04/15] enable unicorn/no-negated-condition for react-web-cli Flip negated if/else conditions to positive-first style in AblyCliTerminal.tsx (handlePtyData, handleWebSocketOpen) and use-terminal-visibility.ts (recompute ternaries). Co-Authored-By: Claude Opus 4.6 --- eslint.config.js | 1 - packages/react-web-cli/src/AblyCliTerminal.tsx | 14 +++++++------- .../react-web-cli/src/use-terminal-visibility.ts | 8 ++++---- 3 files changed, 11 insertions(+), 12 deletions(-) diff --git a/eslint.config.js b/eslint.config.js index 0170d554..bd4c3b6b 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -147,7 +147,6 @@ export default [ ...tsPlugin.configs.recommended.rules, // Custom overrides for this package "unicorn/prefer-module": "off", - "unicorn/no-negated-condition": "off", "unicorn/filename-case": "off", "unicorn/prevent-abbreviations": "off", "unicorn/no-array-reduce": "off", diff --git a/packages/react-web-cli/src/AblyCliTerminal.tsx b/packages/react-web-cli/src/AblyCliTerminal.tsx index 24f821af..8cdd18be 100644 --- a/packages/react-web-cli/src/AblyCliTerminal.tsx +++ b/packages/react-web-cli/src/AblyCliTerminal.tsx @@ -733,7 +733,9 @@ const AblyCliTerminalInner = ( `⚠️ DIAGNOSTIC: handlePtyData called. isSessionActive: ${isSessionActiveReference.current}, data: "${sanitizedData}"`, ); - if (!isSessionActiveReference.current) { + if (isSessionActiveReference.current) { + debugLog(`⚠️ DIAGNOSTIC: Session already active, not buffering data`); + } else { ptyBuffer.current += data; debugLog( @@ -784,8 +786,6 @@ const AblyCliTerminalInner = ( clearPtyBuffer(); } - } else { - debugLog(`⚠️ DIAGNOSTIC: Session already active, not buffering data`); } }, [ @@ -1419,10 +1419,7 @@ const AblyCliTerminalInner = ( setShowManualReconnectPrompt(false); // Only clear buffer for new sessions, not when resuming - if (!sessionId) { - clearPtyBuffer(); // Clear buffer for new session prompt detection - debugLog(`⚠️ DIAGNOSTIC: Cleared PTY buffer for new session`); - } else { + if (sessionId) { debugLog( `⚠️ DIAGNOSTIC: Skipping PTY buffer clear for resumed session ${sessionId}`, ); @@ -1436,6 +1433,9 @@ const AblyCliTerminalInner = ( `⚠️ DIAGNOSTIC: Resumed session but not active - checking for existing prompt`, ); } + } else { + clearPtyBuffer(); // Clear buffer for new session prompt detection + debugLog(`⚠️ DIAGNOSTIC: Cleared PTY buffer for new session`); } debugLog( diff --git a/packages/react-web-cli/src/use-terminal-visibility.ts b/packages/react-web-cli/src/use-terminal-visibility.ts index 2dd4e9ff..47aeef0e 100644 --- a/packages/react-web-cli/src/use-terminal-visibility.ts +++ b/packages/react-web-cli/src/use-terminal-visibility.ts @@ -18,11 +18,11 @@ export function useTerminalVisibility( useEffect(() => { function recompute(intersecting: boolean | null = null) { const isIntersecting = - intersecting != null ? intersecting : lastIntersectionRef.current; + intersecting == null ? lastIntersectionRef.current : intersecting; const pageVisible = - typeof document !== "undefined" - ? document.visibilityState === "visible" - : true; + typeof document === "undefined" + ? true + : document.visibilityState === "visible"; setVisible(isIntersecting && pageVisible); } From 3a3b1f41b42ec2a9363bd8fa1b6db66d43c79e03 Mon Sep 17 00:00:00 2001 From: Andy Ford Date: Wed, 1 Apr 2026 11:44:27 +0100 Subject: [PATCH 05/15] install eslint-plugin-react-hooks and enable rules for react-web-cli Add rules-of-hooks (error) to catch hook ordering bugs, and exhaustive-deps (warn) to flag stale closures in the large AblyCliTerminal component. This is the highest-value lint addition for catching React-specific bugs. Co-Authored-By: Claude Opus 4.6 --- eslint.config.js | 5 +++++ package.json | 1 + pnpm-lock.yaml | 7 +++++++ 3 files changed, 13 insertions(+) diff --git a/eslint.config.js b/eslint.config.js index bd4c3b6b..d061ffae 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -9,6 +9,7 @@ import eslintPluginPrettier from "eslint-plugin-prettier"; import eslint from "@eslint/js"; // Import base eslint config import vitest from '@vitest/eslint-plugin' import eslintPluginReact from "eslint-plugin-react" +import eslintPluginReactHooks from "eslint-plugin-react-hooks" export default [ { @@ -120,6 +121,7 @@ export default [ files: ["packages/react-web-cli/**/*.{ts,tsx}"], plugins: { react: eslintPluginReact, + "react-hooks": eslintPluginReactHooks, "@typescript-eslint": tsPlugin, }, languageOptions: { @@ -145,6 +147,9 @@ export default [ ...eslintPluginReact.configs["jsx-runtime"].rules, // TypeScript rules ...tsPlugin.configs.recommended.rules, + // React hooks rules + "react-hooks/rules-of-hooks": "error", + "react-hooks/exhaustive-deps": "warn", // Custom overrides for this package "unicorn/prefer-module": "off", "unicorn/filename-case": "off", diff --git a/package.json b/package.json index f11307f9..b6f06a16 100644 --- a/package.json +++ b/package.json @@ -158,6 +158,7 @@ "eslint-config-prettier": "^10.1.2", "eslint-plugin-n": "^17.17.0", "eslint-plugin-prettier": "^5.2.6", + "eslint-plugin-react-hooks": "^5.2.0", "eslint-plugin-unicorn": "^58.0.0", "execa": "^9.5.2", "fs-extra": "^11.3.0", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index e2b85a09..6fb86871 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -172,6 +172,9 @@ importers: eslint-plugin-prettier: specifier: ^5.2.6 version: 5.2.6(eslint-config-prettier@10.1.2(eslint@9.39.4(jiti@2.4.2)))(eslint@9.39.4(jiti@2.4.2))(prettier@3.5.3) + eslint-plugin-react-hooks: + specifier: ^5.2.0 + version: 5.2.0(eslint@9.39.4(jiti@2.4.2)) eslint-plugin-unicorn: specifier: ^58.0.0 version: 58.0.0(eslint@9.39.4(jiti@2.4.2)) @@ -9341,6 +9344,10 @@ snapshots: dependencies: eslint: 9.34.0(jiti@2.4.2) + eslint-plugin-react-hooks@5.2.0(eslint@9.39.4(jiti@2.4.2)): + dependencies: + eslint: 9.39.4(jiti@2.4.2) + eslint-plugin-react-refresh@0.4.19(eslint@9.34.0(jiti@2.4.2)): dependencies: eslint: 9.34.0(jiti@2.4.2) From a9a3cb5a0129c20ec01e2c917511d0b8743176cd Mon Sep 17 00:00:00 2001 From: Andy Ford Date: Wed, 1 Apr 2026 11:48:09 +0100 Subject: [PATCH 06/15] enable no-console as warn for react-web-cli Migrate ~25 console.log calls to debugLog() so they only fire when ABLY_CLI_DEBUG is enabled. console.warn and console.error remain allowed. Update two tests that asserted on console.log to enable debug mode and match the "[AblyCLITerminal DEBUG]" prefix. Co-Authored-By: Claude Opus 4.6 --- eslint.config.js | 2 +- .../src/AblyCliTerminal.test.tsx | 10 ++++-- .../react-web-cli/src/AblyCliTerminal.tsx | 14 ++++---- .../react-web-cli/src/global-reconnect.ts | 32 ++++++++++--------- packages/react-web-cli/src/terminal-shared.ts | 5 +-- 5 files changed, 36 insertions(+), 27 deletions(-) diff --git a/eslint.config.js b/eslint.config.js index d061ffae..78662b2e 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -170,7 +170,7 @@ export default [ "@typescript-eslint/no-unsafe-argument": "off", "@typescript-eslint/no-unsafe-return": "off", "@typescript-eslint/no-base-to-string": "off", - "no-console": "off", + "no-console": ["warn", { allow: ["warn", "error"] }], "no-control-regex": "off", // Terminal escape sequences use control chars "n/no-missing-import": "off", // TSX imports are handled by TypeScript "react/prop-types": "off", // Using TypeScript for prop validation diff --git a/packages/react-web-cli/src/AblyCliTerminal.test.tsx b/packages/react-web-cli/src/AblyCliTerminal.test.tsx index e580a1a9..e0669148 100644 --- a/packages/react-web-cli/src/AblyCliTerminal.test.tsx +++ b/packages/react-web-cli/src/AblyCliTerminal.test.tsx @@ -1003,7 +1003,8 @@ describe("AblyCliTerminal - Connection Status and Animation", () => { expectedHash, ); - // Spy on console.log before rendering to catch all logs + // Enable debug logging so debugLog() calls actually reach console.log + (globalThis as any).ABLY_CLI_DEBUG = true; const consoleLogSpy = vi.spyOn(console, "log"); // Render component with resumeOnReload enabled so it reads the stored sessionId @@ -1012,10 +1013,12 @@ describe("AblyCliTerminal - Connection Status and Animation", () => { // Wait for the session to be restored await waitFor(() => { expect(consoleLogSpy).toHaveBeenCalledWith( + "[AblyCLITerminal DEBUG]", "[AblyCLITerminal] Restored session with matching credentials for domain:", "web-cli-terminal.ably-dev.com", ); }); + (globalThis as any).ABLY_CLI_DEBUG = false; // Wait for the WebSocket to reconnect with the restored sessionId // The component may create multiple connections as it loads the sessionId @@ -1881,7 +1884,8 @@ describe("AblyCliTerminal - Credential Validation", () => { expectedHash, ); - // Spy on console.log before rendering + // Enable debug logging so debugLog() calls actually reach console.log + (globalThis as any).ABLY_CLI_DEBUG = true; const consoleLogSpy = vi.spyOn(console, "log"); // Render with matching credentials (default config has test-key:test-token) @@ -1890,10 +1894,12 @@ describe("AblyCliTerminal - Credential Validation", () => { // Wait for the session to be restored await waitFor(() => { expect(consoleLogSpy).toHaveBeenCalledWith( + "[AblyCLITerminal DEBUG]", "[AblyCLITerminal] Restored session with matching credentials for domain:", "web-cli-terminal.ably-dev.com", ); }); + (globalThis as any).ABLY_CLI_DEBUG = false; // Wait for WebSocket to send auth with sessionId await waitFor( diff --git a/packages/react-web-cli/src/AblyCliTerminal.tsx b/packages/react-web-cli/src/AblyCliTerminal.tsx index 8cdd18be..12113e63 100644 --- a/packages/react-web-cli/src/AblyCliTerminal.tsx +++ b/packages/react-web-cli/src/AblyCliTerminal.tsx @@ -1687,7 +1687,7 @@ const AblyCliTerminalInner = ( // Immediately enter "reconnecting" state so countdown / spinner UI is active updateConnectionStatusAndExpose("reconnecting"); - console.log( + debugLog( "[AblyCLITerminal handleWebSocketError] Entered reconnection branch. isCancelled=", grIsCancelledState(), "maxReached=", @@ -1705,13 +1705,13 @@ const AblyCliTerminalInner = ( startConnectingAnimation(true); grIncrement(); - console.log( + debugLog( "[AblyCLITerminal handleWebSocketError] grIncrement done. Attempts now:", grGetAttempts(), ); if (connectWebSocketReference.current) { - console.log( + debugLog( "[AblyCLITerminal handleWebSocketError] Scheduling reconnect...", ); grScheduleReconnect(connectWebSocketReference.current!, websocketUrl); @@ -2635,7 +2635,7 @@ const AblyCliTerminalInner = ( `ably.cli.credentialHash.${urlDomain}`, ); - console.log("[AblyCLITerminal] Credential validation:", { + debugLog("[AblyCLITerminal] Credential validation:", { urlDomain, storedSessionId, storedHash, @@ -2649,7 +2649,7 @@ const AblyCliTerminalInner = ( `⚠️ DIAGNOSTIC: Restoring sessionId ${storedSessionId} from sessionStorage`, ); setSessionId(storedSessionId); - console.log( + debugLog( "[AblyCLITerminal] Restored session with matching credentials for domain:", urlDomain, ); @@ -2663,7 +2663,7 @@ const AblyCliTerminalInner = ( globalThis.sessionStorage.removeItem( `ably.cli.credentialHash.${urlDomain}`, ); - console.log( + debugLog( "[AblyCLITerminal] Cleared session due to credential mismatch for domain:", urlDomain, ); @@ -2848,7 +2848,7 @@ const AblyCliTerminalInner = ( } debugLog("[AblyCLITerminal] Creating secondary WebSocket instance"); - console.log( + debugLog( "[AblyCLITerminal] Secondary WebSocket connecting to:", websocketUrl, ); diff --git a/packages/react-web-cli/src/global-reconnect.ts b/packages/react-web-cli/src/global-reconnect.ts index 4ff7c76b..08ea63f7 100644 --- a/packages/react-web-cli/src/global-reconnect.ts +++ b/packages/react-web-cli/src/global-reconnect.ts @@ -3,6 +3,8 @@ * This is kept outside React's lifecycle to maintain state between component remounts */ +import { debugLog } from "./terminal-shared"; + // Global state let attempts = 0; let isCancelled = false; @@ -31,7 +33,7 @@ export function getBackoffDelay(attempt: number): number { * Reset the reconnection state */ export function resetState(): void { - console.log( + debugLog( `[GlobalReconnect] resetState called. Current attempts (before potential reset): ${attempts}, isCancelled: ${isCancelled}`, ); // ATTEMPTS ARE NO LONGER RESET HERE. They should be reset by a successful connection @@ -42,14 +44,14 @@ export function resetState(): void { // scheduleReconnect will handle its own timer. isCancelled drives this. if (isCancelled) { if (countdownTimer) { - console.log( + debugLog( "[GlobalReconnect] resetState: Clearing countdownTimer because isCancelled=true", ); clearInterval(countdownTimer); countdownTimer = null; } if (reconnectTimer) { - console.log( + debugLog( "[GlobalReconnect] resetState: Clearing reconnectTimer because isCancelled=true", ); clearTimeout(reconnectTimer); @@ -65,7 +67,7 @@ export function resetState(): void { */ export function increment(): void { attempts++; - console.log(`[GlobalReconnect] Attempt counter incremented to ${attempts}`); + debugLog(`[GlobalReconnect] Attempt counter incremented to ${attempts}`); } /** @@ -100,7 +102,7 @@ export function isMaxAttemptsReached(): boolean { * Cancel reconnection attempts */ export function cancelReconnect(): void { - console.log( + debugLog( "[GlobalReconnect] cancelReconnect called. Clearing timers and resetting attempts.", ); isCancelled = true; @@ -135,7 +137,7 @@ export function scheduleReconnect( url: string, ): void { if (attempts >= maxAttempts) { - console.log( + debugLog( `[GlobalReconnect] scheduleReconnect: Aborting due to maxAttemptsReached (attempts=${attempts}, max=${maxAttempts})`, ); console.warn("[GlobalReconnect] Maximum reconnection attempts reached."); @@ -148,7 +150,7 @@ export function scheduleReconnect( isCancelled = false; const delay = getBackoffDelay(attempts); - console.log( + debugLog( `[GlobalReconnect] scheduleReconnect: Current attempts: ${attempts}, Calculated delay: ${delay}ms, isCancelled: ${isCancelled}`, ); @@ -175,33 +177,33 @@ export function scheduleReconnect( } if (reconnectTimer) { - console.log( + debugLog( "[GlobalReconnect] scheduleReconnect: Clearing existing reconnectTimer before setting new one.", ); clearTimeout(reconnectTimer); reconnectTimer = null; // Explicitly nullify after clearing } - console.log( + debugLog( `[GlobalReconnect] PRE-SETIMEOUT: About to set timer for attempt #${attempts + 1} with delay ${delay}ms.`, ); reconnectTimer = setTimeout(() => { - console.log( + debugLog( `[GlobalReconnect] SETTIMEOUT_FIRED_RAW for attempt #${attempts + 1}. isCancelled: ${isCancelled}`, ); if (isCancelled) { - console.log( + debugLog( `[GlobalReconnect] reconnectTimer FIRED but isCancelled is true. Aborting reconnectCallback for attempt #${attempts + 1}.`, ); return; } - console.log( + debugLog( `[GlobalReconnect] reconnectTimer FIRED. Attempting connection to ${url}, attempt #${attempts + 1}`, ); reconnectCallback(); }, delay); - console.log( + debugLog( `[GlobalReconnect] scheduleReconnect: NEW reconnectTimer scheduled with ID: ${String(reconnectTimer)} for attempt #${attempts + 1}`, ); } @@ -212,7 +214,7 @@ export function setMaxAttempts(value: number): void { } export function successfulConnectionReset(): void { - console.log( + debugLog( `[GlobalReconnect] successfulConnectionReset called. Resetting attempts from ${attempts} to 0.`, ); attempts = 0; @@ -223,7 +225,7 @@ export function successfulConnectionReset(): void { countdownTimer = null; } if (reconnectTimer) { - console.log( + debugLog( "[GlobalReconnect] successfulConnectionReset: Clearing reconnectTimer.", ); clearTimeout(reconnectTimer); diff --git a/packages/react-web-cli/src/terminal-shared.ts b/packages/react-web-cli/src/terminal-shared.ts index 43006402..12d95c0e 100644 --- a/packages/react-web-cli/src/terminal-shared.ts +++ b/packages/react-web-cli/src/terminal-shared.ts @@ -203,7 +203,7 @@ export function createAuthPayload( if (parsedConfig.accessToken) { payload.accessToken = parsedConfig.accessToken; } - console.log("[createAuthPayload] Using signed config auth", { + debugLog("[createAuthPayload] Using signed config auth", { hasApiKey: !!payload.apiKey, hasAccessToken: !!payload.accessToken, }); @@ -222,7 +222,7 @@ export function createAuthPayload( payload.ciAuthToken = ciToken; // Debug logging in CI if (win.__ABLY_CLI_CI_MODE__ === "true") { - console.log("[CI Auth] Including CI auth token in payload", { + debugLog("[CI Auth] Including CI auth token in payload", { tokenLength: ciToken.length, testGroup: win.__ABLY_CLI_TEST_GROUP__ || "unknown", runId: win.__ABLY_CLI_RUN_ID__ || "unknown", @@ -331,6 +331,7 @@ export function showConnectingMessage( */ export function debugLog(...args: unknown[]): void { if (typeof globalThis !== "undefined" && (globalThis as any).ABLY_CLI_DEBUG) { + // eslint-disable-next-line no-console -- deliberate debug output gate console.log("[AblyCLITerminal DEBUG]", ...args); } } From 669442a87f4ca043a7eb82a63116b2a5c3a38d72 Mon Sep 17 00:00:00 2001 From: Andy Ford Date: Wed, 1 Apr 2026 11:54:29 +0100 Subject: [PATCH 07/15] enable @typescript-eslint/no-explicit-any as warn for react-web-cli Create types.ts with ControlMessage union, AblyCliGlobals interface, TerminalWithConnectingState, and WebSocketMessageData type to replace all ~24 explicit 'any' annotations in source files with proper types. Key changes: - Control message handler now uses discriminated union type - globalThis casts use AblyCliGlobals interface instead of 'any' - Terminal extension properties use TerminalWithConnectingState - messageDataToUint8Array uses WebSocketMessageData type - debounce utility uses unknown[] instead of any[] - terminal-box stub uses unknown instead of any Co-Authored-By: Claude Opus 4.6 --- eslint.config.js | 2 +- .../react-web-cli/src/AblyCliTerminal.tsx | 36 ++++++------ packages/react-web-cli/src/terminal-box.ts | 14 ++--- packages/react-web-cli/src/terminal-shared.ts | 34 +++++++---- packages/react-web-cli/src/types.ts | 58 +++++++++++++++++++ 5 files changed, 108 insertions(+), 36 deletions(-) create mode 100644 packages/react-web-cli/src/types.ts diff --git a/eslint.config.js b/eslint.config.js index 78662b2e..6290d078 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -163,7 +163,7 @@ export default [ caughtErrorsIgnorePattern: "^_", }, ], - "@typescript-eslint/no-explicit-any": "off", + "@typescript-eslint/no-explicit-any": "warn", "@typescript-eslint/no-unsafe-assignment": "off", "@typescript-eslint/no-unsafe-call": "off", "@typescript-eslint/no-unsafe-member-access": "off", diff --git a/packages/react-web-cli/src/AblyCliTerminal.tsx b/packages/react-web-cli/src/AblyCliTerminal.tsx index 12113e63..189ad25e 100644 --- a/packages/react-web-cli/src/AblyCliTerminal.tsx +++ b/packages/react-web-cli/src/AblyCliTerminal.tsx @@ -34,6 +34,7 @@ import { useTerminalVisibility } from "./use-terminal-visibility.js"; import { SplitSquareHorizontal, X } from "lucide-react"; import { hashCredentials } from "./utils/crypto"; import { getConnectionMessage } from "./connection-messages"; +import type { ControlMessage, AblyCliGlobals } from "./types"; import { HandshakeFilterState, createHandshakeFilterState, @@ -53,7 +54,7 @@ import { /** * Simple debounce utility function to prevent rapid successive calls */ -function debounce any>( +function debounce void>( function_: T, wait: number, ): (...arguments_: Parameters) => void { @@ -148,7 +149,7 @@ if (globalThis.window !== undefined) { "cliDebug", ); if (urlFlag === "true") { - (globalThis as any).ABLY_CLI_DEBUG = true; + (globalThis as typeof globalThis & AblyCliGlobals).ABLY_CLI_DEBUG = true; } } catch { /* ignore URL parsing errors in non-browser env */ @@ -594,11 +595,9 @@ const AblyCliTerminalInner = ( connectionStatusReference.current = status; // Only report status changes from the primary terminal - if ( - typeof (globalThis as any).setWindowTestFlagOnStatusChange === - "function" - ) { - (globalThis as any).setWindowTestFlagOnStatusChange(status); + const g = globalThis as typeof globalThis & AblyCliGlobals; + if (typeof g.setWindowTestFlagOnStatusChange === "function") { + g.setWindowTestFlagOnStatusChange(status); } if (onConnectionStatusChange) { @@ -839,7 +838,7 @@ const AblyCliTerminalInner = ( // Extracted control message handler to avoid duplication const handleControlMessage = useCallback( - (message: any) => { + (message: ControlMessage) => { if (message.type === "hello" && typeof message.sessionId === "string") { debugLog( `⚠️ DIAGNOSTIC: Received hello message with sessionId=${message.sessionId}`, @@ -1286,7 +1285,7 @@ const AblyCliTerminalInner = ( socketReference.current && socketReference.current.readyState === WebSocket.OPEN ) { - if ((globalThis as any).ABLY_CLI_DEBUG) + if ((globalThis as typeof globalThis & AblyCliGlobals).ABLY_CLI_DEBUG) console.warn( "[AblyCLITerminal] connectWebSocket already open/connecting – skip", ); @@ -1348,7 +1347,8 @@ const AblyCliTerminalInner = ( `⚠️ DIAGNOSTIC: New WebSocket created with ID: ${Math.random().toString(36).slice(2, 10)}`, ); - (globalThis as any).ablyCliSocket = newSocket; // For E2E tests + (globalThis as typeof globalThis & AblyCliGlobals).ablyCliSocket = + newSocket; socketReference.current = newSocket; // Use ref for listeners setSocket(newSocket); // Trigger effect to add listeners @@ -1658,7 +1658,7 @@ const AblyCliTerminalInner = ( message = getConnectionMessage("maxReconnects"); } else if (grIsCancelledState()) { message = getConnectionMessage("reconnectCancelled"); - } else if ((event as any).isTimeout) { + } else if ((event as Event & { isTimeout?: boolean }).isTimeout) { message = getConnectionMessage("connectionTimeout"); } else { message = getConnectionMessage("connectionFailed"); @@ -2385,7 +2385,8 @@ const AblyCliTerminalInner = ( useEffect(() => { // Expose a debug function to get current component state for Playwright - (globalThis as any).getAblyCliTerminalReactState = () => ({ + const g = globalThis as typeof globalThis & AblyCliGlobals; + g.getAblyCliTerminalReactState = () => ({ componentConnectionStatus, isSessionActive, connectionHelpMessage, @@ -2398,7 +2399,7 @@ const AblyCliTerminalInner = ( }); // Expose a function to get terminal buffer content for testing - (globalThis as any).getTerminalBufferText = () => { + g.getTerminalBufferText = () => { if (!term.current) return ""; const buffer = term.current.buffer.active; let text = ""; @@ -2412,7 +2413,7 @@ const AblyCliTerminalInner = ( }; // Expose terminal buffer info for debugging - (globalThis as any).getTerminalBufferInfo = () => { + g.getTerminalBufferInfo = () => { if (!term.current) return { exists: false }; const buffer = term.current.buffer.active; return { @@ -2426,9 +2427,10 @@ const AblyCliTerminalInner = ( }; return () => { - delete (globalThis as any).getAblyCliTerminalReactState; - delete (globalThis as any).getTerminalBufferText; - delete (globalThis as any).getTerminalBufferInfo; + const g2 = globalThis as typeof globalThis & AblyCliGlobals; + delete g2.getAblyCliTerminalReactState; + delete g2.getTerminalBufferText; + delete g2.getTerminalBufferInfo; }; }, [ componentConnectionStatus, diff --git a/packages/react-web-cli/src/terminal-box.ts b/packages/react-web-cli/src/terminal-box.ts index b267b8b1..65681afd 100644 --- a/packages/react-web-cli/src/terminal-box.ts +++ b/packages/react-web-cli/src/terminal-box.ts @@ -14,17 +14,17 @@ export interface TerminalBox { row: number; width: number; content: string[]; - term: any; + term: unknown; height: number; } -export function drawBox(..._args: any[]): TerminalBox { +export function drawBox(..._args: unknown[]): TerminalBox { return { row: 0, width: 0, content: [], term: null, height: 0 }; } -export function clearBox(_: TerminalBox): void {} +export function clearBox(_box: TerminalBox): void {} export function updateLine( - _: TerminalBox, - __: number, - ___: string, - ____?: string, + _box: TerminalBox, + _line: number, + _text: string, + _color?: string, ): void {} export function updateSpinner(): void {} diff --git a/packages/react-web-cli/src/terminal-shared.ts b/packages/react-web-cli/src/terminal-shared.ts index 12d95c0e..2b52d89a 100644 --- a/packages/react-web-cli/src/terminal-shared.ts +++ b/packages/react-web-cli/src/terminal-shared.ts @@ -6,6 +6,12 @@ import { Terminal } from "@xterm/xterm"; import { FitAddon } from "@xterm/addon-fit"; import { WebLinksAddon } from "@xterm/addon-web-links"; +import type { + ControlMessage, + TerminalWithConnectingState, + AblyCliGlobals, + WebSocketMessageData, +} from "./types"; // Constants export const MAX_PTY_BUFFER_LENGTH = 10000; @@ -216,7 +222,7 @@ export function createAuthPayload( // Check for CI auth token in window object // This will be injected during test execution - const win = globalThis as any; + const win = globalThis as typeof globalThis & AblyCliGlobals; if (win.__ABLY_CLI_CI_AUTH_TOKEN__) { const ciToken: string = win.__ABLY_CLI_CI_AUTH_TOKEN__; payload.ciAuthToken = ciToken; @@ -240,7 +246,7 @@ export function createAuthPayload( /** * Parses control messages from WebSocket data */ -export function parseControlMessage(data: Uint8Array): any { +export function parseControlMessage(data: Uint8Array): ControlMessage | null { const prefixBytes = new TextEncoder().encode(CONTROL_MESSAGE_PREFIX); // Check if this is a control message @@ -264,7 +270,9 @@ export function parseControlMessage(data: Uint8Array): any { /** * Converts various WebSocket message data types to Uint8Array */ -export async function messageDataToUint8Array(data: any): Promise { +export async function messageDataToUint8Array( + data: WebSocketMessageData, +): Promise { if (typeof data === "string") { return new TextEncoder().encode(data); } else if (data instanceof Blob) { @@ -282,12 +290,12 @@ export async function messageDataToUint8Array(data: any): Promise { * Clears the "Connecting..." message from terminal */ export function clearConnectingMessage(term: Terminal): void { - const termAny = term as any; - if (termAny._connectingLine !== undefined) { + const termExt = term as Terminal & TerminalWithConnectingState; + if (termExt._connectingLine !== undefined) { try { const currentY = term.buffer.active.cursorY; const currentX = term.buffer.active.cursorX; - const connectingLine = termAny._connectingLine; + const connectingLine = termExt._connectingLine; // Move to the connecting line and clear it term.write(`\u001B[${connectingLine + 1};1H`); // Move to line @@ -296,8 +304,8 @@ export function clearConnectingMessage(term: Terminal): void { // Move cursor back to previous position term.write(`\u001B[${currentY + 1};${currentX + 1}H`); - delete termAny._connectingLine; - delete termAny._connectingMessageLength; + delete termExt._connectingLine; + delete termExt._connectingMessageLength; } catch (error) { console.warn("Could not clear connecting message:", error); } @@ -317,8 +325,9 @@ export function showConnectingMessage( term.writeln(message); // Store line number for later clearing - (term as any)._connectingLine = cursorY; - (term as any)._connectingMessageLength = message.length; + (term as Terminal & TerminalWithConnectingState)._connectingLine = cursorY; + (term as Terminal & TerminalWithConnectingState)._connectingMessageLength = + message.length; } catch (error) { console.error(`[showConnectingMessage] Error:`, error); // If buffer is not ready, just write without tracking line number @@ -330,7 +339,10 @@ export function showConnectingMessage( * Debug logging helper */ export function debugLog(...args: unknown[]): void { - if (typeof globalThis !== "undefined" && (globalThis as any).ABLY_CLI_DEBUG) { + if ( + typeof globalThis !== "undefined" && + (globalThis as typeof globalThis & AblyCliGlobals).ABLY_CLI_DEBUG + ) { // eslint-disable-next-line no-console -- deliberate debug output gate console.log("[AblyCLITerminal DEBUG]", ...args); } diff --git a/packages/react-web-cli/src/types.ts b/packages/react-web-cli/src/types.ts new file mode 100644 index 00000000..e24946c3 --- /dev/null +++ b/packages/react-web-cli/src/types.ts @@ -0,0 +1,58 @@ +/** + * Shared type definitions for the react-web-cli package. + * Replaces `any` casts with proper types for type safety. + */ + +import type { ConnectionStatus } from "./AblyCliTerminal"; + +/** + * Control messages sent from the server over WebSocket. + */ +export interface ControlMessageHello { + type: "hello"; + sessionId: string; +} + +export interface ControlMessageStatus { + type: "status"; + payload: string; + reason?: string; + code?: number; +} + +export type ControlMessage = ControlMessageHello | ControlMessageStatus; + +/** + * Terminal instance augmented with connection tracking fields. + * xterm.js Terminal instances are extended at runtime with these properties. + */ +export interface TerminalWithConnectingState { + _connectingLine?: number; + _connectingMessageLength?: number; +} + +/** + * Global augmentation for debug flags and test helpers injected at runtime. + */ +export interface AblyCliGlobals { + ABLY_CLI_DEBUG?: boolean; + ablyCliSocket?: WebSocket; + setWindowTestFlagOnStatusChange?: (status: ConnectionStatus) => void; + getAblyCliTerminalReactState?: () => Record; + getTerminalBufferText?: () => string; + getTerminalBufferInfo?: () => Record; + /** CI auth token injected during E2E test execution */ + __ABLY_CLI_CI_AUTH_TOKEN__?: string; + __ABLY_CLI_CI_MODE__?: string; + __ABLY_CLI_TEST_GROUP__?: string; + __ABLY_CLI_RUN_ID__?: string; +} + +/** + * WebSocket message data types the browser may deliver. + */ +export type WebSocketMessageData = + | string + | Blob + | ArrayBuffer + | ArrayBufferView; From 222d7ed06df5dac1fab18f958dc791d7c721334c Mon Sep 17 00:00:00 2001 From: Andy Ford Date: Wed, 1 Apr 2026 12:27:38 +0100 Subject: [PATCH 08/15] upgrade react-web-cli to recommended-type-checked and enable no-unsafe-* as warn Switch from tsPlugin.configs.recommended to recommended-type-checked for full type-aware linting. Fix errors from additional rules (no-unnecessary-type-assertion, no-misused-promises, no-floating-promises) and remove dead code checking for stream/hijack properties on typed ControlMessage. Enable no-unsafe-* family as warnings. Disable require-await and unbound-method in test config. Co-Authored-By: Claude Opus 4.6 --- eslint.config.js | 18 +- .../react-web-cli/src/AblyCliTerminal.tsx | 307 +++++++++--------- 2 files changed, 163 insertions(+), 162 deletions(-) diff --git a/eslint.config.js b/eslint.config.js index 6290d078..17d3925e 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -145,8 +145,8 @@ export default [ // React recommended rules ...eslintPluginReact.configs.recommended.rules, ...eslintPluginReact.configs["jsx-runtime"].rules, - // TypeScript rules - ...tsPlugin.configs.recommended.rules, + // TypeScript rules (type-checked for full safety) + ...tsPlugin.configs["recommended-type-checked"].rules, // React hooks rules "react-hooks/rules-of-hooks": "error", "react-hooks/exhaustive-deps": "warn", @@ -164,12 +164,12 @@ export default [ }, ], "@typescript-eslint/no-explicit-any": "warn", - "@typescript-eslint/no-unsafe-assignment": "off", - "@typescript-eslint/no-unsafe-call": "off", - "@typescript-eslint/no-unsafe-member-access": "off", - "@typescript-eslint/no-unsafe-argument": "off", - "@typescript-eslint/no-unsafe-return": "off", - "@typescript-eslint/no-base-to-string": "off", + "@typescript-eslint/no-unsafe-assignment": "warn", + "@typescript-eslint/no-unsafe-call": "warn", + "@typescript-eslint/no-unsafe-member-access": "warn", + "@typescript-eslint/no-unsafe-argument": "warn", + "@typescript-eslint/no-unsafe-return": "warn", + "@typescript-eslint/no-base-to-string": "warn", "no-console": ["warn", { allow: ["warn", "error"] }], "no-control-regex": "off", // Terminal escape sequences use control chars "n/no-missing-import": "off", // TSX imports are handled by TypeScript @@ -223,6 +223,8 @@ export default [ ...vitest.configs.recommended.rules, "@typescript-eslint/no-explicit-any": "off", "@typescript-eslint/no-unused-expressions": "off", + "@typescript-eslint/require-await": "off", + "@typescript-eslint/unbound-method": "off", "vitest/no-focused-tests": "error", // Equivalent to mocha/no-exclusive-tests "vitest/no-disabled-tests": "warn", // Equivalent to mocha/no-skipped-tests }, diff --git a/packages/react-web-cli/src/AblyCliTerminal.tsx b/packages/react-web-cli/src/AblyCliTerminal.tsx index 189ad25e..d2b29eaf 100644 --- a/packages/react-web-cli/src/AblyCliTerminal.tsx +++ b/packages/react-web-cli/src/AblyCliTerminal.tsx @@ -1714,7 +1714,7 @@ const AblyCliTerminalInner = ( debugLog( "[AblyCLITerminal handleWebSocketError] Scheduling reconnect...", ); - grScheduleReconnect(connectWebSocketReference.current!, websocketUrl); + grScheduleReconnect(connectWebSocketReference.current, websocketUrl); } else { console.error( "[AblyCLITerminal handleWebSocketError] connectWebSocketRef.current is null, cannot schedule reconnect!", @@ -1901,7 +1901,7 @@ const AblyCliTerminalInner = ( ); if (connectWebSocketReference.current) { - grScheduleReconnect(connectWebSocketReference.current!, websocketUrl); + grScheduleReconnect(connectWebSocketReference.current, websocketUrl); } else { console.error( "[AblyCLITerminal handleWebSocketClose] connectWebSocketRef.current is null, cannot schedule reconnect!", @@ -2448,8 +2448,11 @@ const AblyCliTerminalInner = ( debugLog( "[AblyCLITerminal] New socket detected, attaching event listeners.", ); + const messageListener = (event: MessageEvent) => { + void handleWebSocketMessage(event); + }; socket.addEventListener("open", handleWebSocketOpen); - socket.addEventListener("message", handleWebSocketMessage); + socket.addEventListener("message", messageListener); socket.addEventListener("close", handleWebSocketClose); socket.addEventListener("error", handleWebSocketError); @@ -2459,7 +2462,7 @@ const AblyCliTerminalInner = ( "[AblyCLITerminal] Cleaning up WebSocket event listeners for old socket.", ); socket.removeEventListener("open", handleWebSocketOpen); - socket.removeEventListener("message", handleWebSocketMessage); + socket.removeEventListener("message", messageListener); socket.removeEventListener("close", handleWebSocketClose); socket.removeEventListener("error", handleWebSocketError); }; @@ -2677,7 +2680,7 @@ const AblyCliTerminalInner = ( ); }; - initializeSession(); + void initializeSession(); }, [signedConfig, resumeOnReload, websocketUrl]); // Store credential hash when it becomes available if we already have a sessionId @@ -2920,85 +2923,48 @@ const AblyCliTerminalInner = ( }); // WebSocket message handler with binary framing support - newSocket.addEventListener("message", async (event) => { - try { - const data = await messageDataToUint8Array(event.data); - - const message = parseControlMessage(data); - if (message) { - // Handle control messages - if ( - message.type === "hello" && - typeof message.sessionId === "string" - ) { - debugLog( - `[Secondary] Received hello. sessionId=${message.sessionId}`, - ); - const previousSessionId = secondarySessionId; - setSecondarySessionId(message.sessionId); - - // Persist to session storage if enabled - if (resumeOnReload && globalThis.window !== undefined) { - globalThis.sessionStorage.setItem( - "ably.cli.secondarySessionId", - message.sessionId, - ); - } - - // Clear the "Connecting..." message and status box - if (secondaryTerm.current) { - clearConnectingMessage(secondaryTerm.current); - secondaryTerm.current.focus(); - } - clearSecondaryStatusDisplay(); - - // Check if this is a resumed session - const isResumedSession = - previousSessionId !== null && - previousSessionId === message.sessionId; + newSocket.addEventListener("message", (event) => { + void (async () => { + try { + const data = await messageDataToUint8Array(event.data); - if (isResumedSession) { - // For resumed sessions, activate immediately without waiting for prompt + const message = parseControlMessage(data); + if (message) { + // Handle control messages + if ( + message.type === "hello" && + typeof message.sessionId === "string" + ) { debugLog( - `[Secondary] Resumed existing session ${message.sessionId} - activating immediately`, + `[Secondary] Received hello. sessionId=${message.sessionId}`, ); - updateSecondaryConnectionStatus("connected"); - - if (!isSecondarySessionActive) { - activateSecondarySessionAndSendCommand(); + const previousSessionId = secondarySessionId; + setSecondarySessionId(message.sessionId); + + // Persist to session storage if enabled + if (resumeOnReload && globalThis.window !== undefined) { + globalThis.sessionStorage.setItem( + "ably.cli.secondarySessionId", + message.sessionId, + ); } - } else { - // For new sessions, wait for prompt detection - debugLog( - `[Secondary] New session ${message.sessionId} - waiting for prompt before activating`, - ); - updateSecondaryConnectionStatus("connected"); - } - - return; - } - if (message.type === "status") { - debugLog( - `[Secondary] Received server status message: ${message.payload}`, - ); - - if (message.payload === "connected") { - // Clear any overlay when connected - clearSecondaryStatusDisplay(); - // Clear the "Connecting..." message + // Clear the "Connecting..." message and status box if (secondaryTerm.current) { clearConnectingMessage(secondaryTerm.current); secondaryTerm.current.focus(); } + clearSecondaryStatusDisplay(); - // Check if we're resuming an existing session - const isResuming = secondarySessionId !== null; + // Check if this is a resumed session + const isResumedSession = + previousSessionId !== null && + previousSessionId === message.sessionId; - if (isResuming) { - // For resumed sessions, activate immediately + if (isResumedSession) { + // For resumed sessions, activate immediately without waiting for prompt debugLog( - `[Secondary] Resuming session ${secondarySessionId} - activating immediately`, + `[Secondary] Resumed existing session ${message.sessionId} - activating immediately`, ); updateSecondaryConnectionStatus("connected"); @@ -3008,113 +2974,146 @@ const AblyCliTerminalInner = ( } else { // For new sessions, wait for prompt detection debugLog( - `[Secondary] New session - waiting for prompt before activating`, + `[Secondary] New session ${message.sessionId} - waiting for prompt before activating`, ); updateSecondaryConnectionStatus("connected"); - - // Start timeout fallback in case prompt is never detected - clearSecondaryPromptDetectionTimeout(); - secondaryPromptDetectionTimeoutReference.current = setTimeout( - () => { - debugLog( - `[Secondary] Prompt detection timeout - activating session anyway after ${PROMPT_DETECTION_TIMEOUT_MS}ms`, - ); - if (!isSecondarySessionActive) { - activateSecondarySessionAndSendCommand(); - } - }, - PROMPT_DETECTION_TIMEOUT_MS, - ); } return; } - - // Handle error & disconnected - if ( - message.payload === "error" || - message.payload === "disconnected" - ) { - const reason = - message.reason || - (message.payload === "error" - ? "Server error" - : "Server disconnected"); - if (secondaryTerm.current) - secondaryTerm.current.writeln( - `\r\n--- ${message.payload === "error" ? "Error" : "Session Ended (from server)"}: ${reason} ---`, - ); - updateSecondaryConnectionStatus( - message.payload as ConnectionStatus, + if (message.type === "status") { + debugLog( + `[Secondary] Received server status message: ${message.payload}`, ); - if (secondaryTerm.current && message.payload === "disconnected") { - const title = "ERROR: SERVER DISCONNECT"; - const message1 = `Connection closed by server (${message.code})${message.reason ? `: ${message.reason}` : ""}.`; - const message2 = ""; - const message3 = `Press ⏎ to try reconnecting manually.`; + if (message.payload === "connected") { + // Clear any overlay when connected + clearSecondaryStatusDisplay(); + // Clear the "Connecting..." message if (secondaryTerm.current) { - secondaryStatusBoxReference.current = drawBox( - secondaryTerm.current, - boxColour.red, - title, - [message1, message2, message3], - 60, + clearConnectingMessage(secondaryTerm.current); + secondaryTerm.current.focus(); + } + + // Check if we're resuming an existing session + const isResuming = secondarySessionId !== null; + + if (isResuming) { + // For resumed sessions, activate immediately + debugLog( + `[Secondary] Resuming session ${secondarySessionId} - activating immediately`, + ); + updateSecondaryConnectionStatus("connected"); + + if (!isSecondarySessionActive) { + activateSecondarySessionAndSendCommand(); + } + } else { + // For new sessions, wait for prompt detection + debugLog( + `[Secondary] New session - waiting for prompt before activating`, + ); + updateSecondaryConnectionStatus("connected"); + + // Start timeout fallback in case prompt is never detected + clearSecondaryPromptDetectionTimeout(); + secondaryPromptDetectionTimeoutReference.current = setTimeout( + () => { + debugLog( + `[Secondary] Prompt detection timeout - activating session anyway after ${PROMPT_DETECTION_TIMEOUT_MS}ms`, + ); + if (!isSecondarySessionActive) { + activateSecondarySessionAndSendCommand(); + } + }, + PROMPT_DETECTION_TIMEOUT_MS, ); - setSecondaryOverlay({ - variant: "error", - title, - lines: [message1, message2, message3], - }); } - secondaryShowManualReconnectPromptReference.current = true; - setSecondaryShowManualReconnectPrompt(true); + return; + } + + // Handle error & disconnected + if ( + message.payload === "error" || + message.payload === "disconnected" + ) { + const reason = + message.reason || + (message.payload === "error" + ? "Server error" + : "Server disconnected"); + if (secondaryTerm.current) + secondaryTerm.current.writeln( + `\r\n--- ${message.payload === "error" ? "Error" : "Session Ended (from server)"}: ${reason} ---`, + ); + updateSecondaryConnectionStatus( + message.payload as ConnectionStatus, + ); + + if ( + secondaryTerm.current && + message.payload === "disconnected" + ) { + const title = "ERROR: SERVER DISCONNECT"; + const message1 = `Connection closed by server (${message.code})${message.reason ? `: ${message.reason}` : ""}.`; + const message2 = ""; + const message3 = `Press ⏎ to try reconnecting manually.`; + + if (secondaryTerm.current) { + secondaryStatusBoxReference.current = drawBox( + secondaryTerm.current, + boxColour.red, + title, + [message1, message2, message3], + 60, + ); + setSecondaryOverlay({ + variant: "error", + title, + lines: [message1, message2, message3], + }); + } + + secondaryShowManualReconnectPromptReference.current = true; + setSecondaryShowManualReconnectPrompt(true); + } + return; } return; } - return; - } - // Check for PTY stream/hijack meta-message - if (message.stream === true && typeof message.hijack === "boolean") { - debugLog( - "[AblyCLITerminal] [Secondary] Received PTY meta-message. Ignoring.", - ); return; } - // Control message was handled, return - return; - } + // Not a control message - process as PTY data + const dataString = new TextDecoder().decode(data); - // Not a control message - process as PTY data - const dataString = new TextDecoder().decode(data); + // Filter Docker handshake JSON using the shared buffering logic + const filteredData = filterDockerHandshake( + dataString, + secondaryHandshakeFilterState.current, + ); - // Filter Docker handshake JSON using the shared buffering logic - const filteredData = filterDockerHandshake( - dataString, - secondaryHandshakeFilterState.current, - ); + // Only write if we have data after filtering + if (filteredData.length > 0) { + // Always write PTY data to terminal for the secondary terminal + // This ensures character echo works properly + if (secondaryTerm.current) { + secondaryTerm.current.write(filteredData); + } - // Only write if we have data after filtering - if (filteredData.length > 0) { - // Always write PTY data to terminal for the secondary terminal - // This ensures character echo works properly - if (secondaryTerm.current) { - secondaryTerm.current.write(filteredData); + // Call handleSecondaryPtyData for prompt detection + handleSecondaryPtyData(filteredData); } - - // Call handleSecondaryPtyData for prompt detection - handleSecondaryPtyData(filteredData); + } catch (error) { + console.error( + "[AblyCLITerminal] [Secondary] Error processing message:", + error, + ); } - } catch (error) { - console.error( - "[AblyCLITerminal] [Secondary] Error processing message:", - error, - ); - } + })(); }); // WebSocket error handler From c2dd9fc2e09ded85a81815ddb6f33a28cb4ac516 Mon Sep 17 00:00:00 2001 From: Andy Ford Date: Wed, 1 Apr 2026 12:46:38 +0100 Subject: [PATCH 09/15] fix no-unsafe-* violations in react-web-cli source files Add type assertions for JSON.parse calls (signedConfig parsing, control message parsing) and cast MessageEvent.data to WebSocketMessageData. Remove dead code checking for stream/hijack on typed ControlMessage. Source files now have zero no-unsafe-* warnings. Co-Authored-By: Claude Opus 4.6 --- .../react-web-cli/src/AblyCliTerminal.tsx | 21 ++++++++++++++----- packages/react-web-cli/src/terminal-shared.ts | 7 +++++-- 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/packages/react-web-cli/src/AblyCliTerminal.tsx b/packages/react-web-cli/src/AblyCliTerminal.tsx index d2b29eaf..b1ad6ad9 100644 --- a/packages/react-web-cli/src/AblyCliTerminal.tsx +++ b/packages/react-web-cli/src/AblyCliTerminal.tsx @@ -34,7 +34,11 @@ import { useTerminalVisibility } from "./use-terminal-visibility.js"; import { SplitSquareHorizontal, X } from "lucide-react"; import { hashCredentials } from "./utils/crypto"; import { getConnectionMessage } from "./connection-messages"; -import type { ControlMessage, AblyCliGlobals } from "./types"; +import type { + ControlMessage, + AblyCliGlobals, + WebSocketMessageData, +} from "./types"; import { HandshakeFilterState, createHandshakeFilterState, @@ -1502,7 +1506,9 @@ const AblyCliTerminalInner = ( `⚠️ DIAGNOSTIC: handleWebSocketMessage called with event.data type: ${typeof event.data}`, ); try { - const data = await messageDataToUint8Array(event.data); + const data = await messageDataToUint8Array( + event.data as WebSocketMessageData, + ); const message = parseControlMessage(data); if (message) { @@ -1564,7 +1570,7 @@ const AblyCliTerminalInner = ( try { const jsonString = ctrlLine.slice("ABLY_CTRL:".length); - const message_ = JSON.parse(jsonString); + const message_ = JSON.parse(jsonString) as ControlMessage; debugLog( `⚠️ DIAGNOSTIC: Found text-based control message:`, message_, @@ -2608,7 +2614,10 @@ const AblyCliTerminalInner = ( let apiKeyForHash: string | undefined; let accessTokenForHash: string | undefined; try { - const parsedConfig = JSON.parse(signedConfig); + const parsedConfig = JSON.parse(signedConfig) as { + apiKey?: string; + accessToken?: string; + }; apiKeyForHash = parsedConfig.apiKey; accessTokenForHash = parsedConfig.accessToken; } catch { @@ -2926,7 +2935,9 @@ const AblyCliTerminalInner = ( newSocket.addEventListener("message", (event) => { void (async () => { try { - const data = await messageDataToUint8Array(event.data); + const data = await messageDataToUint8Array( + event.data as WebSocketMessageData, + ); const message = parseControlMessage(data); if (message) { diff --git a/packages/react-web-cli/src/terminal-shared.ts b/packages/react-web-cli/src/terminal-shared.ts index 2b52d89a..98ed2b8d 100644 --- a/packages/react-web-cli/src/terminal-shared.ts +++ b/packages/react-web-cli/src/terminal-shared.ts @@ -202,7 +202,10 @@ export function createAuthPayload( // Extract fields from signed config for server convenience // Server uses these to set environment variables like ABLY_ANONYMOUS_USER_MODE try { - const parsedConfig = JSON.parse(signedConfig); + const parsedConfig = JSON.parse(signedConfig) as { + apiKey?: string; + accessToken?: string; + }; if (parsedConfig.apiKey) { payload.apiKey = parsedConfig.apiKey; } @@ -260,7 +263,7 @@ export function parseControlMessage(data: Uint8Array): ControlMessage | null { try { const jsonBytes = data.slice(prefixBytes.length); const jsonStr = new TextDecoder().decode(jsonBytes); - return JSON.parse(jsonStr); + return JSON.parse(jsonStr) as ControlMessage; } catch (error) { console.error("Failed to parse control message:", error); return null; From 2c5f8181c3821fa6d80199c2adb596eba5a73e97 Mon Sep 17 00:00:00 2001 From: Andy Ford Date: Wed, 1 Apr 2026 12:56:50 +0100 Subject: [PATCH 10/15] promote react-web-cli type safety rules from warn to error All no-unsafe-*, no-explicit-any, no-base-to-string, and no-console rules now enforced as errors in source files. Test files disable these rules since tests legitimately use `any` for mocking. Only react-hooks/exhaustive-deps remains as warn (non-trivial to fix without risking behavior changes). Co-Authored-By: Claude Opus 4.6 --- eslint.config.js | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/eslint.config.js b/eslint.config.js index 17d3925e..e657abfd 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -163,14 +163,14 @@ export default [ caughtErrorsIgnorePattern: "^_", }, ], - "@typescript-eslint/no-explicit-any": "warn", - "@typescript-eslint/no-unsafe-assignment": "warn", - "@typescript-eslint/no-unsafe-call": "warn", - "@typescript-eslint/no-unsafe-member-access": "warn", - "@typescript-eslint/no-unsafe-argument": "warn", - "@typescript-eslint/no-unsafe-return": "warn", - "@typescript-eslint/no-base-to-string": "warn", - "no-console": ["warn", { allow: ["warn", "error"] }], + "@typescript-eslint/no-explicit-any": "error", + "@typescript-eslint/no-unsafe-assignment": "error", + "@typescript-eslint/no-unsafe-call": "error", + "@typescript-eslint/no-unsafe-member-access": "error", + "@typescript-eslint/no-unsafe-argument": "error", + "@typescript-eslint/no-unsafe-return": "error", + "@typescript-eslint/no-base-to-string": "error", + "no-console": ["error", { allow: ["warn", "error"] }], "no-control-regex": "off", // Terminal escape sequences use control chars "n/no-missing-import": "off", // TSX imports are handled by TypeScript "react/prop-types": "off", // Using TypeScript for prop validation @@ -225,6 +225,14 @@ export default [ "@typescript-eslint/no-unused-expressions": "off", "@typescript-eslint/require-await": "off", "@typescript-eslint/unbound-method": "off", + // Tests legitimately use `any` for mocking + "@typescript-eslint/no-unsafe-assignment": "off", + "@typescript-eslint/no-unsafe-call": "off", + "@typescript-eslint/no-unsafe-member-access": "off", + "@typescript-eslint/no-unsafe-argument": "off", + "@typescript-eslint/no-unsafe-return": "off", + "@typescript-eslint/no-base-to-string": "off", + "no-console": "off", "vitest/no-focused-tests": "error", // Equivalent to mocha/no-exclusive-tests "vitest/no-disabled-tests": "warn", // Equivalent to mocha/no-skipped-tests }, From db33a3c087bca35ecbf5b4f696d466dd6daacd06 Mon Sep 17 00:00:00 2001 From: Andy Ford Date: Wed, 1 Apr 2026 13:40:00 +0100 Subject: [PATCH 11/15] enable strict TypeScript rules and consistent-type-imports for react-web-cli Cherry-pick no-deprecated (warn), no-useless-constructor, unified-signatures, return-await, and consistent-type-imports (all error) from the main CLI config. Auto-fix converts 5 value imports to type imports across AblyCliTerminal.tsx and use-terminal-visibility.ts. Co-Authored-By: Claude Opus 4.6 --- eslint.config.js | 5 +++++ packages/react-web-cli/src/AblyCliTerminal.tsx | 9 +++++---- packages/react-web-cli/src/use-terminal-visibility.ts | 3 ++- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/eslint.config.js b/eslint.config.js index e657abfd..561dbe29 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -170,6 +170,11 @@ export default [ "@typescript-eslint/no-unsafe-argument": "error", "@typescript-eslint/no-unsafe-return": "error", "@typescript-eslint/no-base-to-string": "error", + "@typescript-eslint/no-deprecated": "warn", + "@typescript-eslint/no-useless-constructor": "error", + "@typescript-eslint/unified-signatures": "error", + "@typescript-eslint/return-await": "error", + "@typescript-eslint/consistent-type-imports": "error", "no-console": ["error", { allow: ["warn", "error"] }], "no-control-regex": "off", // Terminal escape sequences use control chars "n/no-missing-import": "off", // TSX imports are handled by TypeScript diff --git a/packages/react-web-cli/src/AblyCliTerminal.tsx b/packages/react-web-cli/src/AblyCliTerminal.tsx index b1ad6ad9..7c65110f 100644 --- a/packages/react-web-cli/src/AblyCliTerminal.tsx +++ b/packages/react-web-cli/src/AblyCliTerminal.tsx @@ -6,10 +6,11 @@ import React, { useImperativeHandle, forwardRef, } from "react"; -import { Terminal } from "@xterm/xterm"; -import { FitAddon } from "@xterm/addon-fit"; +import type { Terminal } from "@xterm/xterm"; +import type { FitAddon } from "@xterm/addon-fit"; import "@xterm/xterm/css/xterm.css"; -import TerminalOverlay, { OverlayVariant } from "./TerminalOverlay"; +import type { OverlayVariant } from "./TerminalOverlay"; +import TerminalOverlay from "./TerminalOverlay"; import { drawBox, clearBox, @@ -39,8 +40,8 @@ import type { AblyCliGlobals, WebSocketMessageData, } from "./types"; +import type { HandshakeFilterState } from "./terminal-shared"; import { - HandshakeFilterState, createHandshakeFilterState, filterDockerHandshake, createTerminal, diff --git a/packages/react-web-cli/src/use-terminal-visibility.ts b/packages/react-web-cli/src/use-terminal-visibility.ts index 47aeef0e..7b6b94dd 100644 --- a/packages/react-web-cli/src/use-terminal-visibility.ts +++ b/packages/react-web-cli/src/use-terminal-visibility.ts @@ -1,4 +1,5 @@ -import React, { useEffect, useState, useRef } from "react"; +import type React from "react"; +import { useEffect, useState, useRef } from "react"; /** * useTerminalVisibility detects whether the referenced element is currently visible to the user. From f3bf517596ee5abb7b9ef30f16d826e1a310bcfb Mon Sep 17 00:00:00 2001 From: Andy Ford Date: Wed, 1 Apr 2026 13:49:30 +0100 Subject: [PATCH 12/15] fix safe exhaustive-deps warnings in react-web-cli MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - handleCloseSplit: suppress — deps are stable but defined later (hoisting) - handleCloseSecondary: suppress — same hoisting constraint - handlePtyData: remove unused deps (clearInstallInstructionsTimer, updateConnectionStatusAndExpose, updateSessionActive) - handleControlMessage: add activateSessionAndSendCommand and clearPromptDetectionTimeout; remove unused clearPtyBuffer, updateSessionActive - activateSecondarySessionAndSendCommand: suppress hoisting constraint - startConnectingAnimation: move spinnerFrames to module scope - handleWebSocketOpen: remove unused deps (clearAnimationMessages, credentialHash, resumeOnReload, updateConnectionStatusAndExpose) - termCleanupReference: capture .current in local var before cleanup - useEffect (initial connection): add resumeOnReload - startInactivityTimer: add clearInactivityTimer, remove module-scope deps Co-Authored-By: Claude Opus 4.6 --- .../react-web-cli/src/AblyCliTerminal.tsx | 34 +++++++++---------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/packages/react-web-cli/src/AblyCliTerminal.tsx b/packages/react-web-cli/src/AblyCliTerminal.tsx index 7c65110f..2aa1ae0c 100644 --- a/packages/react-web-cli/src/AblyCliTerminal.tsx +++ b/packages/react-web-cli/src/AblyCliTerminal.tsx @@ -161,6 +161,10 @@ if (globalThis.window !== undefined) { } } +// Static spinner frames — hoisted to module scope so the array identity is +// stable between renders and doesn't trigger exhaustive-deps warnings. +const spinnerFrames = ["● ", " ● ", " ●", " ● "]; + const AblyCliTerminalInner = ( { websocketUrl, @@ -297,6 +301,7 @@ const AblyCliTerminalInner = ( } } }, 50); + // eslint-disable-next-line react-hooks/exhaustive-deps -- resumeOnReload is a stable prop; updateSecondaryConnectionStatus is defined later in the component (hoisting constraint) but has [] deps }, []); // Imperative handle for external control of split operations @@ -496,6 +501,7 @@ const AblyCliTerminalInner = ( } } }, 50); + // eslint-disable-next-line react-hooks/exhaustive-deps -- updateSecondaryConnectionStatus has [] deps but is defined later (hoisting constraint) }, [resumeOnReload]); // Track the current sessionId received from the server (if any) @@ -547,8 +553,6 @@ const AblyCliTerminalInner = ( // Keep a ref for sessionId to use in closures const sessionIdReference = useRef(sessionId); - // Use block-based spinner where empty dots are invisible in most monospace fonts - const spinnerFrames = ["● ", " ● ", " ●", " ● "]; const spinnerIntervalReference = useRef | null>(null); @@ -793,11 +797,8 @@ const AblyCliTerminalInner = ( } }, [ - updateConnectionStatusAndExpose, - updateSessionActive, clearPtyBuffer, clearStatusDisplay, - clearInstallInstructionsTimer, activateSessionAndSendCommand, ], ); @@ -1034,14 +1035,13 @@ const AblyCliTerminalInner = ( sessionId, onSessionId, clearStatusDisplay, - updateSessionActive, updateConnectionStatusAndExpose, - clearPtyBuffer, resumeOnReload, websocketUrl, credentialHash, onSessionEnd, - grSuccessfulConnectionReset, + activateSessionAndSendCommand, + clearPromptDetectionTimeout, ], ); @@ -1097,6 +1097,7 @@ const AblyCliTerminalInner = ( } }, 50); } + // eslint-disable-next-line react-hooks/exhaustive-deps -- updateSecondaryConnectionStatus has [] deps but is defined later (hoisting constraint) }, [clearSecondaryPromptDetectionTimeout]); const handleSecondaryPtyData = useCallback( @@ -1489,14 +1490,10 @@ const AblyCliTerminalInner = ( // persistence handled by dedicated useEffect debugLog("WebSocket OPEN handler completed. sessionId:", sessionId); }, [ - clearAnimationMessages, initialCommand, - updateConnectionStatusAndExpose, clearPtyBuffer, sessionId, - resumeOnReload, clearConnectionTimeout, - credentialHash, signedConfig, signature, ]); @@ -2338,11 +2335,15 @@ const AblyCliTerminalInner = ( // Initial connection on mount - do NOT connect here, let the dedicated connection effects handle it // This prevents duplicate connections from multiple effects running in the same render cycle + // Capture ref value for cleanup — React warns if we read .current + // inside the cleanup function because it may have changed by then. + const termCleanup = termCleanupReference.current; + // Cleanup terminal on unmount return () => { // Execute resize listener cleanup if it exists - if (termCleanupReference.current) { - termCleanupReference.current(); + if (termCleanup) { + termCleanup(); } if (term.current) { @@ -2387,7 +2388,7 @@ const AblyCliTerminalInner = ( componentConnectionStatus, clearPtyBuffer, connectWebSocket, - // resumeOnReload is intentionally omitted - it's a prop that doesn't change during component lifecycle + resumeOnReload, ]); useEffect(() => { @@ -2579,8 +2580,7 @@ const AblyCliTerminalInner = ( }, INACTIVITY_TIMEOUT_MS); }, [ INACTIVITY_TIMEOUT_MS, - grCancelReconnect, - grResetState, + clearInactivityTimer, updateConnectionStatusAndExpose, ]); From a9133a183031389ce7046eb95fdf2d96093fb1c6 Mon Sep 17 00:00:00 2001 From: Andy Ford Date: Wed, 1 Apr 2026 13:54:48 +0100 Subject: [PATCH 13/15] fix exhaustive-deps warnings using ref pattern for unstable deps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - handleClosePrimary: suppress — deps defined later in component (hoisting) - connectWebSocket: use credentialsInitializedRef for debug-only state read; remove unnecessary ref dep (term) - handleWebSocketMessage: use handleControlMessageRef to avoid deep dep chain through sessionId state; remove all stale deps from array - connectSecondaryWebSocket: suppress — already accessed via ref; missing deps include hoisted callbacks and state causing cascading recreations Co-Authored-By: Claude Opus 4.6 --- .../react-web-cli/src/AblyCliTerminal.tsx | 23 ++++++++----------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/packages/react-web-cli/src/AblyCliTerminal.tsx b/packages/react-web-cli/src/AblyCliTerminal.tsx index 2aa1ae0c..10227fef 100644 --- a/packages/react-web-cli/src/AblyCliTerminal.tsx +++ b/packages/react-web-cli/src/AblyCliTerminal.tsx @@ -453,6 +453,7 @@ const AblyCliTerminalInner = ( // If there's no secondary terminal, just close everything (same as handleCloseSplit) handleCloseSplit(); } + // eslint-disable-next-line react-hooks/exhaustive-deps -- isSecondarySessionActive, secondarySessionId, updateSessionActive, updateSecondaryConnectionStatus are all defined later in the component (hoisting constraint); values are current at call time via closures }, [handleCloseSplit, resumeOnReload]); /** Close the secondary pane and return to single-pane mode */ @@ -508,6 +509,8 @@ const AblyCliTerminalInner = ( const [sessionId, setSessionId] = useState(null); const [credentialHash, setCredentialHash] = useState(null); const [credentialsInitialized, setCredentialsInitialized] = useState(false); + const credentialsInitializedRef = useRef(false); + credentialsInitializedRef.current = credentialsInitialized; // Track the second terminal's sessionId const [secondarySessionId, setSecondarySessionId] = useState( @@ -1044,6 +1047,8 @@ const AblyCliTerminalInner = ( clearPromptDetectionTimeout, ], ); + const handleControlMessageRef = useRef(handleControlMessage); + handleControlMessageRef.current = handleControlMessage; // Function to clear the secondary terminal overlay and status displays const clearSecondaryStatusDisplay = useCallback(() => { @@ -1274,7 +1279,7 @@ const AblyCliTerminalInner = ( "⚠️ DIAGNOSTIC: connectWebSocket called - start of connection process", ); debugLog( - `⚠️ DIAGNOSTIC: Current sessionId: ${sessionId}, credentialsInitialized: ${credentialsInitialized}`, + `⚠️ DIAGNOSTIC: Current sessionId: ${sessionId}, credentialsInitialized: ${credentialsInitializedRef.current}`, ); // Skip attempt if terminal not visible to avoid unnecessary server load @@ -1411,7 +1416,6 @@ const AblyCliTerminalInner = ( clearConnectionTimeout, connectionStartTime, clearInstallInstructionsTimer, - term, ]); const socketReference = useRef(null); // Ref to hold the current socket for cleanup @@ -1513,7 +1517,7 @@ const AblyCliTerminalInner = ( // Handle control messages // Handle control messages using the extracted handler - handleControlMessage(message); + handleControlMessageRef.current(message); return; } @@ -1575,7 +1579,7 @@ const AblyCliTerminalInner = ( ); // Process the control message using the extracted handler - handleControlMessage(message_); + handleControlMessageRef.current(message_); } catch (error) { console.warn( "[WebSocket] Failed to parse text-based control message:", @@ -1625,15 +1629,7 @@ const AblyCliTerminalInner = ( console.error("[AblyCLITerminal] Error processing message:", error); } }, - [ - handlePtyData, - onSessionEnd, - updateConnectionStatusAndExpose, - updateSessionActive, - credentialHash, - resumeOnReload, - sessionId, - ], + [handlePtyData], ); const handleWebSocketError = useCallback( @@ -3219,6 +3215,7 @@ const AblyCliTerminalInner = ( }); return newSocket; + // eslint-disable-next-line react-hooks/exhaustive-deps -- callback is accessed via connectSecondaryWebSocketReference ref; missing deps include hoisted callbacks and state that would cause cascading recreations }, [ websocketUrl, signedConfig, From 068d9bf660e3ff9742c4b2527424de22dced4ff1 Mon Sep 17 00:00:00 2001 From: Andy Ford Date: Wed, 1 Apr 2026 13:57:00 +0100 Subject: [PATCH 14/15] suppress exhaustive-deps for mount-only terminal initialization effects MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two useEffects intentionally use minimal dep arrays to avoid destroying and recreating xterm Terminal instances on state changes: - Primary terminal init (empty []) — 12 missing deps - Secondary terminal init ([isSplit]) — 4 missing deps Co-Authored-By: Claude Opus 4.6 --- packages/react-web-cli/src/AblyCliTerminal.tsx | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/packages/react-web-cli/src/AblyCliTerminal.tsx b/packages/react-web-cli/src/AblyCliTerminal.tsx index 10227fef..055813f9 100644 --- a/packages/react-web-cli/src/AblyCliTerminal.tsx +++ b/packages/react-web-cli/src/AblyCliTerminal.tsx @@ -799,11 +799,7 @@ const AblyCliTerminalInner = ( } } }, - [ - clearPtyBuffer, - clearStatusDisplay, - activateSessionAndSendCommand, - ], + [clearPtyBuffer, clearStatusDisplay, activateSessionAndSendCommand], ); // Secondary terminal instance references @@ -2359,7 +2355,8 @@ const AblyCliTerminalInner = ( grResetState(); // Ensure global state is clean clearConnectionTimeout(); // Clear any pending connection timeout }; - }, []); // Empty deps - this effect only runs once on mount to initialize the terminal + // eslint-disable-next-line react-hooks/exhaustive-deps -- mount-only initialization; adding deps would destroy/recreate the terminal on every state change + }, []); // Single unified effect for initial connection (handles both mount and visibility changes) // When resumeOnReload is enabled, waits for credentialsInitialized to ensure sessionId restoration completes first @@ -3526,7 +3523,8 @@ const AblyCliTerminalInner = ( setSecondarySessionId(null); setSecondaryOverlay(null); }; - }, [isSplit]); // Only re-run when split mode changes, not when connectSecondaryWebSocket changes + // eslint-disable-next-line react-hooks/exhaustive-deps -- secondary terminal init; adding state deps would re-init mid-session + }, [isSplit]); // Persist secondary sessionId to localStorage whenever it changes (if enabled) useEffect(() => { From 41791ac243740c66388d6ee310aba3f89fab76a0 Mon Sep 17 00:00:00 2001 From: Andy Ford Date: Wed, 1 Apr 2026 13:57:15 +0100 Subject: [PATCH 15/15] promote react-hooks/exhaustive-deps to error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All 16 warnings have been resolved — either by fixing deps, using ref patterns, or suppressing with documented rationale. Promote to error to prevent regressions. Co-Authored-By: Claude Opus 4.6 --- eslint.config.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/eslint.config.js b/eslint.config.js index 561dbe29..92e894da 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -149,7 +149,7 @@ export default [ ...tsPlugin.configs["recommended-type-checked"].rules, // React hooks rules "react-hooks/rules-of-hooks": "error", - "react-hooks/exhaustive-deps": "warn", + "react-hooks/exhaustive-deps": "error", // Custom overrides for this package "unicorn/prefer-module": "off", "unicorn/filename-case": "off",