From fafd5a8dd0d06329d0a149542d2feaa4927acc96 Mon Sep 17 00:00:00 2001 From: "tembo[bot]" <208362400+tembo[bot]@users.noreply.github.com> Date: Wed, 29 Jul 2026 18:56:05 +0000 Subject: [PATCH] Fix login reliability and client timeout causing duplicate writes Three related failures made browser-backed commands unreliable, and one of them silently duplicated user data. 1. Kernel client HTTP timeout (the data-loss one) The SDK's client-side timeout defaults to 60s, but automations are dispatched with timeout_sec up to 120s (addCustomFood). The client gave up while the browser was still working, so Cronometer committed the change while crono reported "Request to Kernel API timed out". Every retry then created another copy - logging one item three times produced three identical custom foods. Set an explicit client timeout above the largest timeout_sec. The browser session budget had the same shape of bug: 120s when auto credentials exist, but a command can spend ~60s logging in before a 120s automation starts, so the session could expire mid-operation. Use a single 300s budget. 2. Login navigated via the marketing homepage Login loaded cronometer.com and clicked the header "Log In" link. That click frequently does not navigate, and because the loop recorded a successful *click* it also suppressed the direct-navigation fallback below it. The form was never reachable and login failed with "Could not find email input on https://cronometer.com/". Navigate straight to /login/. 3. Login verification raced the app boot Success was inferred from the absence of login UI after a 500ms wait, which is shorter than the GWT app's swap from login form to app shell. Logins that had actually succeeded reported "Login verification failed". Wait for the shell to settle and confirm with a positive signal - that the diary renders - reusing the existing check, now extracted as buildDiaryPresentedCheckCode() and shared with buildLoginCheckCode(). Also clarify the auto-login error message. When Cronometer throttles logins it serves the logged-out marketing page with no error text, which surfaced as "Your credentials may be incorrect" and invited a retry loop that extended the throttle. Verified end-to-end against a live account: `crono recipes` now logs in headlessly on the first attempt and returns results. Co-authored-by: Dan --- src/automation/runner.ts | 6 +++- src/kernel/client.ts | 23 +++++++++++-- src/kernel/login.ts | 66 +++++++++++++++++++++---------------- tests/kernel/client.test.ts | 40 ++++++++++++++++++++++ tests/kernel/login.test.ts | 26 +++++++++++++-- 5 files changed, 126 insertions(+), 35 deletions(-) create mode 100644 tests/kernel/client.test.ts diff --git a/src/automation/runner.ts b/src/automation/runner.ts index 981de38..93e5455 100644 --- a/src/automation/runner.ts +++ b/src/automation/runner.ts @@ -215,7 +215,11 @@ async function autoLogin( throw new Error( `Auto-login failed: ${data.error ?? data.loginError ?? "Login verification failed"}.\n` + - "Your credentials may be incorrect. Run `crono login` to update them." + "Your credentials may be incorrect — run `crono login` to update them.\n" + + "If they are known good, Cronometer is most likely throttling logins: it\n" + + "then serves the logged-out marketing page with no error text, which\n" + + "surfaces as this message. Wait a few minutes before trying again;\n" + + "retrying in a loop extends the throttle." ); } } diff --git a/src/kernel/client.ts b/src/kernel/client.ts index de2d88f..739960c 100644 --- a/src/kernel/client.ts +++ b/src/kernel/client.ts @@ -17,6 +17,25 @@ import type { } from "../automation/types.js"; import { getCredential } from "../credentials.js"; +/** + * Client-side HTTP timeout for Kernel API requests. + * + * The SDK defaults to 60s, but automations are dispatched with `timeout_sec` + * values up to 120s (see `addCustomFood`). When the client gives up first the + * browser keeps going, so Cronometer commits the change while crono reports + * failure — and a retry then duplicates the data. Keep this comfortably above + * the largest `timeout_sec` used by any automation. + */ +const KERNEL_REQUEST_TIMEOUT_MS = 10 * 60 * 1000; + +/** + * Lifetime of a remote browser session. + * + * A single command can spend ~60s logging in before a 120s automation even + * starts, so the previous 120s budget could expire mid-operation. + */ +const BROWSER_SESSION_TIMEOUT_SEC = 300; + export type KernelClient = AutomationClient; export type { CustomFoodEntry, @@ -44,7 +63,7 @@ export async function getKernelClient(): Promise { } process.env["KERNEL_API_KEY"] = apiKey; - const kernel = new Kernel(); + const kernel = new Kernel({ timeout: KERNEL_REQUEST_TIMEOUT_MS }); return createAutomationClient(createKernelRuntimeFactory(kernel)); } @@ -54,7 +73,7 @@ function createKernelRuntimeFactory(kernel: Kernel): AutomationRuntimeFactory { const browser = await kernel.browsers.create({ headless: hasAutoCredentials, stealth: true, - timeout_seconds: hasAutoCredentials ? 120 : 300, + timeout_seconds: BROWSER_SESSION_TIMEOUT_SEC, }); return { diff --git a/src/kernel/login.ts b/src/kernel/login.ts index 9741d7c..9993664 100644 --- a/src/kernel/login.ts +++ b/src/kernel/login.ts @@ -17,10 +17,7 @@ export function buildLoginCheckCode(): string { await page.waitForTimeout(1000); const url = page.url(); ${buildLoginPresentedCheckCode()} - const diaryPresented = await page.evaluate(() => { - return !!document.querySelector('i.diary-date-previous, i.diary-date-next') || - /Energy\\s+\\d+\\.?\\d*\\s*kcal/i.test(document.body.innerText); - }).catch(() => false); + ${buildDiaryPresentedCheckCode()} const isLoggedIn = url.includes('#diary') && !url.includes('/login') && !url.includes('/signin') && !loginPresented && diaryPresented; return { success: true, loggedIn: isLoggedIn, url, loginPresented, diaryPresented }; `; @@ -41,36 +38,24 @@ export function buildNavigateToLoginCode(): string { * Generate Playwright code that automates Cronometer login. * Fills email/password, submits, and verifies login succeeded. * Credentials are embedded via JSON.stringify for safe escaping. + * + * Navigates directly to /login/. An earlier version loaded the marketing + * homepage and clicked its "Log In" link, but that click frequently does not + * navigate — and because the loop recorded a successful *click* it also + * suppressed the direct-navigation fallback, leaving the form unreachable and + * failing with "Could not find email input on https://cronometer.com/". */ export function buildAutoLoginCode(username: string, password: string): string { const safeUser = JSON.stringify(username); const safePass = JSON.stringify(password); return ` - // Navigate to cronometer.com and click through to the login page - await page.goto('https://cronometer.com', { waitUntil: 'domcontentloaded', timeout: 15000 }); - await page.waitForLoadState('networkidle', { timeout: 8000 }).catch(() => {}); - - // Click the "Log In" link in the top navigation - const loginLinkSelectors = ['a[href="/login/"]', 'a[href="/login"]', 'a:has-text("Log In")', 'a:has-text("Login")']; - let clickedLogin = false; - for (const sel of loginLinkSelectors) { - try { - const el = page.locator(sel); - if (await el.count() > 0) { - await el.first().click(); - clickedLogin = true; - break; - } - } catch {} - } - if (!clickedLogin) { - // Fallback: navigate directly - await page.goto('https://cronometer.com/login/', { waitUntil: 'domcontentloaded', timeout: 15000 }); - } + // Navigate straight to the login page. + await page.goto('https://cronometer.com/login/', { waitUntil: 'domcontentloaded', timeout: 20000 }); + await page.waitForLoadState('networkidle', { timeout: 10000 }).catch(() => {}); // Wait for login page to load - await page.waitForSelector('input[type="email"], input[name="username"], input[name="email"], #email, #username', { timeout: 10000 }).catch(() => {}); + await page.waitForSelector('input[type="email"], input[name="username"], input[name="email"], #email, #username', { timeout: 15000 }).catch(() => {}); // Fill email — try multiple selectors const emailSelectors = ['input[type="email"]', 'input[name="username"]', 'input[name="email"]', '#email', '#username']; @@ -123,13 +108,23 @@ export function buildAutoLoginCode(username: string, password: string): string { return { success: false, loggedIn: false, url: page.url(), error: 'Could not find submit button on ' + page.url() }; } - // Wait for navigation after login + // Wait for navigation after login. The GWT app then needs a moment to swap + // the login UI for the app shell — checking too early made a login that had + // actually succeeded report "Login verification failed". await page.waitForURL(u => !u.href.includes('/login') && !u.href.includes('/signin'), { timeout: 15000 }).catch(() => {}); - await page.waitForTimeout(500); + await page.waitForTimeout(3000); + await page.waitForLoadState('networkidle', { timeout: 10000 }).catch(() => {}); + + // Confirm with a positive signal — that the diary actually renders — rather + // than relying only on the absence of login UI, which races the app's boot. + await page.goto('https://cronometer.com/#diary', { waitUntil: 'domcontentloaded', timeout: 20000 }).catch(() => {}); + await page.waitForLoadState('networkidle', { timeout: 15000 }).catch(() => {}); + await page.waitForTimeout(2000); const url = page.url(); ${buildLoginPresentedCheckCode()} - const loggedIn = !url.includes('/login') && !url.includes('/signin') && !loginPresented; + ${buildDiaryPresentedCheckCode()} + const loggedIn = !url.includes('/login') && !url.includes('/signin') && (diaryPresented || !loginPresented); // If still on login page, check for error messages (rate limit, wrong creds, etc.) let loginError = null; @@ -157,6 +152,19 @@ export function buildAutoLoginCode(username: string, password: string): string { `; } +/** + * Generate code that sets `diaryPresented` — a positive signal that the diary + * UI actually rendered, rather than the mere absence of login UI. + */ +function buildDiaryPresentedCheckCode(): string { + return ` + const diaryPresented = await page.evaluate(() => { + return !!document.querySelector('i.diary-date-previous, i.diary-date-next') || + /Energy\\s+\\d+\\.?\\d*\\s*kcal/i.test(document.body.innerText); + }).catch(() => false); + `; +} + function buildLoginPresentedCheckCode(): string { return ` const loginPresented = await page.evaluate(() => { diff --git a/tests/kernel/client.test.ts b/tests/kernel/client.test.ts new file mode 100644 index 0000000..aa41307 --- /dev/null +++ b/tests/kernel/client.test.ts @@ -0,0 +1,40 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +// Mock the @onkernel/sdk module so we can inspect constructor options. +vi.mock("@onkernel/sdk", () => { + const ctor = vi.fn().mockImplementation(() => ({ + browsers: { + create: vi.fn(), + playwright: { execute: vi.fn() }, + deleteByID: vi.fn(), + }, + })); + return { default: ctor, __ctor: ctor }; +}); + +async function getCtor() { + const mod = await import("@onkernel/sdk"); + return (mod as unknown as { __ctor: ReturnType }).__ctor; +} + +describe("getKernelClient", () => { + beforeEach(async () => { + (await getCtor()).mockClear(); + process.env["KERNEL_API_KEY"] = "test-key"; + }); + + it("should give the SDK a client timeout longer than the slowest automation", async () => { + const { getKernelClient } = await import("../../src/kernel/client.js"); + await getKernelClient(); + + const ctor = await getCtor(); + expect(ctor).toHaveBeenCalledTimes(1); + + const opts = ctor.mock.calls[0]?.[0] as { timeout?: number } | undefined; + // The SDK default is 60s, but addCustomFood dispatches with timeout_sec 120. + // A client that gives up first lets the browser commit the change while + // crono reports failure, so retries silently duplicate data. + expect(opts?.timeout).toBeDefined(); + expect(opts?.timeout).toBeGreaterThan(120_000); + }); +}); diff --git a/tests/kernel/login.test.ts b/tests/kernel/login.test.ts index cc9e40d..afce758 100644 --- a/tests/kernel/login.test.ts +++ b/tests/kernel/login.test.ts @@ -46,10 +46,17 @@ describe("buildNavigateToLoginCode", () => { }); describe("buildAutoLoginCode", () => { - it("should start at cronometer.com and click login link", () => { + it("should navigate directly to the login page", () => { const code = buildAutoLoginCode("user@test.com", "password123"); - expect(code).toContain("cronometer.com"); - expect(code).toContain("loginLinkSelectors"); + expect(code).toContain("cronometer.com/login/"); + }); + + it("should not depend on clicking the marketing homepage login link", () => { + const code = buildAutoLoginCode("user@test.com", "password123"); + // The homepage link often fails to navigate, and clicking it suppressed + // the direct-navigation fallback. + expect(code).not.toContain("loginLinkSelectors"); + expect(code).not.toContain("clickedLogin"); }); it("should fill email field", () => { @@ -82,6 +89,19 @@ describe("buildAutoLoginCode", () => { expect(code).toContain("!loginPresented"); }); + it("should confirm login with a positive diary signal", () => { + const code = buildAutoLoginCode("user@test.com", "password123"); + expect(code).toContain("diaryPresented"); + expect(code).toContain("diary-date-previous"); + expect(code).toContain("diaryPresented || !loginPresented"); + }); + + it("should let the app shell settle before verifying", () => { + const code = buildAutoLoginCode("user@test.com", "password123"); + // A 500ms wait raced the GWT boot and failed successful logins. + expect(code).not.toContain("waitForTimeout(500)"); + }); + it("should return a result object", () => { const code = buildAutoLoginCode("user@test.com", "password123"); expect(code).toContain("return {");