From ac2ebb76024c2e5b1e9adb42e3d4681c9a91140b Mon Sep 17 00:00:00 2001 From: Stuart Date: Mon, 28 Sep 2026 16:38:12 +1000 Subject: [PATCH 1/8] Fix stale endpoint responses, stale project target, false magic-link success - G3: a project fetch or connection check started for the previous endpoint discards its outcome once the endpoint generation advances, so a late response can never overwrite the current endpoint's status or project list. - D2: replacing the FlutterFlow key without choosing a (different) project clears the stored target so a deploy cannot silently use the old project. - G4: an HTTP-success magic-link response carrying any code other than the success code (or none) is surfaced as an error instead of 'Sent!'. - Regression tests for all three behaviours (unit + e2e). --- app.js | 59 +++++++-- e2e/stu384-connection.spec.js | 179 +++++++++++++++++++++++++++ e2e/stu386-magic-link-signin.spec.js | 32 +++++ src/authMagicLink.js | 10 ++ src/authMagicLink.test.js | 18 +++ 5 files changed, 290 insertions(+), 8 deletions(-) diff --git a/app.js b/app.js index 75ddcf0..f34179e 100644 --- a/app.js +++ b/app.js @@ -47,7 +47,7 @@ import { explainPlusAliasRule, getMagicLinkResultMessage, isKnownProviderPlusAlias, - PLUS_ALIAS_REJECTED_CODE, + isMagicLinkSuccess, trimEmail, } from "./src/authMagicLink.js"; import { @@ -1070,6 +1070,12 @@ let confirmProjectToken = 0; // | 'unauthorized' | 'network-error' let ffConnectionState = "not-configured"; +// Monotonic generation of the configured FlutterFlow endpoint. Every project +// fetch and connection check captures it when it starts and discards its +// outcome if the generation moved on, so a late response from a previous +// endpoint can never overwrite the current endpoint's status or project list. +let ffEndpointGeneration = 0; + async function initializeApiKeys() { // Remove an exportable key left by an earlier version even if its encrypted // credentials were already cleared. @@ -1435,6 +1441,10 @@ async function saveApiKeys() { const flutterflowInput = document.getElementById("flutterflow-api-key-input"); const projectSelect = document.getElementById("flutterflow-projects-select"); const enteredKey = flutterflowInput.value.trim(); + // Captured before the save: a replacement key must not silently inherit the + // target project that was chosen for the key it replaces. + const previousProjectId = flutterflowProjectId; + const isKeyReplaced = Boolean(enteredKey) && enteredKey !== flutterflowApiKey; // Only save if user entered a new value if (enteredKey) { @@ -1448,7 +1458,18 @@ async function saveApiKeys() { projectSelect.focus(); return; } - await saveApiKey("flutterflow_project_id", selectedProjectId); + // The dropdown re-selects the stored project whenever it loads, so an + // unchanged value is not a fresh choice. A replacement key keeps only an + // explicitly chosen (different) project; the old target is cleared so a + // deploy can never silently use it with the new key. + if (isKeyReplaced && selectedProjectId === previousProjectId) { + clearStoredProjectSelection(); + projectSelect.value = ""; + } else { + await saveApiKey("flutterflow_project_id", selectedProjectId); + } + } else if (isKeyReplaced) { + clearStoredProjectSelection(); } // Reinitialize keys @@ -1594,7 +1615,12 @@ function renderApiKeyConnection() { * @returns {Promise} the loaded projects, or null on error. */ async function validateFlutterFlowConnection() { + // The endpoint this check belongs to. If the endpoint changes while the + // check is in flight, its outcome describes a previous endpoint and must be + // discarded rather than overwrite the new endpoint's state. + const generation = ffEndpointGeneration; const apiKey = await getApiKey("flutterflow"); + if (generation !== ffEndpointGeneration) return null; if (!apiKey || !hasStoredKey("flutterflow")) { ffConnectionState = "not-configured"; renderApiKeyConnection(); @@ -1612,12 +1638,14 @@ async function validateFlutterFlowConnection() { getFlutterFlowEndpoint(), ); const projects = await client.listProjects(); + if (generation !== ffEndpointGeneration) return null; ffConnectionState = projects && projects.length > 0 ? "connected" : "no-projects"; renderApiKeyConnection(); return projects; } catch (error) { + if (generation !== ffEndpointGeneration) return null; const msg = String((error && error.message) || ""); ffConnectionState = /(401|403)|denied|unauthorized|scoped|list permission/i.test(msg) ? "unauthorized" @@ -1646,6 +1674,10 @@ function clearStoredProjectSelection() { * re-fetch the project list for the new endpoint when a key is configured. */ function invalidateStaleProjectSelection() { + // Any request still in flight belongs to the previous endpoint: advancing + // the generation makes it discard its outcome instead of writing it over the + // new endpoint's status or project list. + ffEndpointGeneration += 1; clearStoredProjectSelection(); const select = document.getElementById("flutterflow-projects-select"); if (select) { @@ -1726,6 +1758,11 @@ async function fetchProjects(apiKey) { return; } + // The endpoint this fetch belongs to. If it changes while the fetch is in + // flight, its response describes a previous endpoint and must be discarded + // rather than overwrite the new endpoint's list or status. + const generation = ffEndpointGeneration; + // Show loading state select.innerHTML = ''; if (errorElement) errorElement.classList.add("hidden"); @@ -1746,6 +1783,7 @@ async function fetchProjects(apiKey) { getFlutterFlowEndpoint(), ); const projects = await client.listProjects(); + if (generation !== ffEndpointGeneration) return; if (!projects || projects.length === 0) { ffConnectionState = "no-projects"; @@ -1772,6 +1810,7 @@ async function fetchProjects(apiKey) { select.value = flutterflowProjectId; } } catch (error) { + if (generation !== ffEndpointGeneration) return; console.error("Failed to fetch projects:", error); const msg = String((error && error.message) || ""); ffConnectionState = /(401|403)|denied|unauthorized|scoped|list permission/i.test(msg) @@ -5553,10 +5592,14 @@ async function handleMagicLinkRequest() { try { const data = await sendMagicLink(email) - const isAliasRejected = data?.code === PLUS_ALIAS_REJECTED_CODE - if (input && !isAliasRejected) input.value = '' - input?.setAttribute('aria-invalid', String(isAliasRejected)) - setSignInMessage(getMagicLinkResultMessage(data, email), isAliasRejected ? 'error' : 'success') + // Only the explicit success code (or a legacy code-less response) counts + // as sent. Any other code — the plus-alias rejection or an unexpected + // error — keeps the address, marks the field invalid and reports an + // error, so the user is never told to check for a link that was not sent. + const isSuccess = isMagicLinkSuccess(data) + if (input && isSuccess) input.value = '' + input?.setAttribute('aria-invalid', String(!isSuccess)) + setSignInMessage(getMagicLinkResultMessage(data, email), isSuccess ? 'success' : 'error') // A successful send must stay retryable — the user may want to resend to // a different address, or send another link if the first one expires — // so the button is re-enabled either way, never left permanently disabled. @@ -5564,8 +5607,8 @@ async function handleMagicLinkRequest() { // clearly reads as usable again. if (btn) { btn.disabled = false - btn.textContent = isAliasRejected ? 'Send Sign-in Link' : 'Sent!' - if (!isAliasRejected) { + btn.textContent = isSuccess ? 'Sent!' : 'Send Sign-in Link' + if (isSuccess) { setTimeout(() => { if (btn.textContent === 'Sent!') btn.textContent = 'Send Sign-in Link' }, 2500) } } diff --git a/e2e/stu384-connection.spec.js b/e2e/stu384-connection.spec.js index 6705b2e..b77f8c8 100644 --- a/e2e/stu384-connection.spec.js +++ b/e2e/stu384-connection.spec.js @@ -24,6 +24,7 @@ import { * key material (raw or masked) is rendered into a status label. */ const KEY = "ff-secret-token-abc123"; +const NEW_KEY = "ff-secret-token-xyz789"; const PROJ = "proj-abc-123"; const EMAIL = "metered@example.com"; const STAGING_LIST = "https://api.flutterflow.io/v2-staging/listProjects"; @@ -110,6 +111,39 @@ async function saveKeyThroughModal(page, { key, project } = {}) { await dismissOpenModals(page); } +// Gate the production projects endpoint: the next request can be held pending +// and released on demand, so a test can let a stale response land only after +// the endpoint changed. Every other production request answers immediately. +async function gateProductionListProjects(page) { + let holdNext = false; + let releaseHeld = null; + let markHeld = null; + const held = new Promise((resolve) => { + markHeld = resolve; + }); + + await page.route(ENDPOINTS.flutterFlowListProjects, async (route) => { + if (!holdNext) { + await route.fulfill(sampleProjectList()); + return; + } + holdNext = false; + markHeld(); + await new Promise((resolve) => { + releaseHeld = resolve; + }); + await route.fulfill(sampleProjectList()); + }); + + return { + holdNext: () => { + holdNext = true; + }, + waitForHeld: () => held, + release: () => releaseHeld(), + }; +} + test.describe("STU-384 account connection", () => { test.beforeEach(async ({ page }) => { await page.addInitScript(() => { @@ -272,6 +306,151 @@ test.describe("STU-384 account connection", () => { ).toHaveCount(0); }); + test("a late project fetch from the previous endpoint cannot overwrite the new endpoint's list", async ({ page }) => { + await seedSession(page); + await applyDefaultRoutes(page, { + ...signedInContext(), + [ENDPOINTS.flutterFlowListProjects]: sampleProjectList(), + [STAGING_LIST]: stagingProjectList(), + }); + const gate = await gateProductionListProjects(page); + + await page.goto("/"); + await saveKeyThroughModal(page, { key: KEY, project: PROJ }); + await expect(page.locator("#acct-ff-project")).toHaveText(PROJ); + + // Re-open the editor: its production re-fetch is held in flight. + gate.holdNext(); + await page + .locator(".acct-connection button", { hasText: "Configure" }) + .first() + .click(); + await expect(page.locator("#api-keys-modal")).toBeVisible(); + await gate.waitForHeld(); + + // Switch to staging while the production response is still pending, so the + // staging list is what the dropdown must show. + await page + .locator("#api-keys-modal summary", { hasText: "Advanced" }) + .click(); + await page + .locator("#flutterflow-endpoint-select") + .selectOption("https://api.flutterflow.io/v2-staging/"); + await expect( + page.locator(`#flutterflow-projects-select option[value="proj-stg-789"]`), + ).toHaveCount(1); + + // Release the stale production response: it must be discarded, not allowed + // to replace the staging list the new endpoint already returned. + const staleSettled = page.waitForResponse(ENDPOINTS.flutterFlowListProjects); + gate.release(); + await staleSettled; + await page.waitForTimeout(150); + + await expect( + page.locator(`#flutterflow-projects-select option[value="proj-stg-789"]`), + ).toHaveCount(1); + await expect( + page.locator(`#flutterflow-projects-select option[value="proj-abc-123"]`), + ).toHaveCount(0); + await expect(page.locator("#acct-ff-status")).toHaveText( + "Connected to FlutterFlow", + ); + await expect(page.locator("#acct-ff-project")).toHaveText("—"); + }); + + test("a late connection check from the previous endpoint cannot overwrite the new endpoint's status", async ({ page }) => { + await seedSession(page); + await applyDefaultRoutes(page, { + ...signedInContext(), + [ENDPOINTS.flutterFlowListProjects]: sampleProjectList(), + [STAGING_LIST]: stagingProjectList(), + }); + const gate = await gateProductionListProjects(page); + + await page.goto("/"); + await saveKeyThroughModal(page, { key: KEY, project: PROJ }); + + // Start a production connection check and keep its response pending. + gate.holdNext(); + await page.evaluate(() => { + window.validateFlutterFlowConnection(); + }); + await gate.waitForHeld(); + await expect(page.locator("#acct-ff-status")).toHaveText( + "Checking connection…", + ); + + // Switch to staging while the check is still pending; staging's re-fetch + // settles the card to connected. + await page + .locator(".acct-connection button", { hasText: "Configure" }) + .first() + .click(); + await expect(page.locator("#api-keys-modal")).toBeVisible(); + await page + .locator("#api-keys-modal summary", { hasText: "Advanced" }) + .click(); + await page + .locator("#flutterflow-endpoint-select") + .selectOption("https://api.flutterflow.io/v2-staging/"); + await expect(page.locator("#acct-ff-status")).toHaveText( + "Connected to FlutterFlow", + ); + + // Release the stale check with a different outcome: it must not overwrite + // the staging status or resurrect a target project. + const staleSettled = page.waitForResponse(ENDPOINTS.flutterFlowListProjects); + gate.release(); + await staleSettled; + await page.waitForTimeout(150); + + await expect(page.locator("#acct-ff-status")).toHaveText( + "Connected to FlutterFlow", + ); + await expect(page.locator("#acct-ff-project")).toHaveText("—"); + await expect( + page.locator(`#flutterflow-projects-select option[value="proj-stg-789"]`), + ).toHaveCount(1); + }); + + test("replacing the key without choosing a project clears the old target", async ({ page }) => { + await seedSession(page); + await applyDefaultRoutes(page, signedInContext()); + await page.goto("/"); + + await saveKeyThroughModal(page, { key: KEY, project: PROJ }); + await expect(page.locator("#acct-ff-project")).toHaveText(PROJ); + + // Re-open the editor and replace the key. The dropdown re-selects the + // stored project when its list loads, but that is not a choice. + await page + .locator(".acct-connection button", { hasText: "Configure" }) + .first() + .click(); + await expect(page.locator("#api-keys-modal")).toBeVisible(); + const input = page.locator("#flutterflow-api-key-input"); + await input.fill(NEW_KEY); + await input.blur(); + await expect(page.locator("#flutterflow-projects-select")).toHaveValue(PROJ); + + await page.locator("#api-keys-modal .bg-blue-500").click(); + await expect(page.locator("#api-keys-modal")).toBeHidden(); + await page.waitForTimeout(1300); + await dismissOpenModals(page); + + // The old key's target is gone: nothing a later deploy reads can silently + // point at it. + const storedAfter = await page.evaluate(() => + localStorage.getItem("ccc_api_key_flutterflow_project_id"), + ); + expect(storedAfter).toBeNull(); + await expect(page.locator("#acct-ff-project")).toHaveText("—"); + await expect(page.locator("#acct-ff-status")).toHaveText( + "Connected to FlutterFlow", + ); + }); + test("removing the key flips the connection card back to not-configured", async ({ page }) => { await seedSession(page); await applyDefaultRoutes(page, signedInContext()); diff --git a/e2e/stu386-magic-link-signin.spec.js b/e2e/stu386-magic-link-signin.spec.js index d9afc1a..f2b8d4d 100644 --- a/e2e/stu386-magic-link-signin.spec.js +++ b/e2e/stu386-magic-link-signin.spec.js @@ -9,6 +9,7 @@ import { magicLinkAliasRejected, magicLinkSendFailure, magicLinkVerifyFailure, + ok, ENDPOINTS, } from "./fixtures/apiFixtures.js"; @@ -105,6 +106,37 @@ test.describe("Magic-link sign-in", () => { await expect(input).toHaveValue("user+tag@gmail.com"); }); + test("an HTTP-success response with an unexpected error code is surfaced as an error", async ({ page }) => { + await applyDefaultRoutes(page, { + [ENDPOINTS.identity]: guestIdentity(), + [ENDPOINTS.getSubscription]: freeSubscription(), + // HTTP 200, but the body carries an error code that is not the + // specifically handled plus-alias rejection: the send did not happen. + [ENDPOINTS.authSendMagicLink]: ok({ + code: "SEND_FAILED", + message: "We could not send the link right now.", + }), + }); + + await page.goto("/#account"); + await page.locator("#auth-signedout button", { hasText: "Sign In" }).click(); + + const input = page.locator("#signin-email-input"); + const submit = page.locator("#signin-submit-btn"); + await input.fill("person@example.com"); + await submit.click(); + + // The error is styled as an error, the address is kept for retry, and the + // control never claims the link was sent. + await expect(page.locator("#signin-message")).toContainText( + "could not send the link", + ); + await expect(page.locator("#signin-message")).toHaveClass(/signin-message-error/); + await expect(input).toHaveValue("person@example.com"); + await expect(input).toHaveAttribute("aria-invalid", "true"); + await expect(submit).toHaveText("Send Sign-in Link"); + }); + test("recovers from a failed send, and retry succeeds", async ({ page }) => { let attempt = 0; await applyDefaultRoutes(page, { diff --git a/src/authMagicLink.js b/src/authMagicLink.js index a2c57e5..a343b5d 100644 --- a/src/authMagicLink.js +++ b/src/authMagicLink.js @@ -36,6 +36,16 @@ export function explainPlusAliasRule(rawEmail) { return renderRejectionMessage(email, domain); } +/** + * A magic-link send is only successful when the server says so: the explicit + * success code, or a legacy response that carries no code at all. Any other + * code — including the plus-alias rejection — is an error, even though the + * HTTP request itself succeeded. + */ +export function isMagicLinkSuccess(data) { + return !data?.code || data.code === MAGIC_LINK_SUCCESS_CODE; +} + export function getMagicLinkResultMessage(data, email) { if (data?.code === PLUS_ALIAS_REJECTED_CODE) { return data.message || explainPlusAliasRule(email) || 'Please enter your primary email address.'; diff --git a/src/authMagicLink.test.js b/src/authMagicLink.test.js index b876031..99852e3 100644 --- a/src/authMagicLink.test.js +++ b/src/authMagicLink.test.js @@ -5,6 +5,7 @@ import { explainPlusAliasRule, getMagicLinkResultMessage, isKnownProviderPlusAlias, + isMagicLinkSuccess, trimEmail, } from "./authMagicLink.js"; @@ -78,3 +79,20 @@ test("getMagicLinkResultMessage treats legacy success responses as sent", () => `Check your email — we sent a link to user@${firstAllowlistedDomain}` ); }); + +test("isMagicLinkSuccess accepts only the success code or a legacy code-less response", () => { + assert.equal(isMagicLinkSuccess({ code: fixtures.successCode }), true); + assert.equal(isMagicLinkSuccess({ success: true, message: "Magic link sent" }), true); + assert.equal(isMagicLinkSuccess({ message: "Check your email" }), true); + + // The alias rejection is an error despite the HTTP success... + assert.equal( + isMagicLinkSuccess({ code: fixtures.validationCode, message: "Use primary email." }), + false + ); + // ...and so is any other server error code, which must never read as sent. + assert.equal( + isMagicLinkSuccess({ code: "SEND_FAILED", message: "Could not send the link." }), + false + ); +}); From 4924e157732d7118aabea02eecb609b117eeb1df Mon Sep 17 00:00:00 2001 From: Stuart Date: Mon, 28 Sep 2026 16:42:33 +1000 Subject: [PATCH 2/8] Make the stale connection-check test discriminate on a different outcome MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The held check now resolves with an empty project list, so a stale response would flip the card to 'no-projects' — visibly different from staging's 'connected' — instead of repeating the same outcome. --- e2e/stu384-connection.spec.js | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/e2e/stu384-connection.spec.js b/e2e/stu384-connection.spec.js index b77f8c8..ae0fd6d 100644 --- a/e2e/stu384-connection.spec.js +++ b/e2e/stu384-connection.spec.js @@ -113,8 +113,9 @@ async function saveKeyThroughModal(page, { key, project } = {}) { // Gate the production projects endpoint: the next request can be held pending // and released on demand, so a test can let a stale response land only after -// the endpoint changed. Every other production request answers immediately. -async function gateProductionListProjects(page) { +// the endpoint changed. Every other production request answers immediately; +// the held request answers with `heldResponse` on release. +async function gateProductionListProjects(page, heldResponse = sampleProjectList) { let holdNext = false; let releaseHeld = null; let markHeld = null; @@ -132,7 +133,7 @@ async function gateProductionListProjects(page) { await new Promise((resolve) => { releaseHeld = resolve; }); - await route.fulfill(sampleProjectList()); + await route.fulfill(heldResponse()); }); return { @@ -366,7 +367,9 @@ test.describe("STU-384 account connection", () => { [ENDPOINTS.flutterFlowListProjects]: sampleProjectList(), [STAGING_LIST]: stagingProjectList(), }); - const gate = await gateProductionListProjects(page); + // The held check resolves with an empty list: a stale "no-projects" + // outcome is visibly different from staging's "connected". + const gate = await gateProductionListProjects(page, emptyProjectList); await page.goto("/"); await saveKeyThroughModal(page, { key: KEY, project: PROJ }); From ec23faf35683f806fac8a0afdd4bbc0c85dd4430 Mon Sep 17 00:00:00 2001 From: Stuart Date: Mon, 28 Sep 2026 17:50:51 +1000 Subject: [PATCH 3/8] Fix stale old-key project responses and code-less magic-link failures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Devin finding 1 (app.js): a projects fetch or connection check now captures the key it was issued with plus a monotonic request sequence. An outcome that a newer request has superseded, or that was issued for a key since replaced or cleared, is discarded — so a late old-key response can no longer populate the new key's project list or connection status when the endpoint did not change. A preview fetch made with the key being saved stays valid. Devin finding 2 (src/authMagicLink.js): an explicit success:false or truthy error is never a success, with or without a code, and an empty or malformed body is not success-shaped. The legacy message-only response still counts. getMagicLinkResultMessage follows the same predicate, so a 200 that says it failed can no longer read as "Sent!". Regression tests: a stale-old-key fetch held across a key replacement (e2e), a success:false sign-in response (e2e), and unit tests for the failure, error-field and malformed-body classification. --- app.js | 67 +++++++++++++++++++++++----- e2e/stu384-connection.spec.js | 65 +++++++++++++++++++++++++++ e2e/stu386-magic-link-signin.spec.js | 26 +++++++++++ src/authMagicLink.js | 21 ++++++--- src/authMagicLink.test.js | 63 +++++++++++++++++++++++++- 5 files changed, 223 insertions(+), 19 deletions(-) diff --git a/app.js b/app.js index f34179e..d06d929 100644 --- a/app.js +++ b/app.js @@ -1076,6 +1076,41 @@ let ffConnectionState = "not-configured"; // endpoint can never overwrite the current endpoint's status or project list. let ffEndpointGeneration = 0; +// Monotonic sequence of started projects requests (fetches and connection +// checks). Only the newest request may apply its outcome, so a late response +// for a key that has since been replaced — or one superseded by a newer key's +// fetch — can never overwrite the new key's project list or status, even when +// the endpoint did not change. +let ffProjectRequestSeq = 0; + +function beginProjectRequest() { + ffProjectRequestSeq += 1; + return ffProjectRequestSeq; +} + +/** + * Whether a projects request's outcome still describes the current connection + * identity and may be applied. Three things can move under an in-flight + * request: the endpoint (ffEndpointGeneration), the configured key (a + * replacement or a clear), and the newest request (a newer request, in + * particular one made with a different key, supersedes every older one). A + * response that lost any of them must not populate the current key's project + * list or connection status. A request made with the currently configured key + * stays valid even when it was issued before that key was saved — that is the + * editor's preview fetch. + */ +function isCurrentProjectRequest({ + seq, + generation, + storedKeyAtStart, + requestKey, +}) { + if (seq !== ffProjectRequestSeq) return false; + if (generation !== ffEndpointGeneration) return false; + return storedKeyAtStart === flutterflowApiKey + || requestKey === flutterflowApiKey; +} + async function initializeApiKeys() { // Remove an exportable key left by an earlier version even if its encrypted // credentials were already cleared. @@ -1615,12 +1650,17 @@ function renderApiKeyConnection() { * @returns {Promise} the loaded projects, or null on error. */ async function validateFlutterFlowConnection() { - // The endpoint this check belongs to. If the endpoint changes while the - // check is in flight, its outcome describes a previous endpoint and must be - // discarded rather than overwrite the new endpoint's state. + // The connection identity this check belongs to. If the endpoint, the + // configured key, or the newest request moves while the check is in flight, + // its outcome describes a previous identity and must be discarded rather + // than overwrite the new one's state. + const seq = beginProjectRequest(); const generation = ffEndpointGeneration; + const storedKeyAtStart = flutterflowApiKey; const apiKey = await getApiKey("flutterflow"); - if (generation !== ffEndpointGeneration) return null; + const outcomeIsCurrent = () => + isCurrentProjectRequest({ seq, generation, storedKeyAtStart, requestKey: apiKey }); + if (!outcomeIsCurrent()) return null; if (!apiKey || !hasStoredKey("flutterflow")) { ffConnectionState = "not-configured"; renderApiKeyConnection(); @@ -1638,14 +1678,14 @@ async function validateFlutterFlowConnection() { getFlutterFlowEndpoint(), ); const projects = await client.listProjects(); - if (generation !== ffEndpointGeneration) return null; + if (!outcomeIsCurrent()) return null; ffConnectionState = projects && projects.length > 0 ? "connected" : "no-projects"; renderApiKeyConnection(); return projects; } catch (error) { - if (generation !== ffEndpointGeneration) return null; + if (!outcomeIsCurrent()) return null; const msg = String((error && error.message) || ""); ffConnectionState = /(401|403)|denied|unauthorized|scoped|list permission/i.test(msg) ? "unauthorized" @@ -1758,10 +1798,15 @@ async function fetchProjects(apiKey) { return; } - // The endpoint this fetch belongs to. If it changes while the fetch is in - // flight, its response describes a previous endpoint and must be discarded - // rather than overwrite the new endpoint's list or status. + // The connection identity this fetch belongs to. If the endpoint, the + // configured key, or the newest request moves while the fetch is in flight, + // its response describes a previous identity and must be discarded rather + // than overwrite the new one's list or status. + const seq = beginProjectRequest(); const generation = ffEndpointGeneration; + const storedKeyAtStart = flutterflowApiKey; + const outcomeIsCurrent = () => + isCurrentProjectRequest({ seq, generation, storedKeyAtStart, requestKey: apiKey }); // Show loading state select.innerHTML = ''; @@ -1783,7 +1828,7 @@ async function fetchProjects(apiKey) { getFlutterFlowEndpoint(), ); const projects = await client.listProjects(); - if (generation !== ffEndpointGeneration) return; + if (!outcomeIsCurrent()) return; if (!projects || projects.length === 0) { ffConnectionState = "no-projects"; @@ -1810,7 +1855,7 @@ async function fetchProjects(apiKey) { select.value = flutterflowProjectId; } } catch (error) { - if (generation !== ffEndpointGeneration) return; + if (!outcomeIsCurrent()) return; console.error("Failed to fetch projects:", error); const msg = String((error && error.message) || ""); ffConnectionState = /(401|403)|denied|unauthorized|scoped|list permission/i.test(msg) diff --git a/e2e/stu384-connection.spec.js b/e2e/stu384-connection.spec.js index ae0fd6d..3eb5fe1 100644 --- a/e2e/stu384-connection.spec.js +++ b/e2e/stu384-connection.spec.js @@ -417,6 +417,71 @@ test.describe("STU-384 account connection", () => { ).toHaveCount(1); }); + test("a late project fetch from the previous key cannot overwrite the new key's list", async ({ page }) => { + await seedSession(page); + await applyDefaultRoutes(page, signedInContext()); + // Only the held OLD-key request answers with this list, so a stale + // response landing after the key was replaced is unmistakable. + const staleKeyList = () => + ok({ + success: true, + value: JSON.stringify({ + entries: [ + { id: "proj-old-key-111", project: { name: "Old Key Project" } }, + ], + }), + }); + const gate = await gateProductionListProjects(page, staleKeyList); + + await page.goto("/"); + await saveKeyThroughModal(page, { key: KEY, project: PROJ }); + await expect(page.locator("#acct-ff-project")).toHaveText(PROJ); + + // Re-open the editor: its fetch with the saved key is held in flight. + gate.holdNext(); + await page + .locator(".acct-connection button", { hasText: "Configure" }) + .first() + .click(); + await expect(page.locator("#api-keys-modal")).toBeVisible(); + await gate.waitForHeld(); + + // Replace the key while the old key's fetch is still pending. The blur + // fetch for the new key answers immediately, so the dropdown now shows + // the list that belongs to the key being saved. + const input = page.locator("#flutterflow-api-key-input"); + await input.fill(NEW_KEY); + await input.blur(); + await expect( + page.locator(`#flutterflow-projects-select option[value="${PROJ}"]`), + ).toHaveCount(1); + + await page.locator("#api-keys-modal .bg-blue-500").click(); + await expect(page.locator("#api-keys-modal")).toBeHidden(); + await page.waitForTimeout(1300); + await dismissOpenModals(page); + + // Release the stale old-key response: it must be discarded, not allowed + // to replace the new key's list or regress its connection status. + const staleSettled = page.waitForResponse(ENDPOINTS.flutterFlowListProjects); + gate.release(); + await staleSettled; + await page.waitForTimeout(150); + + await expect( + page.locator(`#flutterflow-projects-select option[value="${PROJ}"]`), + ).toHaveCount(1); + await expect( + page.locator( + '#flutterflow-projects-select option[value="proj-old-key-111"]', + ), + ).toHaveCount(0); + await expect(page.locator("#acct-ff-status")).toHaveText( + "Connected to FlutterFlow", + ); + await expect(page.locator("#acct-ff-project")).toHaveText("—"); + }); + test("replacing the key without choosing a project clears the old target", async ({ page }) => { await seedSession(page); await applyDefaultRoutes(page, signedInContext()); diff --git a/e2e/stu386-magic-link-signin.spec.js b/e2e/stu386-magic-link-signin.spec.js index f2b8d4d..934fa89 100644 --- a/e2e/stu386-magic-link-signin.spec.js +++ b/e2e/stu386-magic-link-signin.spec.js @@ -137,6 +137,32 @@ test.describe("Magic-link sign-in", () => { await expect(submit).toHaveText("Send Sign-in Link"); }); + test("an HTTP-success response that declares success:false is surfaced as an error", async ({ page }) => { + await applyDefaultRoutes(page, { + [ENDPOINTS.identity]: guestIdentity(), + [ENDPOINTS.getSubscription]: freeSubscription(), + // HTTP 200 with an explicit failure and no code: this must never read as + // "Sent!" or clear the address. + [ENDPOINTS.authSendMagicLink]: ok({ success: false }), + }); + + await page.goto("/#account"); + await page.locator("#auth-signedout button", { hasText: "Sign In" }).click(); + + const input = page.locator("#signin-email-input"); + const submit = page.locator("#signin-submit-btn"); + await input.fill("person@example.com"); + await submit.click(); + + await expect(page.locator("#signin-message")).toContainText( + "Something went wrong", + ); + await expect(page.locator("#signin-message")).toHaveClass(/signin-message-error/); + await expect(input).toHaveValue("person@example.com"); + await expect(input).toHaveAttribute("aria-invalid", "true"); + await expect(submit).toHaveText("Send Sign-in Link"); + }); + test("recovers from a failed send, and retry succeeds", async ({ page }) => { let attempt = 0; await applyDefaultRoutes(page, { diff --git a/src/authMagicLink.js b/src/authMagicLink.js index a343b5d..51a086a 100644 --- a/src/authMagicLink.js +++ b/src/authMagicLink.js @@ -37,21 +37,28 @@ export function explainPlusAliasRule(rawEmail) { } /** - * A magic-link send is only successful when the server says so: the explicit - * success code, or a legacy response that carries no code at all. Any other - * code — including the plus-alias rejection — is an error, even though the - * HTTP request itself succeeded. + * A magic-link send is only successful when the server says so: an explicit + * `success: true`, the success code, or a legacy response that carries no code + * and no failure field. An explicit `success: false` or a truthy `error` is + * never a success, with or without a code, and an empty or malformed body is + * not success-shaped either. Any other code — including the plus-alias + * rejection — is an error, even though the HTTP request itself succeeded. */ export function isMagicLinkSuccess(data) { - return !data?.code || data.code === MAGIC_LINK_SUCCESS_CODE; + if (!data || typeof data !== 'object' || Array.isArray(data)) return false; + if (data.success === false || data.error) return false; + if (data.code) return data.code === MAGIC_LINK_SUCCESS_CODE; + // Legacy code-less response: only an explicit success or the legacy + // message-only shape counts. + return data.success === true || typeof data.message === 'string'; } export function getMagicLinkResultMessage(data, email) { if (data?.code === PLUS_ALIAS_REJECTED_CODE) { return data.message || explainPlusAliasRule(email) || 'Please enter your primary email address.'; } - if (data?.code === MAGIC_LINK_SUCCESS_CODE || !data?.code) { + if (isMagicLinkSuccess(data)) { return `Check your email — we sent a link to ${trimEmail(email)}`; } - return data.message || 'Something went wrong. Please try again.'; + return data?.message || 'Something went wrong. Please try again.'; } diff --git a/src/authMagicLink.test.js b/src/authMagicLink.test.js index 99852e3..9860fa7 100644 --- a/src/authMagicLink.test.js +++ b/src/authMagicLink.test.js @@ -80,7 +80,7 @@ test("getMagicLinkResultMessage treats legacy success responses as sent", () => ); }); -test("isMagicLinkSuccess accepts only the success code or a legacy code-less response", () => { +test("isMagicLinkSuccess accepts only the success code or a genuinely success-shaped legacy response", () => { assert.equal(isMagicLinkSuccess({ code: fixtures.successCode }), true); assert.equal(isMagicLinkSuccess({ success: true, message: "Magic link sent" }), true); assert.equal(isMagicLinkSuccess({ message: "Check your email" }), true); @@ -96,3 +96,64 @@ test("isMagicLinkSuccess accepts only the success code or a legacy code-less res false ); }); + +test("isMagicLinkSuccess rejects explicit failures, error fields and empty bodies", () => { + // A 200 that says it failed is a failure, code or no code — never "sent". + assert.equal(isMagicLinkSuccess({ success: false }), false); + assert.equal( + isMagicLinkSuccess({ success: false, message: "Could not send the link." }), + false + ); + assert.equal( + isMagicLinkSuccess({ success: false, error: "Mail delivery failed" }), + false + ); + assert.equal( + isMagicLinkSuccess({ success: false, code: fixtures.successCode }), + false + ); + + // A code-less error payload is an error shape, not a legacy success. + assert.equal(isMagicLinkSuccess({ error: "Mail delivery failed" }), false); + + // Empty or malformed bodies are not success-shaped. + assert.equal(isMagicLinkSuccess({}), false); + assert.equal(isMagicLinkSuccess(null), false); + assert.equal(isMagicLinkSuccess(undefined), false); + assert.equal(isMagicLinkSuccess("sent"), false); + assert.equal(isMagicLinkSuccess([]), false); +}); + +test("getMagicLinkResultMessage stays aligned with the success predicate", () => { + // A code-less failure must never read as a sent link. + assert.equal( + getMagicLinkResultMessage({ success: false }, `user@${firstAllowlistedDomain}`), + "Something went wrong. Please try again." + ); + assert.equal( + getMagicLinkResultMessage( + { success: false, message: "Could not send the link." }, + `user@${firstAllowlistedDomain}` + ), + "Could not send the link." + ); + assert.equal( + getMagicLinkResultMessage( + { error: "Mail delivery failed" }, + `user@${firstAllowlistedDomain}` + ), + "Something went wrong. Please try again." + ); + // Even the success code cannot rescue an explicit failure. + assert.equal( + getMagicLinkResultMessage( + { success: false, code: fixtures.successCode, message: "Delivery failed." }, + `user@${firstAllowlistedDomain}` + ), + "Delivery failed." + ); + assert.equal( + getMagicLinkResultMessage(null, `user@${firstAllowlistedDomain}`), + "Something went wrong. Please try again." + ); +}); From 7047351c7c006b51639e3f54681b67d352f10ca3 Mon Sep 17 00:00:00 2001 From: Stuart Date: Mon, 28 Sep 2026 18:34:48 +1000 Subject: [PATCH 4/8] Fix stranded project dropdown and false sent state on failed resend MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Devin finding 1 (app.js): the dropdown fetch and the connection check shared one supersession sequence, so a check starting while a fetch was pending discarded the fetch's response and left the dropdown stuck on "Loading projects..." forever. Each kind now keeps its own sequence — both still gated by endpoint generation and the configured key — so a check cannot supersede a pending fetch and vice versa; same-kind supersession and stale key/endpoint rejection are unchanged. Regression test in e2e/stu384-connection.spec.js holds a listProjects fetch, starts a validation, releases the fetch, and asserts the dropdown populates and the card stays connected (fails before the fix). Devin finding 2 (src/accountAccess.js): createSendLinkController marked every resolved response as "sent" and never consulted isMagicLinkSuccess, so a 200 {code:"SEND_FAILED"} or {success:false} rendered the sent confirmation with the server's error message. The controller now classifies the response with the shared magic-link predicate before emitting "sent" and routes unsuccessful responses through its error state with the server's own message (message, else error, else a generic fallback). Unit tests cover SEND_FAILED, success:false and genuine success; an e2e assertion in stu387 covers the row rendering. Pre-existing tests that used the fictional {code:"sent"} fixture now use MAGIC_LINK_SUCCESS_CODE. --- app.js | 69 +++++++++++++++++++++---------- e2e/stu384-connection.spec.js | 48 +++++++++++++++++++++ e2e/stu387-account-access.spec.js | 28 +++++++++++++ src/accountAccess.js | 22 +++++++++- src/accountAccess.test.js | 61 +++++++++++++++++++++++++-- 5 files changed, 201 insertions(+), 27 deletions(-) diff --git a/app.js b/app.js index d06d929..ae888d4 100644 --- a/app.js +++ b/app.js @@ -1076,24 +1076,34 @@ let ffConnectionState = "not-configured"; // endpoint can never overwrite the current endpoint's status or project list. let ffEndpointGeneration = 0; -// Monotonic sequence of started projects requests (fetches and connection -// checks). Only the newest request may apply its outcome, so a late response -// for a key that has since been replaced — or one superseded by a newer key's -// fetch — can never overwrite the new key's project list or status, even when -// the endpoint did not change. -let ffProjectRequestSeq = 0; +// Monotonic sequences of started projects requests. The dropdown fetch and the +// connection check are independent consumers of the same endpoint, so each kind +// keeps its own sequence: a check starting while a fetch is pending must not +// discard the fetch's response (which would strand the dropdown on +// "Loading projects..." forever), and a newer fetch must not discard a pending +// check. Within one kind only the newest request may apply its outcome, so a +// late response for a key that has since been replaced — or one superseded by a +// newer same-kind request — can never overwrite the new key's project list or +// status, even when the endpoint did not change. +let ffProjectFetchSeq = 0; +let ffConnectionCheckSeq = 0; -function beginProjectRequest() { - ffProjectRequestSeq += 1; - return ffProjectRequestSeq; +function beginProjectFetch() { + ffProjectFetchSeq += 1; + return ffProjectFetchSeq; +} + +function beginConnectionCheck() { + ffConnectionCheckSeq += 1; + return ffConnectionCheckSeq; } /** * Whether a projects request's outcome still describes the current connection * identity and may be applied. Three things can move under an in-flight * request: the endpoint (ffEndpointGeneration), the configured key (a - * replacement or a clear), and the newest request (a newer request, in - * particular one made with a different key, supersedes every older one). A + * replacement or a clear), and the newest request of the same kind (a newer + * fetch supersedes every older fetch; a newer check every older check). A * response that lost any of them must not populate the current key's project * list or connection status. A request made with the currently configured key * stays valid even when it was issued before that key was saved — that is the @@ -1101,11 +1111,12 @@ function beginProjectRequest() { */ function isCurrentProjectRequest({ seq, + currentSeq, generation, storedKeyAtStart, requestKey, }) { - if (seq !== ffProjectRequestSeq) return false; + if (seq !== currentSeq) return false; if (generation !== ffEndpointGeneration) return false; return storedKeyAtStart === flutterflowApiKey || requestKey === flutterflowApiKey; @@ -1651,15 +1662,22 @@ function renderApiKeyConnection() { */ async function validateFlutterFlowConnection() { // The connection identity this check belongs to. If the endpoint, the - // configured key, or the newest request moves while the check is in flight, - // its outcome describes a previous identity and must be discarded rather - // than overwrite the new one's state. - const seq = beginProjectRequest(); + // configured key, or the newest connection check moves while the check is in + // flight, its outcome describes a previous identity and must be discarded + // rather than overwrite the new one's state. A pending dropdown fetch is a + // different kind and does not supersede the check (nor the check it). + const seq = beginConnectionCheck(); const generation = ffEndpointGeneration; const storedKeyAtStart = flutterflowApiKey; const apiKey = await getApiKey("flutterflow"); const outcomeIsCurrent = () => - isCurrentProjectRequest({ seq, generation, storedKeyAtStart, requestKey: apiKey }); + isCurrentProjectRequest({ + seq, + currentSeq: ffConnectionCheckSeq, + generation, + storedKeyAtStart, + requestKey: apiKey, + }); if (!outcomeIsCurrent()) return null; if (!apiKey || !hasStoredKey("flutterflow")) { ffConnectionState = "not-configured"; @@ -1799,14 +1817,21 @@ async function fetchProjects(apiKey) { } // The connection identity this fetch belongs to. If the endpoint, the - // configured key, or the newest request moves while the fetch is in flight, - // its response describes a previous identity and must be discarded rather - // than overwrite the new one's list or status. - const seq = beginProjectRequest(); + // configured key, or the newest dropdown fetch moves while the fetch is in + // flight, its response describes a previous identity and must be discarded + // rather than overwrite the new one's list or status. A connection check is + // a different kind and does not supersede the fetch (nor the fetch it). + const seq = beginProjectFetch(); const generation = ffEndpointGeneration; const storedKeyAtStart = flutterflowApiKey; const outcomeIsCurrent = () => - isCurrentProjectRequest({ seq, generation, storedKeyAtStart, requestKey: apiKey }); + isCurrentProjectRequest({ + seq, + currentSeq: ffProjectFetchSeq, + generation, + storedKeyAtStart, + requestKey: apiKey, + }); // Show loading state select.innerHTML = ''; diff --git a/e2e/stu384-connection.spec.js b/e2e/stu384-connection.spec.js index 3eb5fe1..6ccc878 100644 --- a/e2e/stu384-connection.spec.js +++ b/e2e/stu384-connection.spec.js @@ -417,6 +417,54 @@ test.describe("STU-384 account connection", () => { ).toHaveCount(1); }); + test("a connection check started while a dropdown fetch is pending must not strand the dropdown", async ({ page }) => { + await seedSession(page); + await applyDefaultRoutes(page, signedInContext()); + const gate = await gateProductionListProjects(page); + + await page.goto("/"); + await saveKeyThroughModal(page, { key: KEY, project: PROJ }); + await expect(page.locator("#acct-ff-project")).toHaveText(PROJ); + + // Re-open the editor: its dropdown re-fetch is held in flight, so the + // dropdown shows the loading option while the response is pending. + gate.holdNext(); + await page + .locator(".acct-connection button", { hasText: "Configure" }) + .first() + .click(); + await expect(page.locator("#api-keys-modal")).toBeVisible(); + await gate.waitForHeld(); + await expect(page.locator("#flutterflow-projects-select")).toContainText( + "Loading projects...", + ); + + // Start a connection check for the same key and endpoint while the fetch + // is still pending. It answers immediately (the gate holds one request); + // the held fetch is the only response still outstanding. + await page.evaluate(() => window.validateFlutterFlowConnection()); + await expect(page.locator("#acct-ff-status")).toHaveText( + "Connected to FlutterFlow", + ); + + // Release the fetch. It belongs to the current key and endpoint, so its + // response must still populate the dropdown — the check must not have + // superseded it. If it did, the dropdown would be stranded on + // "Loading projects..." forever. + const fetchSettled = page.waitForResponse(ENDPOINTS.flutterFlowListProjects); + gate.release(); + await fetchSettled; + await expect( + page.locator(`#flutterflow-projects-select option[value="${PROJ}"]`), + ).toHaveCount(1); + await expect(page.locator("#flutterflow-projects-select")).not.toContainText( + "Loading projects...", + ); + await expect(page.locator("#acct-ff-status")).toHaveText( + "Connected to FlutterFlow", + ); + }); + test("a late project fetch from the previous key cannot overwrite the new key's list", async ({ page }) => { await seedSession(page); await applyDefaultRoutes(page, signedInContext()); diff --git a/e2e/stu387-account-access.spec.js b/e2e/stu387-account-access.spec.js index 0d645dd..940c4a0 100644 --- a/e2e/stu387-account-access.spec.js +++ b/e2e/stu387-account-access.spec.js @@ -4,6 +4,7 @@ import { guestIdentity, professionalSubscription, magicLinkSent, + ok, err, ENDPOINTS, } from "./fixtures/apiFixtures.js"; @@ -162,6 +163,33 @@ test.describe("STU-387 account access & data", () => { expect(rateLimitText?.trim()).not.toBe(serverErrorText?.trim()); }); + test("a resolved 200 that reports a send failure renders an error, never the sent confirmation", async ({ page }) => { + await seedSession(page); + await applyDefaultRoutes(page, { + ...refreshOverride(), + [ENDPOINTS.identity]: identityWithUsage({ email: EMAIL, count: 12 }), + [ENDPOINTS.getSubscription]: professionalSubscription(), + // HTTP 200, but the body says the send failed: the request resolving is + // not proof a link was sent, so the row must surface the server's + // failure message instead of the sent confirmation. + [ENDPOINTS.authSendMagicLink]: ok({ + code: "SEND_FAILED", + message: "We could not send the link right now.", + }), + }); + await page.goto("/"); + await openAccount(page); + + const msg = page.locator("#send-new-link-msg"); + const btn = page.locator("#send-new-link-btn"); + await btn.click(); + + await expect(msg).toContainText("We could not send the link right now."); + await expect(msg).toHaveClass(/error/); + await expect(msg).not.toContainText("Check your email"); + await expect(btn).toBeEnabled(); + }); + test("a second send while one is in flight is refused (single request)", async ({ page }) => { let calls = 0; await seedSession(page); diff --git a/src/accountAccess.js b/src/accountAccess.js index 9f3cf2d..4555d46 100644 --- a/src/accountAccess.js +++ b/src/accountAccess.js @@ -12,6 +12,8 @@ // the identity/metering cookie, and stored credentials are never cleared by // any control in this section. +import { isMagicLinkSuccess } from "./authMagicLink.js"; + // --------------------------------------------------------------------------- // Key contract // --------------------------------------------------------------------------- @@ -84,11 +86,26 @@ export function performSignOut({ storage, onIdentityChanged, onPlanChanged }) { // Send new link (reauth) // --------------------------------------------------------------------------- +/** + * The most useful user-facing message an unsuccessful send response carries: + * its `message`, else its `error` string, else a generic failure. Never empty, + * so the row's error line always says something. + */ +function sendFailureMessage(data) { + const message = data?.message || data?.error; + if (typeof message === "string" && message.trim()) return message.trim(); + return "The server could not send the link."; +} + /** * Controller for the "Send new link" control. It reflects the ACTUAL backing * response or failure via onState(), moves through pending -> sent|error, and * hard-blocks a duplicate request while one is still in flight (settle first, - * then allow the next send). + * then allow the next send). A resolved request is not automatically a sent + * link: the response is classified with the shared magic-link contract first, + * so an HTTP-success body that reports a failure (an error code, + * success:false, or an error field) becomes the error state carrying the + * server's message instead of a false "sent" confirmation. */ export function createSendLinkController({ sendLink, getEmail, onState }) { let inFlight = false; @@ -99,6 +116,9 @@ export function createSendLinkController({ sendLink, getEmail, onState }) { try { onState({ status: "pending", error: null }); const data = await sendLink(getEmail()); + if (!isMagicLinkSuccess(data)) { + throw new Error(sendFailureMessage(data)); + } result = { status: "sent", data }; onState({ status: "sent", data }); return result; diff --git a/src/accountAccess.test.js b/src/accountAccess.test.js index 6bc7eb8..d9b6718 100644 --- a/src/accountAccess.test.js +++ b/src/accountAccess.test.js @@ -30,6 +30,7 @@ import { isDeleteAccountSupported, DELETE_ACCOUNT_MESSAGE, } from "./accountAccess.js"; +import { MAGIC_LINK_SUCCESS_CODE } from "./authMagicLink.js"; // Minimal in-memory Storage stand-in so the logic is hermetically testable with // no browser, network, clock dependency, or host-timezone coupling. @@ -99,7 +100,10 @@ test("criterion 1: sign out clears the session but never scrubs identity or usag test("criterion 2: a successful reauth reports pending then sent with the actual response", async () => { const states = []; const send = createSendLinkController({ - sendLink: async () => ({ code: "sent", message: "Check your email" }), + sendLink: async () => ({ + code: MAGIC_LINK_SUCCESS_CODE, + message: "Check your email", + }), getEmail: () => "a@b.com", onState: (state) => states.push(state), }); @@ -108,7 +112,56 @@ test("criterion 2: a successful reauth reports pending then sent with the actual assert.equal(result.status, "sent"); assert.equal(result.data.message, "Check your email"); assert.equal(states[states.length - 1].status, "sent"); - assert.equal(states[states.length - 1].data.code, "sent"); + assert.equal(states[states.length - 1].data.code, MAGIC_LINK_SUCCESS_CODE); +}); + +test("criterion 2: a resolved 200 that reports a send failure is an error, never 'sent'", async () => { + const states = []; + const send = createSendLinkController({ + sendLink: async () => ({ + code: "SEND_FAILED", + message: "We could not send the link right now.", + }), + getEmail: () => "a@b.com", + onState: (state) => states.push(state), + }); + const result = await send(); + assert.equal(states[0].status, "pending"); + assert.equal(result.status, "error"); + // The server's own message is preserved so the row can say why. + assert.equal(result.error.message, "We could not send the link right now."); + assert.equal(states[states.length - 1].status, "error"); + // A resolved HTTP request is not proof of a sent link: no state may claim it. + assert.ok( + !states.some((state) => state.status === "sent"), + "a failed send must never render as sent", + ); +}); + +test("criterion 2: a resolved 200 with success:false is an error even without a code", async () => { + const states = []; + const send = createSendLinkController({ + sendLink: async () => ({ success: false, error: "Mail delivery failed" }), + getEmail: () => "a@b.com", + onState: (state) => states.push(state), + }); + const result = await send(); + assert.equal(result.status, "error"); + assert.equal(result.error.message, "Mail delivery failed"); + assert.equal(states[states.length - 1].status, "error"); +}); + +test("criterion 2: a genuine success body still reports sent", async () => { + const states = []; + const send = createSendLinkController({ + sendLink: async () => ({ success: true, message: "Magic link sent" }), + getEmail: () => "a@b.com", + onState: (state) => states.push(state), + }); + const result = await send(); + assert.equal(result.status, "sent"); + assert.equal(result.data.message, "Magic link sent"); + assert.equal(states[states.length - 1].status, "sent"); }); test("criterion 2: a failed reauth surfaces the real failure, not a canned success", async () => { @@ -151,7 +204,7 @@ test("criterion 2: a duplicate reauth while one is in flight is refused and send } finally { // Always settle the in-flight request so a failing assertion cannot strand // a forever-pending promise and cancel the sibling tests. - release({ code: "sent" }); + release({ code: MAGIC_LINK_SUCCESS_CODE }); } firstResult = await first; assert.equal(firstResult.status, "sent"); @@ -163,7 +216,7 @@ test("criterion 2: after a settle the controller accepts the next send", async ( const send = createSendLinkController({ sendLink: async () => { sendLinkCalls += 1; - return { code: "sent" }; + return { code: MAGIC_LINK_SUCCESS_CODE }; }, getEmail: () => "a@b.com", onState: () => {}, From f496402610611d306d7ae4f27f6ad95e0e0bfddb Mon Sep 17 00:00:00 2001 From: Stuart Date: Mon, 28 Sep 2026 21:47:34 +1000 Subject: [PATCH 5/8] Bind project selections to the key+endpoint that confirmed them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Devin finding 3 (app.js): a project chosen from the old key's dropdown before the key was replaced was still persisted as the deploy target, so a later deploy could target a project the new key never listed. Third instance of the stale-identity class, so the approach changes instead of patching the instance: a selection now carries an identity binding ({projectId, key, endpointGeneration}) recorded only from a completed listProjects response, and every read of a selection — saveApiKeys preservation, the dropdown value, the deploy confirm modal and every deploy target read — goes through isProjectSelectionConfirmed (or the modal list-identity gate). A key replacement or endpoint change moves the binding out of match, so no previously confirmed selection can be used until a completed fetch re-confirms it or it is cleared. Regression tests: selecting a project from the old key's list and replacing the key now leaves no stored target and requires a fresh selection (fails before the fix); a stored target the current key's completed list omits is refused at deploy time (fails before the fix). --- app.js | 252 ++++++++++++++++++++++++++++++---- e2e/stu384-connection.spec.js | 99 +++++++++++++ 2 files changed, 326 insertions(+), 25 deletions(-) diff --git a/app.js b/app.js index ae888d4..6514084 100644 --- a/app.js +++ b/app.js @@ -1088,6 +1088,25 @@ let ffEndpointGeneration = 0; let ffProjectFetchSeq = 0; let ffConnectionCheckSeq = 0; +// Identity binding for project selections. A selection is only usable as a +// deploy target while it is confirmed for the key + endpoint generation it was +// made under. This binding is the one thing every read of a selection checks, +// so a key replacement or endpoint change makes every previously confirmed +// selection unusable until a completed listProjects response re-confirms it or +// it is cleared. See isProjectSelectionConfirmed. +let projectSelectionIdentity = null; // { projectId, key, generation } | null + +// Identity of the list currently populating the API-key editor's project +// dropdown: the key + endpoint generation whose completed fetch produced the +// options on screen. Starting a fetch clears it (the options are replaced by +// the loading placeholder); only an applied response sets it. +let projectListIdentity = null; // { key, generation } | null + +// Identity of the list currently populating the deploy confirmation modal's +// project dropdown. A target may only be read from that dropdown while this +// identity still matches the current key + endpoint generation. +let confirmProjectListIdentity = null; // { key, generation } | null + function beginProjectFetch() { ffProjectFetchSeq += 1; return ffProjectFetchSeq; @@ -1098,6 +1117,64 @@ function beginConnectionCheck() { return ffConnectionCheckSeq; } +/** + * Binds a project selection to the identity that produced it. The selection is + * an option of a completed fetch's list, so the binding copies that list's + * identity (the key it was fetched with and the endpoint generation it ran in). + * Called from the dropdown's change handler (a user choice) and when a + * completed response lists the stored selection (a re-confirmation). + */ +function bindProjectSelection(projectId, key, generation) { + projectSelectionIdentity = projectId ? { projectId, key, generation } : null; +} + +/** + * The single gate every read of a project selection goes through. A selection + * may be used as the deploy target only while the binding names the same + * project and was confirmed under the key + endpoint generation being checked. + * A key replacement or endpoint change moves one of them, so the selection + * stops matching and cannot be read as a target until a completed fetch + * re-confirms it. The key is a parameter so the save path can check the key it + * is saving before that key has been loaded into memory. + */ +function isProjectSelectionConfirmed(projectId, key = flutterflowApiKey) { + if (!projectId) return false; + const identity = projectSelectionIdentity; + if (!identity) return false; + return ( + identity.projectId === projectId && + identity.key === key && + identity.generation === ffEndpointGeneration + ); +} + +/** + * Reconciles the stored selection against a completed listProjects response + * for the current key + endpoint. A response that lists the stored project + * re-confirms it; one that does not strips its confirmation, so no later read + * can deploy to a project this key's completed response did not offer. + * Storage is left alone — the account card keeps mirroring the stored + * configuration; only the deploy gate changes. An in-flight explicit choice + * for a different project is never overwritten. + */ +function reconcileStoredProjectSelection(projects, key, generation) { + if (!flutterflowProjectId) return; + if ( + projectSelectionIdentity && + projectSelectionIdentity.projectId !== flutterflowProjectId + ) { + return; + } + const listed = (projects || []).some( + (project) => (project.id || project.projectId) === flutterflowProjectId, + ); + if (listed) { + bindProjectSelection(flutterflowProjectId, key, generation); + } else if (projectSelectionIdentity) { + bindProjectSelection("", null, null); + } +} + /** * Whether a projects request's outcome still describes the current connection * identity and may be applied. Three things can move under an in-flight @@ -1487,9 +1564,6 @@ async function saveApiKeys() { const flutterflowInput = document.getElementById("flutterflow-api-key-input"); const projectSelect = document.getElementById("flutterflow-projects-select"); const enteredKey = flutterflowInput.value.trim(); - // Captured before the save: a replacement key must not silently inherit the - // target project that was chosen for the key it replaces. - const previousProjectId = flutterflowProjectId; const isKeyReplaced = Boolean(enteredKey) && enteredKey !== flutterflowApiKey; // Only save if user entered a new value @@ -1497,6 +1571,10 @@ async function saveApiKeys() { await saveApiKey("flutterflow", enteredKey); } + // The key the selection is saved under: a replacement key is the effective + // one even though initializeApiKeys has not loaded it into memory yet. + const effectiveKey = enteredKey || flutterflowApiKey; + const selectedProjectId = projectSelect?.value.trim() || ""; if (selectedProjectId) { if (!validateFlutterFlowProjectId(selectedProjectId)) { @@ -1504,15 +1582,30 @@ async function saveApiKeys() { projectSelect.focus(); return; } - // The dropdown re-selects the stored project whenever it loads, so an - // unchanged value is not a fresh choice. A replacement key keeps only an - // explicitly chosen (different) project; the old target is cleared so a - // deploy can never silently use it with the new key. - if (isKeyReplaced && selectedProjectId === previousProjectId) { + // A selection is persisted only while the identity binding confirms it for + // the key being saved: the dropdown's change handler binds a choice to the + // key + endpoint generation of the list that produced it, and a completed + // response re-confirms the stored value. A choice made from the old key's + // list (or a value auto-reselected under a replaced key) carries the old + // identity, so it is cleared instead of becoming the new key's target. + if (isProjectSelectionConfirmed(selectedProjectId, effectiveKey)) { + await saveApiKey("flutterflow_project_id", selectedProjectId); + } else { clearStoredProjectSelection(); projectSelect.value = ""; - } else { - await saveApiKey("flutterflow_project_id", selectedProjectId); + // The options on screen came from a list that does not belong to the key + // being saved; refresh them for the new key so the user can make a + // fresh, confirmable choice. + if ( + effectiveKey && + !( + projectListIdentity && + projectListIdentity.key === effectiveKey && + projectListIdentity.generation === ffEndpointGeneration + ) + ) { + fetchProjects(effectiveKey); + } } } else if (isKeyReplaced) { clearStoredProjectSelection(); @@ -1569,6 +1662,11 @@ async function clearAllApiKeys() { sessionStorage.removeItem(SESSION_STORAGE_KEY_PREFIX + "flutterflow"); sessionStorage.removeItem(SESSION_STORAGE_KEY_PREFIX + "flutterflow_project_id"); + // No key and no list: every project selection binding is meaningless now. + projectSelectionIdentity = null; + projectListIdentity = null; + confirmProjectListIdentity = null; + // Reinitialize keys await initializeApiKeys(); @@ -1700,6 +1798,10 @@ async function validateFlutterFlowConnection() { ffConnectionState = projects && projects.length > 0 ? "connected" : "no-projects"; + // A completed check for the current key is authoritative for what that key + // can reach: it re-confirms the stored selection when the response lists + // it, and strips its confirmation when it does not. + reconcileStoredProjectSelection(projects, apiKey, generation); renderApiKeyConnection(); return projects; } catch (error) { @@ -1716,12 +1818,14 @@ async function validateFlutterFlowConnection() { /** * Clears the currently selected FlutterFlow project so a stale selection * (belonging to a different endpoint, or a removed key) is never reused as a - * deploy/sync default. Storage, the in-memory value and the dropdown mirror - * each other: the account connection card and the deploy confirmation read the - * same stored value, so invalidating it keeps them consistent. + * deploy/sync default. Storage, the in-memory value, its identity binding and + * the dropdown mirror each other: the account connection card and the deploy + * confirmation read the same stored value, so invalidating it keeps them + * consistent. */ function clearStoredProjectSelection() { flutterflowProjectId = ""; + projectSelectionIdentity = null; localStorage.removeItem(STORAGE_KEY_PREFIX + "flutterflow_project_id"); sessionStorage.removeItem(SESSION_STORAGE_KEY_PREFIX + "flutterflow_project_id"); } @@ -1736,6 +1840,8 @@ function invalidateStaleProjectSelection() { // the generation makes it discard its outcome instead of writing it over the // new endpoint's status or project list. ffEndpointGeneration += 1; + projectListIdentity = null; + confirmProjectListIdentity = null; clearStoredProjectSelection(); const select = document.getElementById("flutterflow-projects-select"); if (select) { @@ -1782,6 +1888,26 @@ function setupFlutterFlowValidation() { }, 500), ); } + + const projectSelect = document.getElementById("flutterflow-projects-select"); + if (projectSelect) { + projectSelect.addEventListener("change", () => { + const chosen = projectSelect.value.trim(); + // A choice is one of the options a completed fetch produced, so it is + // bound to that list's identity. Without a completed list there is + // nothing to bind, and any previous binding no longer matches the value + // on screen. + if (chosen && projectListIdentity) { + bindProjectSelection( + chosen, + projectListIdentity.key, + projectListIdentity.generation, + ); + } else { + bindProjectSelection("", null, null); + } + }); + } } /** @@ -1833,7 +1959,9 @@ async function fetchProjects(apiKey) { requestKey: apiKey, }); - // Show loading state + // Show loading state. The options are replaced by the loading placeholder + // below, so no list is on screen until this fetch's response is applied. + projectListIdentity = null; select.innerHTML = ''; if (errorElement) errorElement.classList.add("hidden"); @@ -1855,6 +1983,15 @@ async function fetchProjects(apiKey) { const projects = await client.listProjects(); if (!outcomeIsCurrent()) return; + // The applied options belong to this fetch's key + endpoint generation. + projectListIdentity = { key: apiKey, generation }; + // A response issued with the already-stored key re-confirms (or strips) the + // stored selection. A preview fetch for a not-yet-saved key never does: a + // replacement key keeps only a choice explicitly made from its own list. + if (storedKeyAtStart === apiKey) { + reconcileStoredProjectSelection(projects, apiKey, generation); + } + if (!projects || projects.length === 0) { ffConnectionState = "no-projects"; renderApiKeyConnection(); @@ -3712,6 +3849,20 @@ const commitState = { }, }; +/** + * The only way a deploy resolves its target from stored state. The stored + * project is returned only while the identity binding confirms it for the key + * the deploy will actually use (the stored key); otherwise "" is returned and + * the caller fails or asks for a fresh selection. Every deploy read goes + * through here, so a selection that survived a key or endpoint change cannot + * reach a push. + */ +async function getConfirmedStoredProjectId() { + const apiKey = await getApiKey("flutterflow"); + const projectId = await getApiKey("flutterflow_project_id"); + return isProjectSelectionConfirmed(projectId, apiKey) ? projectId : ""; +} + /** * Commits generated code to FlutterFlow with full state tracking. * @param {string} dartCode - The generated Dart code to commit @@ -3737,13 +3888,20 @@ async function commitToFlutterFlow(dartCode, fileName, options = {}) { commitState.setState(CommitState.PREPARING); try { - // Get credentials + // Get credentials. The target project is read through the identity gate: + // a stored selection that is not confirmed for the stored key is never + // used. const apiKey = await getApiKey("flutterflow"); - const projectId = await getApiKey("flutterflow_project_id"); + const projectId = await getConfirmedStoredProjectId(); - if (!apiKey || !projectId) { + if (!apiKey) { + throw new Error( + "FlutterFlow API Key not configured. Please set your API key in the API Keys settings.", + ); + } + if (!projectId) { throw new Error( - "FlutterFlow credentials not configured. Please set your API key and Project ID in the API Keys settings.", + "No confirmed FlutterFlow project target. Choose a project from the loaded list in API Keys settings.", ); } @@ -3936,8 +4094,13 @@ async function executeCommit(code, options = {}) { // Step 3: Validate FlutterFlow credentials commitState.setState(CommitState.VALIDATING); const apiKey = await getApiKey("flutterflow"); + const storedProjectId = await getApiKey("flutterflow_project_id"); + // The modal's choice was gated on its list identity at read time; the + // stored fallback is gated here on the selection binding, so a stored + // selection that is not confirmed for the stored key is never used. const projectId = - commitTargetProjectId || (await getApiKey("flutterflow_project_id")); + commitTargetProjectId || + (isProjectSelectionConfirmed(storedProjectId, apiKey) ? storedProjectId : ""); if (!apiKey) { throw new Error( @@ -3946,7 +4109,9 @@ async function executeCommit(code, options = {}) { } if (!projectId) { throw new Error( - "FlutterFlow Project ID not configured. Please add it in API Keys settings.", + storedProjectId + ? "The FlutterFlow project target is not confirmed for your current API key. Reopen the deploy dialog and choose a project from the loaded list." + : "FlutterFlow Project ID not configured. Please add it in API Keys settings.", ); } @@ -4173,14 +4338,23 @@ async function executeBundleCommit(bundlePlan, options = {}) { commitState.setState(CommitState.VALIDATING); const apiKey = await getApiKey("flutterflow"); + const storedProjectId = await getApiKey("flutterflow_project_id"); + // The modal's choice was gated on its list identity at read time; the + // stored fallback is gated here on the selection binding, so a stored + // selection that is not confirmed for the stored key is never used. const projectId = - commitTargetProjectId || (await getApiKey("flutterflow_project_id")); + commitTargetProjectId || + (isProjectSelectionConfirmed(storedProjectId, apiKey) ? storedProjectId : ""); if (!apiKey) { throw new Error("FlutterFlow API Key not configured. Please add it in API Keys settings."); } if (!projectId) { - throw new Error("FlutterFlow Project ID not configured. Please add it in API Keys settings."); + throw new Error( + storedProjectId + ? "The FlutterFlow project target is not confirmed for your current API key. Reopen the deploy dialog and choose a project from the loaded list." + : "FlutterFlow Project ID not configured. Please add it in API Keys settings.", + ); } if (!validateFlutterFlowProjectId(projectId)) { throw new Error("Invalid FlutterFlow Project ID format."); @@ -6914,6 +7088,11 @@ async function populateConfirmProjectSelect() { // bails instead of resetting the selection with stale data. const token = ++confirmProjectToken; const isCurrent = () => token === confirmProjectToken; + const generation = ffEndpointGeneration; + + // The options are replaced by the loading placeholder below, so the previous + // list is no longer on screen and nothing may be read from it. + confirmProjectListIdentity = null; const apiKey = await getApiKey("flutterflow"); const storedId = await getApiKey("flutterflow_project_id"); @@ -6938,6 +7117,12 @@ async function populateConfirmProjectSelect() { const projects = await client.listProjects(); if (!isCurrent()) return; + // A completed response for the current key is authoritative: it re-confirms + // the stored selection when it lists it and strips its confirmation when it + // does not, so the stored fallback can never target a project this key's + // list did not offer. + reconcileStoredProjectSelection(projects, apiKey, generation); + if (!projects || projects.length === 0) { select.innerHTML = ''; return; @@ -6952,6 +7137,10 @@ async function populateConfirmProjectSelect() { select.appendChild(option); }); + // Only now do the options belong to this key + endpoint generation and may + // a value read from them be used as the deploy target. + confirmProjectListIdentity = { key: apiKey, generation }; + if (storedId) select.value = storedId; } catch (error) { if (!isCurrent()) return; @@ -6962,13 +7151,26 @@ async function populateConfirmProjectSelect() { } /** - * Reads the project chosen in the confirm modal, falling back to the stored - * API Keys default when none was selected. + * Reads the project chosen in the confirm modal. A target is only returned + * while the dropdown still holds a list a completed fetch produced for the + * current key + endpoint: a value that survives a key or endpoint change is not + * a confirmed target. Returns null when no confirmed choice exists, so the + * caller falls back to the stored selection (itself identity-gated) or fails + * rather than deploying to an unconfirmed project. */ function readCommitTargetProjectId() { const select = document.getElementById("confirm-project-select"); const chosen = select?.value?.trim(); - return chosen || null; + if (!chosen) return null; + const identity = confirmProjectListIdentity; + if ( + !identity || + identity.key !== flutterflowApiKey || + identity.generation !== ffEndpointGeneration + ) { + return null; + } + return chosen; } /** diff --git a/e2e/stu384-connection.spec.js b/e2e/stu384-connection.spec.js index 6ccc878..2f294be 100644 --- a/e2e/stu384-connection.spec.js +++ b/e2e/stu384-connection.spec.js @@ -567,6 +567,105 @@ test.describe("STU-384 account connection", () => { ); }); + test("a project chosen from the old key's list cannot become the replaced key's target", async ({ page }) => { + await seedSession(page); + await applyDefaultRoutes(page, signedInContext()); + await page.goto("/"); + + await saveKeyThroughModal(page, { key: KEY, project: PROJ }); + await expect(page.locator("#acct-ff-project")).toHaveText(PROJ); + + // Re-open the editor: the dropdown shows the OLD key's list. + await page + .locator(".acct-connection button", { hasText: "Configure" }) + .first() + .click(); + await expect(page.locator("#api-keys-modal")).toBeVisible(); + await expect( + page.locator('#flutterflow-projects-select option[value="proj-def-456"]'), + ).toHaveCount(1); + + // Choose a project from the old key's list before replacing the key. The + // choice was produced by the old key's completed fetch, so it must not be + // persisted as the new key's deploy target. + await page.locator("#flutterflow-projects-select").selectOption("proj-def-456"); + + const input = page.locator("#flutterflow-api-key-input"); + await input.fill(NEW_KEY); + await page.locator("#api-keys-modal .bg-blue-500").click(); + await expect(page.locator("#api-keys-modal")).toBeHidden(); + await page.waitForTimeout(1300); + await dismissOpenModals(page); + + // No target survives the key replacement: the old-key choice is not the + // new key's target, and no later read can point at it. + const storedAfter = await page.evaluate(() => + localStorage.getItem("ccc_api_key_flutterflow_project_id"), + ); + expect(storedAfter).toBeNull(); + await expect(page.locator("#acct-ff-project")).toHaveText("—"); + await expect(page.locator("#acct-ff-status")).toHaveText( + "Connected to FlutterFlow", + ); + + // The editor requires a fresh selection: the list loaded for the new key, + // but nothing is selected from it. + await page + .locator(".acct-connection button", { hasText: "Configure" }) + .first() + .click(); + await expect(page.locator("#api-keys-modal")).toBeVisible(); + await expect( + page.locator(`#flutterflow-projects-select option[value="${PROJ}"]`), + ).toHaveCount(1); + await expect(page.locator("#flutterflow-projects-select")).toHaveValue(""); + await page.evaluate(() => window.closeApiKeysModal()); + + // The deploy dialog likewise preselects nothing: a target may only come + // from the list its own completed fetch produced for the current key. + await page.evaluate(() => window.__CCC_OPEN_COMMIT_CONFIRM__()); + await expect(page.locator("#confirm-project-select")).toHaveValue(""); + }); + + test("a stored target the current key's completed list omits is not deployable", async ({ page }) => { + await seedSession(page); + await applyDefaultRoutes(page, signedInContext()); + await page.goto("/"); + + // Establish a stored, confirmed target. + await saveKeyThroughModal(page, { key: KEY, project: PROJ }); + + // The key's project list changes: a later completed response for the same + // key no longer lists the stored target. + await applyDefaultRoutes(page, { + ...signedInContext(), + [ENDPOINTS.flutterFlowListProjects]: ok({ + success: true, + value: JSON.stringify({ + entries: [ + { id: "proj-def-456", project: { name: "Test Project Beta" } }, + ], + }), + }), + }); + + // The real deploy dialog's completed fetch does not list the stored + // target, so it must not be preselected... + await page.evaluate(() => window.__CCC_OPEN_COMMIT_CONFIRM__()); + await expect( + page.locator('#confirm-project-select option[value="proj-def-456"]'), + ).toHaveCount(1); + await expect(page.locator("#confirm-project-select")).toHaveValue(""); + + // ...and confirming must fail before any push: the stored target lost its + // confirmation with the completed list, so no deploy may target it. + await page.evaluate(() => window.confirmCommitToFlutterFlow()); + const terminal = page.locator("#commit-terminal-modal"); + await expect(terminal).toBeVisible(); + await expect(terminal).toContainText("not confirmed"); + await expect(terminal).not.toContainText(PROJ); + }); + test("removing the key flips the connection card back to not-configured", async ({ page }) => { await seedSession(page); await applyDefaultRoutes(page, signedInContext()); From f151de4f82c928f389f251078e4fc0daabbea80f Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Mon, 28 Sep 2026 16:11:37 +0000 Subject: [PATCH 6/8] Fix stale-outcome races in shared connection state and stored deploy target Three follow-on bugs from the identity-binding gate (Devin Review on the merged head): - A previewed key's leftover dropdown choice could erase the stored target on save: the unconfirmed-selection path now clears the stored project only on a real key replacement; an unchanged configured key keeps its target and reloads its list instead. - An older dropdown fetch could overwrite a newer connection check's outcome: a shared issue/applied order counter across both request kinds now gates writes to ffConnectionState and stored-selection confirmation, while the newest fetch still fills its own dropdown. - A direct commit during the post-reload project refresh refused the saved target on an unpopulated binding: the confirmed-stored-target read now verifies the project against the current key's real list (same completed-response guard, superseded/failed verifies still refuse) and the deploy dialog's stored fallback uses the same read. Regression coverage: preview-then-clear-save preserves the stored target, a late fetch can't resurrect connected after a failed check, and a direct commit during a pending startup check still deploys. Co-Authored-By: Stuart Gardoll --- app.js | 154 +++++++++++++++++++++++++------ e2e/stu384-connection.spec.js | 167 ++++++++++++++++++++++++++++++++++ 2 files changed, 294 insertions(+), 27 deletions(-) diff --git a/app.js b/app.js index 338705d..f24e837 100644 --- a/app.js +++ b/app.js @@ -1089,6 +1089,16 @@ let ffEndpointGeneration = 0; let ffProjectFetchSeq = 0; let ffConnectionCheckSeq = 0; +// Issue order across BOTH request kinds. The dropdown fetch and the +// connection check are superseded independently within their kind, but they +// write shared state — the account connection status and the stored +// project's confirmation. A request may write shared state only when no newer +// request of either kind has already applied its outcome, so an older +// response can never re-mark the account connected (or restore a stored +// target's confirmation) after a newer request already failed. +let ffRequestOrderSeq = 0; +let ffAppliedOutcomeOrder = 0; + // Identity binding for project selections. A selection is only usable as a // deploy target while it is confirmed for the key + endpoint generation it was // made under. This binding is the one thing every read of a selection checks, @@ -1599,10 +1609,21 @@ async function saveApiKeys() { if (isProjectSelectionConfirmed(selectedProjectId, effectiveKey)) { await saveApiKey("flutterflow_project_id", selectedProjectId); } else { - clearStoredProjectSelection(); + // The on-screen choice belongs to a different key's list. Only a key + // replacement may drop the stored target: with the configured key + // unchanged, that choice is a preview leftover — the stored project + // stays configured and merely the options are reloaded for it. + if (isKeyReplaced) { + clearStoredProjectSelection(); + } else { + // Drop the preview binding so the reloaded list's reconcile can + // re-confirm the stored project (its different-project guard would + // otherwise keep skipping it, leaving the stored target refused). + projectSelectionIdentity = null; + } projectSelect.value = ""; // The options on screen came from a list that does not belong to the key - // being saved; refresh them for the new key so the user can make a + // being saved; refresh them for the saved key so the user can make a // fresh, confirmable choice. if ( effectiveKey && @@ -1773,6 +1794,7 @@ async function validateFlutterFlowConnection() { // rather than overwrite the new one's state. A pending dropdown fetch is a // different kind and does not supersede the check (nor the check it). const seq = beginConnectionCheck(); + const order = ++ffRequestOrderSeq; const generation = ffEndpointGeneration; const storedKeyAtStart = flutterflowApiKey; const apiKey = await getApiKey("flutterflow"); @@ -1784,8 +1806,13 @@ async function validateFlutterFlowConnection() { storedKeyAtStart, requestKey: apiKey, }); - if (!outcomeIsCurrent()) return null; + // Everything a check writes is shared connection state, so a check that + // started before a newer request's outcome was applied has nothing left to + // write: skip it entirely rather than flash "validating" over the newer + // result and then fail silently. + if (!outcomeIsCurrent() || order <= ffAppliedOutcomeOrder) return null; if (!apiKey || !hasStoredKey("flutterflow")) { + ffAppliedOutcomeOrder = order; ffConnectionState = "not-configured"; renderApiKeyConnection(); return null; @@ -1803,6 +1830,10 @@ async function validateFlutterFlowConnection() { ); const projects = await client.listProjects(); if (!outcomeIsCurrent()) return null; + // A newer request of either kind already applied its outcome: this older + // response must not write shared state over it. + if (order <= ffAppliedOutcomeOrder) return null; + ffAppliedOutcomeOrder = order; ffConnectionState = projects && projects.length > 0 ? "connected" : "no-projects"; @@ -1814,6 +1845,8 @@ async function validateFlutterFlowConnection() { return projects; } catch (error) { if (!outcomeIsCurrent()) return null; + if (order <= ffAppliedOutcomeOrder) return null; + ffAppliedOutcomeOrder = order; const msg = String((error && error.message) || ""); ffConnectionState = /(401|403)|denied|unauthorized|scoped|list permission/i.test(msg) ? "unauthorized" @@ -1956,6 +1989,7 @@ async function fetchProjects(apiKey) { // rather than overwrite the new one's list or status. A connection check is // a different kind and does not supersede the fetch (nor the fetch it). const seq = beginProjectFetch(); + const order = ++ffRequestOrderSeq; const generation = ffEndpointGeneration; const storedKeyAtStart = flutterflowApiKey; const outcomeIsCurrent = () => @@ -1966,6 +2000,15 @@ async function fetchProjects(apiKey) { storedKeyAtStart, requestKey: apiKey, }); + // The dropdown is this fetch's own surface: the newest fetch still fills it + // even when its shared-state outcome is stale. Shared writes — connection + // status and stored-project confirmation — go to the newest request of + // either kind only, so an older list cannot re-mark the account connected + // after a newer check failed. + const mayWriteConnectionOutcome = () => order > ffAppliedOutcomeOrder; + const claimConnectionOutcome = () => { + ffAppliedOutcomeOrder = Math.max(ffAppliedOutcomeOrder, order); + }; // Show loading state. The options are replaced by the loading placeholder // below, so no list is on screen until this fetch's response is applied. @@ -1974,8 +2017,9 @@ async function fetchProjects(apiKey) { if (errorElement) errorElement.classList.add("hidden"); // A real request is in flight; the account connection card shows validating - // until the outcome is known (never "connected" from storage alone). - if (hasStoredKey("flutterflow") || apiKey) { + // until the outcome is known (never "connected" from storage alone). An + // already-superseded request leaves the newer applied status alone. + if (mayWriteConnectionOutcome() && (hasStoredKey("flutterflow") || apiKey)) { ffConnectionState = "validating"; } renderApiKeyConnection(); @@ -1993,22 +2037,30 @@ async function fetchProjects(apiKey) { // The applied options belong to this fetch's key + endpoint generation. projectListIdentity = { key: apiKey, generation }; + const outcomeIsNewest = mayWriteConnectionOutcome(); + if (outcomeIsNewest) claimConnectionOutcome(); // A response issued with the already-stored key re-confirms (or strips) the - // stored selection. A preview fetch for a not-yet-saved key never does: a - // replacement key keeps only a choice explicitly made from its own list. - if (storedKeyAtStart === apiKey) { + // stored selection — but only while its connection outcome is still the + // newest: after a newer request's failure, this older success must not + // restore the target's confirmation. A preview fetch for a not-yet-saved + // key never confirms anything regardless. + if (outcomeIsNewest && storedKeyAtStart === apiKey) { reconcileStoredProjectSelection(projects, apiKey, generation); } if (!projects || projects.length === 0) { - ffConnectionState = "no-projects"; - renderApiKeyConnection(); + if (outcomeIsNewest) { + ffConnectionState = "no-projects"; + renderApiKeyConnection(); + } select.innerHTML = ''; return; } - ffConnectionState = "connected"; - renderApiKeyConnection(); + if (outcomeIsNewest) { + ffConnectionState = "connected"; + renderApiKeyConnection(); + } // Populate dropdown select.innerHTML = ''; @@ -2028,10 +2080,13 @@ async function fetchProjects(apiKey) { if (!outcomeIsCurrent()) return; console.error("Failed to fetch projects:", error); const msg = String((error && error.message) || ""); - ffConnectionState = /(401|403)|denied|unauthorized|scoped|list permission/i.test(msg) - ? "unauthorized" - : "network-error"; - renderApiKeyConnection(); + if (mayWriteConnectionOutcome()) { + claimConnectionOutcome(); + ffConnectionState = /(401|403)|denied|unauthorized|scoped|list permission/i.test(msg) + ? "unauthorized" + : "network-error"; + renderApiKeyConnection(); + } select.innerHTML = ''; if (errorElement) { errorElement.textContent = `Failed to load projects: ${error.message}`; @@ -3868,7 +3923,45 @@ const commitState = { async function getConfirmedStoredProjectId() { const apiKey = await getApiKey("flutterflow"); const projectId = await getApiKey("flutterflow_project_id"); - return isProjectSelectionConfirmed(projectId, apiKey) ? projectId : ""; + if (!apiKey || !projectId) return ""; + if (isProjectSelectionConfirmed(projectId, apiKey)) return projectId; + + // The binding is process-local and starts empty on load, so a stored target + // is unconfirmed while the startup listProjects is still pending — and stays + // unconfirmed when no fetch ever ran. Before refusing the deploy, verify the + // stored target against the current key's real list here: the same guard + // applies (a completed response that omits the project leaves it refused), + // and a superseded or failed verification still returns "". + const generation = ffEndpointGeneration; + const order = ++ffRequestOrderSeq; + let projects; + try { + const client = new FlutterFlowApiClient( + apiKey, + "", + "main", + getFlutterFlowEndpoint(), + ); + projects = await client.listProjects(); + } catch { + return ""; + } + // A response that describes a superseded identity — the endpoint moved on, + // or a newer request already applied its outcome — confirms nothing. + if (generation !== ffEndpointGeneration) return ""; + if (order > ffAppliedOutcomeOrder) ffAppliedOutcomeOrder = order; + const listed = (projects || []).some( + (project) => (project.id || project.projectId) === projectId, + ); + if (!listed) return ""; + // Never overwrite an in-flight explicit choice for a different project. + if ( + !projectSelectionIdentity || + projectSelectionIdentity.projectId === projectId + ) { + bindProjectSelection(projectId, apiKey, generation); + } + return projectId; } /** @@ -4104,11 +4197,12 @@ async function executeCommit(code, options = {}) { const apiKey = await getApiKey("flutterflow"); const storedProjectId = await getApiKey("flutterflow_project_id"); // The modal's choice was gated on its list identity at read time; the - // stored fallback is gated here on the selection binding, so a stored - // selection that is not confirmed for the stored key is never used. + // stored fallback goes through the same verifying read every deploy uses, + // so a stored selection that is not confirmed for the stored key is never + // used — and one merely awaiting confirmation is verified here rather + // than refused on a pending binding. const projectId = - commitTargetProjectId || - (isProjectSelectionConfirmed(storedProjectId, apiKey) ? storedProjectId : ""); + commitTargetProjectId || (await getConfirmedStoredProjectId()); if (!apiKey) { throw new Error( @@ -4348,11 +4442,12 @@ async function executeBundleCommit(bundlePlan, options = {}) { const apiKey = await getApiKey("flutterflow"); const storedProjectId = await getApiKey("flutterflow_project_id"); // The modal's choice was gated on its list identity at read time; the - // stored fallback is gated here on the selection binding, so a stored - // selection that is not confirmed for the stored key is never used. + // stored fallback goes through the same verifying read every deploy uses, + // so a stored selection that is not confirmed for the stored key is never + // used — and one merely awaiting confirmation is verified here rather + // than refused on a pending binding. const projectId = - commitTargetProjectId || - (isProjectSelectionConfirmed(storedProjectId, apiKey) ? storedProjectId : ""); + commitTargetProjectId || (await getConfirmedStoredProjectId()); if (!apiKey) { throw new Error("FlutterFlow API Key not configured. Please add it in API Keys settings."); @@ -7115,6 +7210,7 @@ async function populateConfirmProjectSelect() { // closed and reopened while this is in flight, its token falls behind and it // bails instead of resetting the selection with stale data. const token = ++confirmProjectToken; + const order = ++ffRequestOrderSeq; const isCurrent = () => token === confirmProjectToken; const generation = ffEndpointGeneration; @@ -7148,8 +7244,12 @@ async function populateConfirmProjectSelect() { // A completed response for the current key is authoritative: it re-confirms // the stored selection when it lists it and strips its confirmation when it // does not, so the stored fallback can never target a project this key's - // list did not offer. - reconcileStoredProjectSelection(projects, apiKey, generation); + // list did not offer. The confirmation write is shared connection state, + // so it applies only while no newer request has already applied an outcome. + if (order > ffAppliedOutcomeOrder) { + ffAppliedOutcomeOrder = order; + reconcileStoredProjectSelection(projects, apiKey, generation); + } if (!projects || projects.length === 0) { select.innerHTML = ''; diff --git a/e2e/stu384-connection.spec.js b/e2e/stu384-connection.spec.js index 2f294be..96e0056 100644 --- a/e2e/stu384-connection.spec.js +++ b/e2e/stu384-connection.spec.js @@ -706,6 +706,173 @@ test.describe("STU-384 account connection", () => { await expect(page.locator("#acct-ff-status")).not.toContainText("••••"); }); + test("a previewed key's leftover choice cannot erase the stored target on save", async ({ page }) => { + await seedSession(page); + await applyDefaultRoutes(page, signedInContext()); + await page.goto("/"); + + await saveKeyThroughModal(page, { key: KEY, project: PROJ }); + await expect(page.locator("#acct-ff-project")).toHaveText(PROJ); + + // The preview key answers with a distinct list: its only project is + // unmistakably the preview key's, never the configured key's. listProjects + // carries the key in its Authorization header, so the route can tell the + // two requests apart. + const previewList = () => + ok({ + success: true, + value: JSON.stringify({ + entries: [ + { id: "proj-prev-999", project: { name: "Preview Only" } }, + ], + }), + }); + await page.route(ENDPOINTS.flutterFlowListProjects, async (route) => { + const auth = route.request().headers()["authorization"] || ""; + await route.fulfill(auth.includes(NEW_KEY) ? previewList() : sampleProjectList()); + }); + + // Re-open the editor and preview a different key without saving it: the + // blur fetch fills the dropdown with the preview key's list, and picking + // from it binds that choice to the preview key — not the configured one. + await page + .locator(".acct-connection button", { hasText: "Configure" }) + .first() + .click(); + await expect(page.locator("#api-keys-modal")).toBeVisible(); + const input = page.locator("#flutterflow-api-key-input"); + await input.fill(NEW_KEY); + await input.blur(); + await expect( + page.locator('#flutterflow-projects-select option[value="proj-prev-999"]'), + ).toHaveCount(1); + await page + .locator("#flutterflow-projects-select") + .selectOption("proj-prev-999"); + + // Clear the key input and save: the configured key stays the stored one, + // so the dropdown's preview choice is not a selection for it. The stored + // target must survive instead of being cleared with the preview. + await input.fill(""); + await page.locator("#api-keys-modal .bg-blue-500").click(); + await expect(page.locator("#api-keys-modal")).toBeHidden(); + await page.waitForTimeout(1300); + await dismissOpenModals(page); + + const storedAfter = await page.evaluate(() => + localStorage.getItem("ccc_api_key_flutterflow_project_id"), + ); + expect(storedAfter).not.toBeNull(); + await expect(page.locator("#acct-ff-project")).toHaveText(PROJ); + await expect(page.locator("#acct-ff-status")).toHaveText( + "Connected to FlutterFlow", + ); + + // The deploy dialog must still offer the stored project: its completed + // fetch re-confirms the target the preview almost erased. + await page.evaluate(() => window.__CCC_OPEN_COMMIT_CONFIRM__()); + await expect(page.locator("#confirm-project-select")).toHaveValue(PROJ); + }); + + test("a late dropdown fetch cannot overwrite a newer failed connection check", async ({ page }) => { + await seedSession(page); + await applyDefaultRoutes(page, signedInContext()); + await page.goto("/"); + + await saveKeyThroughModal(page, { key: KEY, project: PROJ }); + await expect(page.locator("#acct-ff-status")).toHaveText( + "Connected to FlutterFlow", + ); + + // From here the next projects request (the editor's dropdown fetch) is + // held pending; the connection check issued behind it answers 401. + let releaseHeld; + let markHeld; + const held = new Promise((resolve) => { + markHeld = resolve; + }); + let heldUsed = false; + await page.route(ENDPOINTS.flutterFlowListProjects, async (route) => { + if (!heldUsed) { + heldUsed = true; + markHeld(); + await new Promise((resolve) => { + releaseHeld = resolve; + }); + await route.fulfill(sampleProjectList()); + return; + } + await route.fulfill(err(401, { error: "unauthorized" })); + }); + + // Open the editor: its dropdown fetch is the held request. + await page + .locator(".acct-connection button", { hasText: "Configure" }) + .first() + .click(); + await expect(page.locator("#api-keys-modal")).toBeVisible(); + await held; + + // The connection check issues behind the held fetch and settles first: + // its 401 is the newest shared outcome. + await page.evaluate(() => window.validateFlutterFlowConnection()); + await expect(page.locator("#acct-ff-status")).toHaveText( + "API key rejected (401/403) — re-enter your key", + ); + + // Release the older fetch. Its options still populate the dropdown (its + // own surface), but its connection outcome is stale: it must not flip the + // card back to connected or re-confirm the stored target over the 401. + const fetchSettled = page.waitForResponse(ENDPOINTS.flutterFlowListProjects); + releaseHeld(); + await fetchSettled; + await page.waitForTimeout(150); + + await expect( + page.locator(`#flutterflow-projects-select option[value="${PROJ}"]`), + ).toHaveCount(1); + await expect(page.locator("#acct-ff-status")).toHaveText( + "API key rejected (401/403) — re-enter your key", + ); + await expect(page.locator("#acct-ff-status")).not.toHaveText( + "Connected to FlutterFlow", + ); + }); + + test("a direct commit issued while the post-reload list is pending still deploys the saved project", async ({ page }) => { + await seedSession(page); + await applyDefaultRoutes(page, signedInContext()); + await page.goto("/"); + + await saveKeyThroughModal(page, { key: KEY, project: PROJ }); + await expect(page.locator("#acct-ff-project")).toHaveText(PROJ); + + // Reload with the startup check held pending: storage has the saved key + + // project but the process-local confirmation binding is still empty. + const gate = await gateProductionListProjects(page); + gate.holdNext(); + await page.reload({ waitUntil: "domcontentloaded" }); + await gate.waitForHeld(); + + // A direct commit must not refuse the saved target merely because the + // binding is unpopulated: it verifies the stored project against the + // current key's real list and proceeds. The proof is the deploy reaching + // the remote pipeline — exportCode is the first call past the target + // gate, and a refused target never produces it. The fixtures' fake + // project source fails the later pubspec merge; that downstream error is + // fine, as long as it is not the target-gate refusal. + const exported = page.waitForRequest(ENDPOINTS.flutterFlowExportCode); + const result = await page.evaluate(() => + window.commitToFlutterFlow( + "class TestWidget extends StatelessWidget {}", + "test_widget.dart", + ), + ); + await exported; + expect(result.error || "").not.toMatch(/not confirmed/i); + gate.release(); + }); + test("signed-out entry can still save the key without a connection card", async ({ page }) => { // No session seeded: the account view shows the signed-out prompt. await applyDefaultRoutes(page, { From cbe2f39b2441ea6a022eb42d2acf6b1e141a1c53 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Mon, 28 Sep 2026 16:28:18 +0000 Subject: [PATCH 7/8] Tighten deploy-target verification and confirm-modal staleness MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-on fixes for the second Devin Review round on f151de4: - getConfirmedStoredProjectId() now refuses its own result when a newer request already applied an outcome (including one that stripped the target's confirmation), and re-checks the configured key + endpoint generation before returning a target — a stale verification can no longer revive a removed project. - A project-scoped key whose listProjects is denied (403) no longer loses its stored target: the denial cannot disprove it, so the deploy proceeds and the push decides reachability; endpoint/key guards still apply. - populateConfirmProjectSelect() now treats an endpoint-generation or stored-key move during its fetch as a stale identity and repopulates for the current one instead of filling the deploy dialog with undeployable options. Regression coverage: stale verify cannot revive a removed target, a scoped key keeps its stored target, and an old endpoint's response cannot fill the deploy dialog (all fail on the previous head). Co-Authored-By: Stuart Gardoll --- app.js | 41 ++++++++--- e2e/stu384-connection.spec.js | 134 +++++++++++++++++++++++++++++++++- 2 files changed, 164 insertions(+), 11 deletions(-) diff --git a/app.js b/app.js index f24e837..3663c00 100644 --- a/app.js +++ b/app.js @@ -3934,7 +3934,7 @@ async function getConfirmedStoredProjectId() { // and a superseded or failed verification still returns "". const generation = ffEndpointGeneration; const order = ++ffRequestOrderSeq; - let projects; + let verified = false; try { const client = new FlutterFlowApiClient( apiKey, @@ -3942,18 +3942,29 @@ async function getConfirmedStoredProjectId() { "main", getFlutterFlowEndpoint(), ); - projects = await client.listProjects(); - } catch { - return ""; + const projects = await client.listProjects(); + verified = (projects || []).some( + (project) => (project.id || project.projectId) === projectId, + ); + } catch (error) { + // A project-scoped key can sync its project without list permission: its + // listProjects denial is a 403, not proof the stored project is absent. + // Nothing can disprove the stored target for such a key, so it stands + // (still guarded by the endpoint generation + key checks below) and the + // push itself decides reachability; any other failure refuses. + const msg = String((error && error.message) || ""); + if (!/denied \(403\)|without list permission/i.test(msg)) return ""; + verified = true; } // A response that describes a superseded identity — the endpoint moved on, - // or a newer request already applied its outcome — confirms nothing. + // the configured key changed, or a newer request already applied its + // outcome (including one that stripped this target's confirmation) — + // verifies nothing. if (generation !== ffEndpointGeneration) return ""; - if (order > ffAppliedOutcomeOrder) ffAppliedOutcomeOrder = order; - const listed = (projects || []).some( - (project) => (project.id || project.projectId) === projectId, - ); - if (!listed) return ""; + if (apiKey !== flutterflowApiKey) return ""; + if (order <= ffAppliedOutcomeOrder) return ""; + ffAppliedOutcomeOrder = order; + if (!verified) return ""; // Never overwrite an in-flight explicit choice for a different project. if ( !projectSelectionIdentity || @@ -7240,6 +7251,13 @@ async function populateConfirmProjectSelect() { ); const projects = await client.listProjects(); if (!isCurrent()) return; + // The response describes the endpoint + key it was issued under. If either + // moved while the request was in flight, its options belong to a different + // identity than the deploy would use — repopulate for the current one + // rather than filling the modal with stale, undeployable projects. + if (generation !== ffEndpointGeneration || apiKey !== flutterflowApiKey) { + return populateConfirmProjectSelect(); + } // A completed response for the current key is authoritative: it re-confirms // the stored selection when it lists it and strips its confirmation when it @@ -7272,6 +7290,9 @@ async function populateConfirmProjectSelect() { if (storedId) select.value = storedId; } catch (error) { if (!isCurrent()) return; + if (generation !== ffEndpointGeneration || apiKey !== flutterflowApiKey) { + return populateConfirmProjectSelect(); + } console.error("Failed to load projects for deploy:", error); select.innerHTML = ''; diff --git a/e2e/stu384-connection.spec.js b/e2e/stu384-connection.spec.js index 96e0056..d2377a8 100644 --- a/e2e/stu384-connection.spec.js +++ b/e2e/stu384-connection.spec.js @@ -869,10 +869,142 @@ test.describe("STU-384 account connection", () => { ), ); await exported; - expect(result.error || "").not.toMatch(/not confirmed/i); + expect(result.error || "").not.toMatch(/no confirmed|not confirmed/i); gate.release(); }); + test("a stale deploy verification cannot revive a target a newer response removed", async ({ page }) => { + await seedSession(page); + await applyDefaultRoutes(page, signedInContext()); + await page.goto("/"); + + await saveKeyThroughModal(page, { key: KEY, project: PROJ }); + await expect(page.locator("#acct-ff-project")).toHaveText(PROJ); + + // A list that omits the stored project: the startup check and the later + // superseding check both answer with it, while the held deploy + // verification answers with the full list. + const omittingList = () => + ok({ + success: true, + value: JSON.stringify({ + entries: [ + { id: "proj-def-456", project: { name: "Beta Only" } }, + ], + }), + }); + let releaseVerify; + let markVerify; + const verifyHeld = new Promise((resolve) => { + markVerify = resolve; + }); + let seen = 0; + await page.route(ENDPOINTS.flutterFlowListProjects, async (route) => { + seen += 1; + if (seen === 2) { + markVerify(); + await new Promise((resolve) => { + releaseVerify = resolve; + }); + await route.fulfill(sampleProjectList()); + return; + } + await route.fulfill(omittingList()); + }); + + // Reload: the process-local binding starts empty, and the startup check + // (request 1) does not re-confirm the omitted stored target. + await page.reload({ waitUntil: "domcontentloaded" }); + await page.waitForResponse(ENDPOINTS.flutterFlowListProjects); + + // A direct commit issues its own verification (request 2) and waits. + const commitPromise = page.evaluate(() => + window.commitToFlutterFlow( + "class TestWidget extends StatelessWidget {}", + "test_widget.dart", + ), + ); + await verifyHeld; + + // A newer check (request 3) settles first with a list that omits the + // stored target — the newest applied outcome. + await page.evaluate(() => window.validateFlutterFlowConnection()); + + // The older verification now lands with a list that still contains the + // project. It must not revive the target the newer outcome refused. + releaseVerify(); + const result = await commitPromise; + expect(result.error || "").toMatch(/no confirmed|not confirmed/i); + }); + + test("a project-scoped key without list permission keeps its stored deploy target", async ({ page }) => { + await seedSession(page); + await applyDefaultRoutes(page, signedInContext()); + await page.goto("/"); + + // Establish a stored target while the key can list projects. + await saveKeyThroughModal(page, { key: KEY, project: PROJ }); + await expect(page.locator("#acct-ff-project")).toHaveText(PROJ); + + // The key becomes scoped to its single project: every listProjects call + // now answers 403, but the key can still reach the project itself. + await page.route(ENDPOINTS.flutterFlowListProjects, async (route) => { + await route.fulfill(err(403, { error: "forbidden" })); + }); + await page.reload({ waitUntil: "domcontentloaded" }); + + // A direct commit must not refuse the saved target for lack of a list: + // the scoped denial cannot disprove it, so the deploy proceeds and the + // push itself decides reachability. + const exported = page.waitForRequest(ENDPOINTS.flutterFlowExportCode); + const result = await page.evaluate(() => + window.commitToFlutterFlow( + "class TestWidget extends StatelessWidget {}", + "test_widget.dart", + ), + ); + await exported; + expect(result.error || "").not.toMatch(/no confirmed|not confirmed/i); + }); + + test("an old endpoint's response cannot fill the deploy dialog", async ({ page }) => { + await seedSession(page); + await applyDefaultRoutes(page, { + ...signedInContext(), + [STAGING_LIST]: stagingProjectList(), + }); + await page.goto("/"); + + await saveKeyThroughModal(page, { key: KEY, project: PROJ }); + await expect(page.locator("#acct-ff-project")).toHaveText(PROJ); + + // Hold the deploy dialog's production fetch so the endpoint can change + // while it is in flight. + const gate = await gateProductionListProjects(page); + gate.holdNext(); + await page.evaluate(() => window.__CCC_OPEN_COMMIT_CONFIRM__()); + await gate.waitForHeld(); + + // Switch to staging while the production response is pending. + await page.evaluate(() => + window.setFlutterFlowEndpoint("https://api.flutterflow.io/v2-staging/"), + ); + + // Release the stale production response: its options belong to the old + // endpoint, so the dialog must repopulate from the new endpoint instead + // of offering undeployable production projects. + const fetchSettled = page.waitForResponse(ENDPOINTS.flutterFlowListProjects); + gate.release(); + await fetchSettled; + + await expect( + page.locator('#confirm-project-select option[value="proj-stg-789"]'), + ).toHaveCount(1); + await expect( + page.locator(`#confirm-project-select option[value="${PROJ}"]`), + ).toHaveCount(0); + }); + test("signed-out entry can still save the key without a connection card", async ({ page }) => { // No session seeded: the account view shows the signed-out prompt. await applyDefaultRoutes(page, { From 683f43d73e2062ee5219cada3390010741e0c9b0 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Mon, 28 Sep 2026 17:08:30 +0000 Subject: [PATCH 8/8] Pair deploy credentials as one identity; separate outcome vs confirmation ordering MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Third Devin Review round on cbe2f39: - executeCommit, executeBundleCommit and commitToFlutterFlow now capture key + endpoint as one deploy identity: the captured key is passed into getConfirmedStoredProjectId so the verification describes the same credential the push will use, and the identity is revalidated after every asynchronous read — a key save or endpoint switch landing mid-deploy can no longer pair the old key with a target confirmed for a different identity. - Ordering of shared request state is split by write domain: connection outcomes (the account status card) order separately from selection confirmations. populateConfirmProjectSelect claims only the confirmation order, so a dialog fetch can no longer claim the outcome a pending check needs to settle the card — the card no longer sits on 'Checking connection…' after both requests finish. - fetchProjects writes account status and claims the connection outcome only for the configured key: a preview fetch for a key that is never saved can no longer mark the saved account rejected, nor flash validating on its card. Preview validity still populates the editor dropdown independently. Regression coverage: a deploy-dialog fetch cannot strand the card on checking, a preview key's failure cannot mark the saved account rejected, and a mid-deploy key save cannot pair the old key with a new target (all fail on the previous head). Co-Authored-By: Stuart Gardoll --- app.js | 119 ++++++++++++++++--------- e2e/stu384-connection.spec.js | 163 ++++++++++++++++++++++++++++++++++ 2 files changed, 242 insertions(+), 40 deletions(-) diff --git a/app.js b/app.js index 3663c00..2024649 100644 --- a/app.js +++ b/app.js @@ -1092,12 +1092,16 @@ let ffConnectionCheckSeq = 0; // Issue order across BOTH request kinds. The dropdown fetch and the // connection check are superseded independently within their kind, but they // write shared state — the account connection status and the stored -// project's confirmation. A request may write shared state only when no newer -// request of either kind has already applied its outcome, so an older -// response can never re-mark the account connected (or restore a stored -// target's confirmation) after a newer request already failed. +// project's confirmation. Ordering is split by what a request writes: an +// applied CONNECTION outcome (the account status card) may only be written by +// a request that actually produces one, so a list response that updates the +// deploy dialog never leaves a pending check unable to settle the card; an +// applied CONFIRMATION (the stored target's binding) is ordered separately so +// the newest list response always wins the stored selection. A request may +// write a domain only while no newer request already applied that domain. let ffRequestOrderSeq = 0; let ffAppliedOutcomeOrder = 0; +let ffAppliedConfirmationOrder = 0; // Identity binding for project selections. A selection is only usable as a // deploy target while it is confirmed for the key + endpoint generation it was @@ -1811,8 +1815,12 @@ async function validateFlutterFlowConnection() { // write: skip it entirely rather than flash "validating" over the newer // result and then fail silently. if (!outcomeIsCurrent() || order <= ffAppliedOutcomeOrder) return null; + const applyConfirmation = () => { + ffAppliedConfirmationOrder = Math.max(ffAppliedConfirmationOrder, order); + }; if (!apiKey || !hasStoredKey("flutterflow")) { ffAppliedOutcomeOrder = order; + applyConfirmation(); ffConnectionState = "not-configured"; renderApiKeyConnection(); return null; @@ -1834,19 +1842,24 @@ async function validateFlutterFlowConnection() { // response must not write shared state over it. if (order <= ffAppliedOutcomeOrder) return null; ffAppliedOutcomeOrder = order; + applyConfirmation(); ffConnectionState = projects && projects.length > 0 ? "connected" : "no-projects"; // A completed check for the current key is authoritative for what that key // can reach: it re-confirms the stored selection when the response lists - // it, and strips its confirmation when it does not. - reconcileStoredProjectSelection(projects, apiKey, generation); + // it, and strips its confirmation when it does not — but only while it is + // still the newest confirmation writer, so a fresher list response wins. + if (order >= ffAppliedConfirmationOrder) { + reconcileStoredProjectSelection(projects, apiKey, generation); + } renderApiKeyConnection(); return projects; } catch (error) { if (!outcomeIsCurrent()) return null; if (order <= ffAppliedOutcomeOrder) return null; ffAppliedOutcomeOrder = order; + applyConfirmation(); const msg = String((error && error.message) || ""); ffConnectionState = /(401|403)|denied|unauthorized|scoped|list permission/i.test(msg) ? "unauthorized" @@ -2017,9 +2030,15 @@ async function fetchProjects(apiKey) { if (errorElement) errorElement.classList.add("hidden"); // A real request is in flight; the account connection card shows validating - // until the outcome is known (never "connected" from storage alone). An - // already-superseded request leaves the newer applied status alone. - if (mayWriteConnectionOutcome() && (hasStoredKey("flutterflow") || apiKey)) { + // until the outcome is known (never "connected" from storage alone). Only a + // request for the key that is actually configured describes the account — a + // preview request for a key that is never saved must not flash validating + // (and must not write any account status below). + if ( + mayWriteConnectionOutcome() && + apiKey === flutterflowApiKey && + (hasStoredKey("flutterflow") || apiKey) + ) { ffConnectionState = "validating"; } renderApiKeyConnection(); @@ -2037,14 +2056,19 @@ async function fetchProjects(apiKey) { // The applied options belong to this fetch's key + endpoint generation. projectListIdentity = { key: apiKey, generation }; - const outcomeIsNewest = mayWriteConnectionOutcome(); + // Shared writes — account status and stored-project confirmation — belong + // to the configured key only: a preview fetch for a key that is never + // saved must not describe the account or confirm a selection, while one + // whose key became the configured key mid-flight still applies. + const requestIsForConfiguredKey = apiKey === flutterflowApiKey; + const outcomeIsNewest = + mayWriteConnectionOutcome() && requestIsForConfiguredKey; if (outcomeIsNewest) claimConnectionOutcome(); - // A response issued with the already-stored key re-confirms (or strips) the - // stored selection — but only while its connection outcome is still the - // newest: after a newer request's failure, this older success must not - // restore the target's confirmation. A preview fetch for a not-yet-saved - // key never confirms anything regardless. - if (outcomeIsNewest && storedKeyAtStart === apiKey) { + // A response for the configured key re-confirms (or strips) the stored + // selection while it is still the newest confirmation response: an older + // list must not restore the target's confirmation over a newer one. + if (order > ffAppliedConfirmationOrder && requestIsForConfiguredKey) { + ffAppliedConfirmationOrder = order; reconcileStoredProjectSelection(projects, apiKey, generation); } @@ -2080,7 +2104,7 @@ async function fetchProjects(apiKey) { if (!outcomeIsCurrent()) return; console.error("Failed to fetch projects:", error); const msg = String((error && error.message) || ""); - if (mayWriteConnectionOutcome()) { + if (mayWriteConnectionOutcome() && apiKey === flutterflowApiKey) { claimConnectionOutcome(); ffConnectionState = /(401|403)|denied|unauthorized|scoped|list permission/i.test(msg) ? "unauthorized" @@ -3920,8 +3944,11 @@ const commitState = { * through here, so a selection that survived a key or endpoint change cannot * reach a push. */ -async function getConfirmedStoredProjectId() { - const apiKey = await getApiKey("flutterflow"); +async function getConfirmedStoredProjectId(forApiKey) { + // The deploy passes the key it captured, so the verification describes the + // same credential identity the push will use — never a key saved mid-flight. + const apiKey = + forApiKey !== undefined ? forApiKey : await getApiKey("flutterflow"); const projectId = await getApiKey("flutterflow_project_id"); if (!apiKey || !projectId) return ""; if (isProjectSelectionConfirmed(projectId, apiKey)) return projectId; @@ -3932,8 +3959,8 @@ async function getConfirmedStoredProjectId() { // stored target against the current key's real list here: the same guard // applies (a completed response that omits the project leaves it refused), // and a superseded or failed verification still returns "". - const generation = ffEndpointGeneration; const order = ++ffRequestOrderSeq; + const generation = ffEndpointGeneration; let verified = false; try { const client = new FlutterFlowApiClient( @@ -3962,8 +3989,8 @@ async function getConfirmedStoredProjectId() { // verifies nothing. if (generation !== ffEndpointGeneration) return ""; if (apiKey !== flutterflowApiKey) return ""; - if (order <= ffAppliedOutcomeOrder) return ""; - ffAppliedOutcomeOrder = order; + if (order <= ffAppliedConfirmationOrder) return ""; + ffAppliedConfirmationOrder = order; if (!verified) return ""; // Never overwrite an in-flight explicit choice for a different project. if ( @@ -4004,7 +4031,14 @@ async function commitToFlutterFlow(dartCode, fileName, options = {}) { // a stored selection that is not confirmed for the stored key is never // used. const apiKey = await getApiKey("flutterflow"); - const projectId = await getConfirmedStoredProjectId(); + const endpoint = getFlutterFlowEndpoint(); + let projectId = await getConfirmedStoredProjectId(apiKey); + // A key save or endpoint switch landing during the verification await + // would pair this captured identity with a target confirmed for another + // one — refuse rather than deploy a mismatched credential pair. + if (apiKey !== flutterflowApiKey || endpoint !== getFlutterFlowEndpoint()) { + projectId = ""; + } if (!apiKey) { throw new Error( @@ -4021,7 +4055,6 @@ async function commitToFlutterFlow(dartCode, fileName, options = {}) { throw new Error("Invalid FlutterFlow Project ID format."); } - const endpoint = getFlutterFlowEndpoint(); const apiClient = new FlutterFlowApiClient( apiKey, projectId, @@ -4206,14 +4239,18 @@ async function executeCommit(code, options = {}) { // Step 3: Validate FlutterFlow credentials commitState.setState(CommitState.VALIDATING); const apiKey = await getApiKey("flutterflow"); + const endpoint = getFlutterFlowEndpoint(); const storedProjectId = await getApiKey("flutterflow_project_id"); // The modal's choice was gated on its list identity at read time; the - // stored fallback goes through the same verifying read every deploy uses, - // so a stored selection that is not confirmed for the stored key is never - // used — and one merely awaiting confirmation is verified here rather - // than refused on a pending binding. - const projectId = - commitTargetProjectId || (await getConfirmedStoredProjectId()); + // stored fallback verifies against this deploy's captured key so the push + // always pairs one credential identity — and a key save or endpoint + // switch landing during that verification refuses rather than pairing + // the captured key with a target confirmed for another identity. + let projectId = + commitTargetProjectId || (await getConfirmedStoredProjectId(apiKey)); + if (apiKey !== flutterflowApiKey || endpoint !== getFlutterFlowEndpoint()) { + projectId = ""; + } if (!apiKey) { throw new Error( @@ -4262,7 +4299,6 @@ async function executeCommit(code, options = {}) { } commitState.setState(CommitState.PUSHING); - const endpoint = getFlutterFlowEndpoint(); const apiClient = new FlutterFlowApiClient( apiKey, projectId, @@ -4451,14 +4487,18 @@ async function executeBundleCommit(bundlePlan, options = {}) { commitState.setState(CommitState.VALIDATING); const apiKey = await getApiKey("flutterflow"); + const endpoint = getFlutterFlowEndpoint(); const storedProjectId = await getApiKey("flutterflow_project_id"); // The modal's choice was gated on its list identity at read time; the - // stored fallback goes through the same verifying read every deploy uses, - // so a stored selection that is not confirmed for the stored key is never - // used — and one merely awaiting confirmation is verified here rather - // than refused on a pending binding. - const projectId = - commitTargetProjectId || (await getConfirmedStoredProjectId()); + // stored fallback verifies against this deploy's captured key so the push + // always pairs one credential identity — and a key save or endpoint + // switch landing during that verification refuses rather than pairing + // the captured key with a target confirmed for another identity. + let projectId = + commitTargetProjectId || (await getConfirmedStoredProjectId(apiKey)); + if (apiKey !== flutterflowApiKey || endpoint !== getFlutterFlowEndpoint()) { + projectId = ""; + } if (!apiKey) { throw new Error("FlutterFlow API Key not configured. Please add it in API Keys settings."); @@ -4479,7 +4519,6 @@ async function executeBundleCommit(bundlePlan, options = {}) { targetIdentity.projectId = projectId; commitState.setState(CommitState.PUSHING); - const endpoint = getFlutterFlowEndpoint(); const apiClient = new FlutterFlowApiClient( apiKey, projectId, @@ -7264,8 +7303,8 @@ async function populateConfirmProjectSelect() { // does not, so the stored fallback can never target a project this key's // list did not offer. The confirmation write is shared connection state, // so it applies only while no newer request has already applied an outcome. - if (order > ffAppliedOutcomeOrder) { - ffAppliedOutcomeOrder = order; + if (order > ffAppliedConfirmationOrder) { + ffAppliedConfirmationOrder = order; reconcileStoredProjectSelection(projects, apiKey, generation); } diff --git a/e2e/stu384-connection.spec.js b/e2e/stu384-connection.spec.js index d2377a8..35eb1cd 100644 --- a/e2e/stu384-connection.spec.js +++ b/e2e/stu384-connection.spec.js @@ -9,6 +9,7 @@ import { sampleProjectList, stagingProjectList, emptyProjectList, + projectExport, ENDPOINTS, } from "./fixtures/apiFixtures.js"; @@ -1005,6 +1006,168 @@ test.describe("STU-384 account connection", () => { ).toHaveCount(0); }); + test("a deploy-dialog fetch cannot strand the connection card on checking", async ({ page }) => { + await seedSession(page); + await applyDefaultRoutes(page, signedInContext()); + await page.goto("/"); + + await saveKeyThroughModal(page, { key: KEY, project: PROJ }); + await expect(page.locator("#acct-ff-status")).toHaveText( + "Connected to FlutterFlow", + ); + + // Hold the startup check (first request after reload). The deploy + // dialog's fetch settles first and must not claim the connection outcome + // — it writes no account status, so doing so would leave the older check + // unable to settle the card. + let releaseCheck; + let markCheck; + const checkHeld = new Promise((resolve) => { + markCheck = resolve; + }); + let seen = 0; + await page.route(ENDPOINTS.flutterFlowListProjects, async (route) => { + seen += 1; + if (seen === 1) { + markCheck(); + await new Promise((resolve) => { + releaseCheck = resolve; + }); + } + await route.fulfill(sampleProjectList()); + }); + await page.reload({ waitUntil: "domcontentloaded" }); + await checkHeld; + + // The dialog's fetch (request 2) resolves while the check is pending. + const modalSettled = page.waitForResponse( + ENDPOINTS.flutterFlowListProjects, + ); + await page.evaluate(() => window.__CCC_OPEN_COMMIT_CONFIRM__()); + await modalSettled; + + // The older check now lands: its status must still reach the card. + releaseCheck(); + await page.evaluate(() => window.closeCommitConfirmModal()); + await openAccount(page); + await expect(page.locator("#acct-ff-status")).toHaveText( + "Connected to FlutterFlow", + ); + }); + + test("a preview key's failure cannot mark the saved account rejected", async ({ page }) => { + await seedSession(page); + await applyDefaultRoutes(page, signedInContext()); + // Requests for the unsaved preview key are denied; the saved key keeps + // answering normally. + await page.route(ENDPOINTS.flutterFlowListProjects, async (route) => { + const auth = route.request().headers()["authorization"] || ""; + await route.fulfill( + auth.includes(NEW_KEY) ? err(401, { error: "unauthorized" }) : sampleProjectList(), + ); + }); + await page.goto("/"); + + await saveKeyThroughModal(page, { key: KEY, project: PROJ }); + await expect(page.locator("#acct-ff-status")).toHaveText( + "Connected to FlutterFlow", + ); + + // Preview a different key without saving: its blur fetch is denied, but + // the outcome describes the preview key — not the saved account — so the + // card must stay connected. + await page + .locator(".acct-connection button", { hasText: "Configure" }) + .first() + .click(); + await expect(page.locator("#api-keys-modal")).toBeVisible(); + const previewSettled = page.waitForResponse( + ENDPOINTS.flutterFlowListProjects, + ); + const input = page.locator("#flutterflow-api-key-input"); + await input.fill(NEW_KEY); + await input.blur(); + await previewSettled; + await page.waitForTimeout(150); + + await expect(page.locator("#acct-ff-status")).toHaveText( + "Connected to FlutterFlow", + ); + }); + + test("a key saved mid-deploy cannot pair the old key with a new target", async ({ page }) => { + await seedSession(page); + const newKeyList = () => + ok({ + success: true, + value: JSON.stringify({ + entries: [ + { id: "proj-b-789", project: { name: "New Key Project" } }, + ], + }), + }); + await applyDefaultRoutes(page, signedInContext()); + // The replacement key answers with its own project's list. + await page.route(ENDPOINTS.flutterFlowListProjects, async (route) => { + const auth = route.request().headers()["authorization"] || ""; + await route.fulfill( + auth.includes(NEW_KEY) ? newKeyList() : sampleProjectList(), + ); + }); + let exportCalls = 0; + await page.route(ENDPOINTS.flutterFlowExportCode, async (route) => { + exportCalls += 1; + await route.fulfill(projectExport()); + }); + await page.goto("/"); + + await saveKeyThroughModal(page, { key: KEY, project: PROJ }); + await expect(page.locator("#acct-ff-project")).toHaveText(PROJ); + + // Capture the old stored key's ciphertext, then replace the credential + // pair through the app's real save path (input blur fires the preview + // fetch, the select change binds the new project to the new key, and + // saveApiKeys persists both — the deploy progress overlay would block a + // UI save, so the app's own handlers are driven directly). + const oldCipher = await page.evaluate(() => + localStorage.getItem("ccc_api_key_flutterflow"), + ); + await page.evaluate((key) => { + const input = document.getElementById("flutterflow-api-key-input"); + input.value = key; + input.dispatchEvent(new FocusEvent("blur")); + }, NEW_KEY); + await page.waitForResponse(ENDPOINTS.flutterFlowListProjects); + await page.evaluate((project) => { + const select = document.getElementById("flutterflow-projects-select"); + select.value = project; + select.dispatchEvent(new Event("change", { bubbles: true })); + }, "proj-b-789"); + await page.evaluate(() => window.saveApiKeys()); + + // Model the save landing mid-deploy: the deploy's first credential read + // still returns the pre-save ciphertext (the captured key), while every + // later read — the target lookup's own reads — sees the saved pair. + await page.evaluate((cipher) => { + const orig = localStorage.getItem.bind(localStorage); + let reads = 0; + localStorage.getItem = (k) => + k === "ccc_api_key_flutterflow" && ++reads === 1 ? cipher : orig(k); + }, oldCipher); + + // The deploy must refuse: its captured key belongs to a different + // credential identity than the saved target now confirms — pushing + // would pair the old key with the new key's project. + const result = await page.evaluate(() => + window.commitToFlutterFlow( + "class TestWidget extends StatelessWidget {}", + "test_widget.dart", + ), + ); + expect(result.error || "").toMatch(/no confirmed|not confirmed/i); + expect(exportCalls).toBe(0); + }); + test("signed-out entry can still save the key without a connection card", async ({ page }) => { // No session seeded: the account view shows the signed-out prompt. await applyDefaultRoutes(page, {