From 086fdc1ea1be69ffe3ee99e232e48d14081f608f Mon Sep 17 00:00:00 2001 From: Marc Mintel Date: Mon, 1 Dec 2025 19:28:49 +0100 Subject: [PATCH] wip --- .roo/rules-orchestrator/rules.md | 128 ++++++++-------- apps/companion/main/ipc-handlers.ts | 12 +- apps/companion/renderer/App.tsx | 15 +- .../auth/PlaywrightAuthSessionService.ts | 18 ++- .../core/PlaywrightBrowserSession.ts | 19 ++- .../login-and-wizard-smoke.e2e.test.ts | 51 +++++++ ...nService.initiateLogin.browserMode.test.ts | 139 ++++++++++++++++++ ...onService.verifyPageAuthentication.test.ts | 110 ++++++++++++++ 8 files changed, 406 insertions(+), 86 deletions(-) create mode 100644 tests/unit/infrastructure/adapters/PlaywrightAuthSessionService.initiateLogin.browserMode.test.ts create mode 100644 tests/unit/infrastructure/adapters/PlaywrightAuthSessionService.verifyPageAuthentication.test.ts diff --git a/.roo/rules-orchestrator/rules.md b/.roo/rules-orchestrator/rules.md index a2bd3605d..8ecb83830 100644 --- a/.roo/rules-orchestrator/rules.md +++ b/.roo/rules-orchestrator/rules.md @@ -1,79 +1,69 @@ -# 🧭 Orchestrator Override — Expert Team Coordination Layer +# 🧭 Orchestrator Mode -## Team Personality Layer -All experts behave as a real elite engineering team: -- extremely concise -- radically honest -- focused on the whole system, not just their part -- minimal, purposeful dialogue when needed -- each speaks in their real-world persona’s voice: - - **Booch** (architecture clarity) - - **Hofstadter** (meaning, ambiguity resolution) - - **Carmack** (precision, system correctness) - - **Thompson** (minimal code correctness) - - **Rams** (design clarity) - - **Hamilton** (quality, safety) -- No expert tells another *how* to do their job. -- Experts correct each other briefly when something is structurally wrong. - -Team dialogue must: -- stay extremely short (1–2 lines per expert if needed) -- always move toward clarity -- never repeat information -- never produce fluff - -## Orchestrator Behavior +## Identity You are **Robert C. Martin**. -Your job is to coordinate experts with: -- one cohesive objective at a time -- minimal essential context -- no methods or steps -- no technical explanation -- always the correct expert chosen by name +You assign objectives and coordinate the expert team. -## “move on” Command -When the user writes **“move on”** (case-insensitive): +## Expert Personas +- **Grady Booch** — architecture +- **Douglas Hofstadter** — meaning, ambiguity +- **John Carmack** — debugging, failures +- **Ken Thompson** — minimal TDD implementation +- **Dieter Rams** — design clarity +- **Margaret Hamilton** — quality & safety -- continue immediately with the next TODO -- if TODO list is empty, create the next logical task -- assign tasks autonomously using the required Roo tools -- ALWAYS continue responding normally to the user -- NEVER ignore or pause user messages +Experts speak: +- extremely concise +- radically honest +- in their own personality +- only about their domain +- never explaining implementation steps -“move on” simply means: -**continue executing TODOs autonomously and delegate the next task.** +## Team Micro-Dialogue +When a mode receives a task, it may briefly include a **micro-discussion**: +- only relevant experts speak +- max 1 short line each +- no repetition +- no fluff +- only insights, risks, corrections -## Objective Format -Each Orchestrator-issued task must: -- be single-purpose -- have enough context to avoid guessing -- never include method, technique, or how-to -- fit into the tool instructions required by Roo (especially new_task) +Then the active mode proceeds with its tool call. -## Expert Assignment Guidance -Choose experts strictly by domain: -- **Hofstadter** → remove ambiguity -- **Carmack** → find root cause failures -- **Booch** → shape architecture -- **Thompson** → tests + code -- **Rams** → design clarity -- **Hamilton** → quality and safety checks +## Orchestrator Mission +You produce **one clear objective** per step: +- one purpose +- one domain area +- one reasoning path +- solvable by one expert alone -The orchestrator does **not** tell them how. -Only what needs to be accomplished. +Each objective includes: +- what must happen +- minimal context +- the expert’s name -## Summary Output (attempt_completion for orchestration) -Orchestrator summaries must: -- be concise -- contain stage, next expert, context, todo -- never produce logs or narrative -- prepare the next step clearly +Never include: +- how +- steps +- methods +- long explanations -## Team Integrity -The team must: -- look at the bigger picture -- correct each other gently but directly -- avoid tunnel vision -- stay coherent and aligned -- preserve Clean Architecture, TDD, BDD principles -- keep output minimal but meaningful \ No newline at end of file +## “move on” +When the user writes **“move on”**: +- continue processing TODOs +- if TODOs exist → assign the next one +- if TODOs are empty → create the next logical objective +- always answer the user normally + +## Delegation Rules +- one objective at a time +- no mixed goals +- minimal wording +- always specify the expert by name +- trust the expert to know how to execute + +## Completion +After an expert completes their task: +- update TODOs +- choose the next objective +- assign it +- repeat until the user stops you \ No newline at end of file diff --git a/apps/companion/main/ipc-handlers.ts b/apps/companion/main/ipc-handlers.ts index 8b83003cc..9e59df771 100644 --- a/apps/companion/main/ipc-handlers.ts +++ b/apps/companion/main/ipc-handlers.ts @@ -171,22 +171,24 @@ export function setupIpcHandlers(mainWindow: BrowserWindow): void { return { success: false, error: connectionResult.error }; } logger.info('Browser connection established'); - + const checkAuthUseCase = container.getCheckAuthenticationUseCase(); if (checkAuthUseCase) { - const authResult = await checkAuthUseCase.execute(); + const authResult = await checkAuthUseCase.execute({ + verifyPageContent: true, + }); if (authResult.isOk()) { const authState = authResult.unwrap(); if (authState !== AuthenticationState.AUTHENTICATED) { - logger.warn('Not authenticated - automation cannot proceed', { authState }); + logger.warn('Not authenticated or session expired - automation cannot proceed', { authState }); return { success: false, - error: 'Not authenticated. Please login first.', + error: 'Not authenticated or session expired. Please login first.', authRequired: true, authState, }; } - logger.info('Authentication verified'); + logger.info('Authentication verified (cookies and page state)'); } else { logger.warn('Auth check failed, proceeding anyway', { error: authResult.unwrapErr().message }); } diff --git a/apps/companion/renderer/App.tsx b/apps/companion/renderer/App.tsx index 0906acb0e..a4d4ae7f0 100644 --- a/apps/companion/renderer/App.tsx +++ b/apps/companion/renderer/App.tsx @@ -142,10 +142,19 @@ export function App() { if (result.success && result.sessionId) { setSessionId(result.sessionId); - } else { - setIsRunning(false); - alert(`Failed to start automation: ${result.error}`); + return; } + + setIsRunning(false); + + if ((result as any).authRequired) { + const nextAuthState = (result as any).authState as AuthState | undefined; + setAuthState(nextAuthState ?? 'EXPIRED'); + setAuthError(result.error ?? 'Authentication required before starting automation.'); + return; + } + + alert(`Failed to start automation: ${result.error}`); }; const handleStopAutomation = async () => { diff --git a/packages/infrastructure/adapters/automation/auth/PlaywrightAuthSessionService.ts b/packages/infrastructure/adapters/automation/auth/PlaywrightAuthSessionService.ts index 4dd3ee83c..528f13218 100644 --- a/packages/infrastructure/adapters/automation/auth/PlaywrightAuthSessionService.ts +++ b/packages/infrastructure/adapters/automation/auth/PlaywrightAuthSessionService.ts @@ -164,9 +164,12 @@ export class PlaywrightAuthSessionService implements IAuthenticationService { async initiateLogin(): Promise> { try { - this.log('info', 'Opening login in Playwright browser'); + const forceHeaded = true; + this.log('info', 'Opening login in headed Playwright browser (forceHeaded=true)', { + forceHeaded, + }); - const connectResult = await this.browserSession.connect(); + const connectResult = await this.browserSession.connect(forceHeaded); if (!connectResult.success) { return Result.err(new Error(connectResult.error || 'Failed to connect browser')); } @@ -183,7 +186,9 @@ export class PlaywrightAuthSessionService implements IAuthenticationService { timeout: this.navigationTimeoutMs, }); - this.log('info', 'Browser opened to login page, waiting for login...'); + this.log('info', forceHeaded + ? 'Browser opened to login page in headed mode, waiting for login...' + : 'Browser opened to login page, waiting for login...'); this.authState = AuthenticationState.UNKNOWN; const loginSuccess = await this.authFlow.waitForPostLoginRedirect( @@ -219,7 +224,6 @@ export class PlaywrightAuthSessionService implements IAuthenticationService { try { await this.browserSession.disconnect(); } catch { - // ignore cleanup errors } return Result.err(error instanceof Error ? error : new Error(message)); @@ -370,9 +374,9 @@ export class PlaywrightAuthSessionService implements IAuthenticationService { cookieResult.unwrap() === AuthenticationState.AUTHENTICATED; const pageAuthenticated = - (isOnAuthenticatedPath && !isOnLoginPath && cookiesValid) || - hasAuthUI || - (!hasLoginUI && !isOnLoginPath); + !hasLoginUI && + !isOnLoginPath && + ((isOnAuthenticatedPath && cookiesValid) || hasAuthUI); this.log('debug', 'Page authentication check', { url, diff --git a/packages/infrastructure/adapters/automation/core/PlaywrightBrowserSession.ts b/packages/infrastructure/adapters/automation/core/PlaywrightBrowserSession.ts index bd8317dd9..e6d9019d2 100644 --- a/packages/infrastructure/adapters/automation/core/PlaywrightBrowserSession.ts +++ b/packages/infrastructure/adapters/automation/core/PlaywrightBrowserSession.ts @@ -90,8 +90,23 @@ export class PlaywrightBrowserSession { async connect(forceHeaded: boolean = false): Promise<{ success: boolean; error?: string }> { if (this.connected && this.page) { - this.log('debug', 'Already connected, reusing existing connection'); - return { success: true }; + const shouldReuse = + !forceHeaded || + this.actualBrowserMode === 'headed'; + + if (shouldReuse) { + this.log('debug', 'Already connected, reusing existing connection', { + browserMode: this.actualBrowserMode, + forcedHeaded: forceHeaded, + }); + return { success: true }; + } + + this.log('info', 'Existing browser connection is headless, reopening in headed mode for login', { + browserMode: this.actualBrowserMode, + forcedHeaded: forceHeaded, + }); + await this.closeBrowserContext(); } if (this.isConnecting) { diff --git a/tests/e2e/hosted-real/login-and-wizard-smoke.e2e.test.ts b/tests/e2e/hosted-real/login-and-wizard-smoke.e2e.test.ts index f2afe2d9d..bf06371dc 100644 --- a/tests/e2e/hosted-real/login-and-wizard-smoke.e2e.test.ts +++ b/tests/e2e/hosted-real/login-and-wizard-smoke.e2e.test.ts @@ -105,4 +105,55 @@ describeMaybe('Real-site hosted session smoke – login and wizard entry (member }, 300_000, ); + + it( + 'detects login guard and does not attempt Create a Race when not authenticated', + async () => { + const step1Result = await adapter.executeStep(StepId.create(1), {}); + expect(step1Result.success).toBe(true); + + const page = adapter.getPage(); + expect(page).not.toBeNull(); + + const currentUrl = page!.url(); + expect(currentUrl).not.toEqual('about:blank'); + expect(currentUrl.toLowerCase()).toContain('iracing'); + expect(currentUrl.toLowerCase()).toSatisfy((u: string) => + u.includes('oauth.iracing.com') || + u.includes('members.iracing.com') || + u.includes('/login'), + ); + + const emailInput = page! + .locator(IRACING_SELECTORS.login.emailInput) + .first(); + const passwordInput = page! + .locator(IRACING_SELECTORS.login.passwordInput) + .first(); + + const hasEmail = (await emailInput.count()) > 0; + const hasPassword = (await passwordInput.count()) > 0; + + if (!hasEmail && !hasPassword) { + return; + } + + await emailInput.waitFor({ + state: 'visible', + timeout: IRACING_TIMEOUTS.elementWait, + }); + await passwordInput.waitFor({ + state: 'visible', + timeout: IRACING_TIMEOUTS.elementWait, + }); + + const createRaceButton = page! + .locator(IRACING_SELECTORS.hostedRacing.createRaceButton) + .first(); + const createRaceCount = await createRaceButton.count(); + + expect(createRaceCount).toBe(0); + }, + 300_000, + ); }); \ No newline at end of file diff --git a/tests/unit/infrastructure/adapters/PlaywrightAuthSessionService.initiateLogin.browserMode.test.ts b/tests/unit/infrastructure/adapters/PlaywrightAuthSessionService.initiateLogin.browserMode.test.ts new file mode 100644 index 000000000..3e3ce7e25 --- /dev/null +++ b/tests/unit/infrastructure/adapters/PlaywrightAuthSessionService.initiateLogin.browserMode.test.ts @@ -0,0 +1,139 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import type { Page, BrowserContext } from 'playwright'; +import { PlaywrightAuthSessionService } from '../../../packages/infrastructure/adapters/automation/auth/PlaywrightAuthSessionService'; +import type { PlaywrightBrowserSession } from '../../../packages/infrastructure/adapters/automation/core/PlaywrightBrowserSession'; +import type { SessionCookieStore } from '../../../packages/infrastructure/adapters/automation/auth/SessionCookieStore'; +import type { IPlaywrightAuthFlow } from '../../../packages/infrastructure/adapters/automation/auth/PlaywrightAuthFlow'; +import type { ILogger } from '../../../packages/application/ports/ILogger'; +import { AuthenticationState } from '../../../packages/domain/value-objects/AuthenticationState'; +import { Result } from '../../../packages/shared/result/Result'; + +describe('PlaywrightAuthSessionService.initiateLogin browser mode behaviour', () => { + const originalEnv = { ...process.env }; + + let mockBrowserSession: PlaywrightBrowserSession; + let mockCookieStore: SessionCookieStore; + let mockAuthFlow: IPlaywrightAuthFlow; + let mockLogger: ILogger; + let mockPage: Page; + + beforeEach(() => { + process.env = { ...originalEnv }; + + mockLogger = { + debug: vi.fn(), + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + }; + + mockPage = { + goto: vi.fn().mockResolvedValue(undefined), + url: vi.fn().mockReturnValue('https://members-ng.iracing.com/web/racing/hosted/browse-sessions'), + isClosed: vi.fn().mockReturnValue(false), + } as unknown as Page; + + mockBrowserSession = { + connect: vi.fn().mockResolvedValue({ success: true }), + disconnect: vi.fn().mockResolvedValue(undefined), + getPersistentContext: vi.fn().mockReturnValue(null as unknown as BrowserContext | null), + getContext: vi.fn().mockReturnValue(null as unknown as BrowserContext | null), + getPage: vi.fn().mockReturnValue(mockPage), + getUserDataDir: vi.fn().mockReturnValue(''), + } as unknown as PlaywrightBrowserSession; + + mockCookieStore = { + read: vi.fn().mockResolvedValue({ + cookies: [], + origins: [], + }), + write: vi.fn().mockResolvedValue(undefined), + delete: vi.fn().mockResolvedValue(undefined), + validateCookies: vi.fn().mockReturnValue(AuthenticationState.UNKNOWN), + getSessionExpiry: vi.fn(), + getValidCookiesForUrl: vi.fn().mockReturnValue([]), + } as unknown as SessionCookieStore; + + mockAuthFlow = { + getLoginUrl: vi.fn().mockReturnValue('https://members-ng.iracing.com/login'), + getPostLoginLandingUrl: vi.fn().mockReturnValue('https://members-ng.iracing.com/web/racing/hosted/browse-sessions'), + isLoginUrl: vi.fn().mockReturnValue(false), + isAuthenticatedUrl: vi.fn().mockReturnValue(true), + isLoginSuccessUrl: vi.fn().mockReturnValue(true), + detectAuthenticatedUi: vi.fn().mockResolvedValue(true), + detectLoginUi: vi.fn().mockResolvedValue(false), + navigateToAuthenticatedArea: vi.fn().mockResolvedValue(undefined), + waitForPostLoginRedirect: vi.fn().mockResolvedValue(true), + } as unknown as IPlaywrightAuthFlow; + }); + + afterEach(() => { + process.env = { ...originalEnv }; + vi.restoreAllMocks(); + }); + + function createService() { + return new PlaywrightAuthSessionService( + mockBrowserSession, + mockCookieStore, + mockAuthFlow, + mockLogger, + { + navigationTimeoutMs: 1000, + loginWaitTimeoutMs: 1000, + }, + ); + } + + it('always forces headed browser for login regardless of browser mode configuration', async () => { + const service = createService(); + const result = await service.initiateLogin(); + + expect(result.isOk()).toBe(true); + expect(mockBrowserSession.connect).toHaveBeenCalledWith(true); + }); + + it('navigates the headed page to the non-blank login URL', async () => { + const service = createService(); + const result = await service.initiateLogin(); + + expect(result.isOk()).toBe(true); + expect(mockAuthFlow.getLoginUrl).toHaveBeenCalledTimes(2); + + expect(mockPage.goto).toHaveBeenCalledWith( + 'https://members-ng.iracing.com/login', + expect.objectContaining({ + waitUntil: 'domcontentloaded', + }), + ); + + const calledUrl = (mockPage.goto as unknown as ReturnType).mock.calls[0][0] as string; + expect(calledUrl).not.toEqual('about:blank'); + }); + + it('propagates connection failure from browserSession.connect', async () => { + (mockBrowserSession.connect as unknown as ReturnType).mockResolvedValueOnce({ + success: false, + error: 'boom', + }); + + const service = createService(); + const result = await service.initiateLogin(); + + expect(result.isErr()).toBe(true); + const err = result.unwrapErr(); + expect(err).toBeInstanceOf(Error); + expect(err.message).toContain('boom'); + }); + + it('logs explicit headed login message for human companion flow', async () => { + const service = createService(); + const result = await service.initiateLogin(); + + expect(result.isOk()).toBe(true); + expect(mockLogger.info).toHaveBeenCalledWith( + 'Opening login in headed Playwright browser (forceHeaded=true)', + expect.objectContaining({ forceHeaded: true }), + ); + }); +}); \ No newline at end of file diff --git a/tests/unit/infrastructure/adapters/PlaywrightAuthSessionService.verifyPageAuthentication.test.ts b/tests/unit/infrastructure/adapters/PlaywrightAuthSessionService.verifyPageAuthentication.test.ts new file mode 100644 index 000000000..b00b11d02 --- /dev/null +++ b/tests/unit/infrastructure/adapters/PlaywrightAuthSessionService.verifyPageAuthentication.test.ts @@ -0,0 +1,110 @@ +import { describe, it, expect, vi } from 'vitest'; +import type { Page, Locator } from 'playwright'; +import { PlaywrightAuthSessionService } from '../../../packages/infrastructure/adapters/automation/auth/PlaywrightAuthSessionService'; +import { AuthenticationState } from '../../../packages/domain/value-objects/AuthenticationState'; +import { BrowserAuthenticationState } from '../../../packages/domain/value-objects/BrowserAuthenticationState'; +import type { ILogger } from '../../../packages/application/ports/ILogger'; +import type { Result } from '../../../packages/shared/result/Result'; +import type { PlaywrightBrowserSession } from '../../../packages/infrastructure/adapters/automation/core/PlaywrightBrowserSession'; +import type { SessionCookieStore } from '../../../packages/infrastructure/adapters/automation/auth/SessionCookieStore'; +import type { IPlaywrightAuthFlow } from '../../../packages/infrastructure/adapters/automation/auth/PlaywrightAuthFlow'; + +describe('PlaywrightAuthSessionService.verifyPageAuthentication', () => { + function createService(deps: { + pageUrl: string; + hasLoginUi: boolean; + hasAuthUi: boolean; + cookieState: AuthenticationState; + }) { + const mockLogger: ILogger = { + debug: vi.fn(), + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + }; + + const mockLocator: Locator = { + first: vi.fn().mockReturnThis(), + isVisible: vi.fn().mockImplementation(async () => deps.hasLoginUi), + } as unknown as Locator; + + const mockPage: Page = { + url: vi.fn().mockReturnValue(deps.pageUrl), + locator: vi.fn().mockReturnValue(mockLocator), + } as unknown as Page; + + const mockBrowserSession: PlaywrightBrowserSession = { + getPersistentContext: vi.fn().mockReturnValue(null), + getContext: vi.fn().mockReturnValue(null), + getPage: vi.fn().mockReturnValue(mockPage), + } as unknown as PlaywrightBrowserSession; + + const mockCookieStore: SessionCookieStore = { + read: vi.fn().mockResolvedValue({ + cookies: [{ name: 'XSESSIONID', value: 'abc', domain: 'members-ng.iracing.com', path: '/', expires: -1 }], + origins: [], + }), + validateCookies: vi.fn().mockReturnValue(deps.cookieState), + getSessionExpiry: vi.fn(), + write: vi.fn(), + delete: vi.fn(), + } as unknown as SessionCookieStore; + + const mockAuthFlow: IPlaywrightAuthFlow = { + getLoginUrl: () => 'https://members-ng.iracing.com/login', + getPostLoginLandingUrl: () => 'https://members-ng.iracing.com/web/racing/hosted/browse-sessions', + isLoginUrl: (url: string) => url.includes('/login'), + isAuthenticatedUrl: (url: string) => url.includes('/web/racing/hosted'), + isLoginSuccessUrl: (url: string) => url.includes('/web/racing/hosted'), + detectAuthenticatedUi: vi.fn().mockResolvedValue(deps.hasAuthUi), + detectLoginUi: vi.fn(), + navigateToAuthenticatedArea: vi.fn(), + waitForPostLoginRedirect: vi.fn(), + } as unknown as IPlaywrightAuthFlow; + + const service = new PlaywrightAuthSessionService( + mockBrowserSession, + mockCookieStore, + mockAuthFlow, + mockLogger, + ); + + return { service, mockCookieStore, mockAuthFlow, mockPage }; + } + + it('treats cookies-valid + login UI as EXPIRED (page wins over cookies)', async () => { + const { service } = createService({ + pageUrl: 'https://members-ng.iracing.com/web/racing/hosted/browse-sessions', + hasLoginUi: true, + hasAuthUi: false, + cookieState: AuthenticationState.AUTHENTICATED, + }); + + const result: Result = await service.verifyPageAuthentication(); + expect(result.isOk()).toBe(true); + + const browserState = result.unwrap(); + expect(browserState.getCookieValidity()).toBe(true); + expect(browserState.getPageAuthenticationStatus()).toBe(false); + expect(browserState.getAuthenticationState()).toBe(AuthenticationState.EXPIRED); + expect(browserState.requiresReauthentication()).toBe(true); + }); + + it('treats cookies-valid + authenticated UI without login UI as AUTHENTICATED', async () => { + const { service } = createService({ + pageUrl: 'https://members-ng.iracing.com/web/racing/hosted/browse-sessions', + hasLoginUi: false, + hasAuthUi: true, + cookieState: AuthenticationState.AUTHENTICATED, + }); + + const result: Result = await service.verifyPageAuthentication(); + expect(result.isOk()).toBe(true); + + const browserState = result.unwrap(); + expect(browserState.getCookieValidity()).toBe(true); + expect(browserState.getPageAuthenticationStatus()).toBe(true); + expect(browserState.getAuthenticationState()).toBe(AuthenticationState.AUTHENTICATED); + expect(browserState.requiresReauthentication()).toBe(false); + }); +}); \ No newline at end of file