diff --git a/app.js b/app.js index a4f106b..00b2fce 100644 --- a/app.js +++ b/app.js @@ -2679,14 +2679,16 @@ function getFileNameFromPath(filePath) { const projectSourceCache = new Map(); // File writes whose provisioning outcome is unconfirmed, per cache key — -// each dirty path maps content -> { uncertain, verified }: `uncertain` counts -// provision attempts that may still write that content (a lost/dropped -// request is not proof the runner stopped), and `verified` credits confirmed -// writes an export sighting must consume first. An export showing content X -// can retire an uncertain X-write only when no verified X-write explains the -// observation — so a confirmed retry can never clear an earlier attempt whose -// identical write may still land over a newer edit. While any entry is -// outstanding, fresh exports are never cached. +// each dirty path maps content -> { uncertain, verified, explainedByVerified }: +// `uncertain` counts provision attempts that may still write that content (a +// lost/dropped request is not proof the runner stopped), and `verified` +// credits confirmed writes an export sighting must consume first. A sighting +// of content X consumes a verified X-credit before it can retire an uncertain +// X-write, and once a verified X-write has explained an X-sighting, +// `explainedByVerified` marks the sighting as spent evidence: a later export +// of the same unchanged X is the same fact, not a new landing, so it can never +// retire an uncertain X-write whose identical content may still land. While +// any entry is outstanding, fresh exports are never cached. const projectSourceDirtyWrites = new Map(); // Unconfirmed sync pushes whose underlying request is still in flight, per @@ -2930,12 +2932,13 @@ async function provisionMissingCodeFiles( ); // A stream that ended (or a body that carried) no definitive result leaves - // the remote decision ambiguous. There are two distinct cases which must not - // be flattened: a server that explicitly refused the request (a 403/5xx - // `response.ok === false`) has made a definitive decision — the write did not - // happen — so that is a FAILURE, not an unknown outcome; only an ok response - // whose stream dropped before a result (or a client wait expiry) leaves the - // remote state genuinely unknown and must be reported UNCONFIRMED. + // the remote decision ambiguous. An explicit 4xx is a definitive pre-write + // refusal — the request was turned away before any write — so it is a + // FAILURE, not an unknown outcome. A 5xx is no such proof: a gateway can + // answer 502/503/504 after forwarding the request to the runner, which may + // already be deploying, so a 5xx — like an ok response whose stream dropped + // before a result, or a client wait expiry — leaves the remote state + // genuinely unknown and must be UNCONFIRMED. if (!result.finalResultReceived) { if (deployOutcomeOfStreamResult(result) === DeployOutcome.FAILED) { // Not marked remoteRefusal: the refusal comes from our deploy runner, @@ -2945,9 +2948,16 @@ async function provisionMissingCodeFiles( `FlutterFlow custom class provisioning failed (HTTP ${result.httpStatus}).`, ); } + // The cause must match what actually happened. A non-ok status (a 5xx, + // typically a gateway) was received, so the runner answered — it did not + // drop the connection — and the message names the status. Only an ok + // response whose stream ended without a result is a dropped connection. + const cause = + result.httpRejected && typeof result.httpStatus === "number" + ? `The FlutterFlow deploy runner returned HTTP ${result.httpStatus} without reporting a result.` + : "The connection to the FlutterFlow deploy runner dropped before it reported a result."; throw new UnconfirmedDeployError( - "The connection to the FlutterFlow deploy runner dropped before it reported a result. " + - "The deploy may still be finishing on the server; open your FlutterFlow project to reconcile " + + `${cause} The deploy may still be finishing on the server; open your FlutterFlow project to reconcile ` + "before retrying.", ); } @@ -3086,14 +3096,21 @@ async function resolveProjectPubspec(apiClient, newDependencies = {}) { // This fresh export is the authoritative read: an observed content // first consumes a verified write credit (the sighting is explained by // a confirmed provision), and only retires an uncertain write when no - // verified write could have produced it. + // verified write could have produced it. A sighting explained by a + // verified write is spent evidence — a later export of the same + // unchanged content is the same fact, so it must not retire the + // uncertain write whose identical content may still land. for (const [path, contents] of [...dirty]) { const observed = projectSource.files.get(path); if (observed !== undefined) { const record = contents.get(observed); if (record) { - if (record.verified > 0) record.verified -= 1; - else record.uncertain -= 1; + if (record.verified > 0) { + record.verified -= 1; + record.explainedByVerified = true; + } else if (!record.explainedByVerified) { + record.uncertain -= 1; + } if (record.uncertain <= 0 && record.verified <= 0) { contents.delete(observed); } @@ -6772,6 +6789,7 @@ document.addEventListener("DOMContentLoaded", async () => { updateProgressFromState(state); } else if (state === CommitState.SUCCESS || state === CommitState.ERROR) { updateProgressFromState(state); + if (commitProgressHideTimer) clearTimeout(commitProgressHideTimer); commitProgressHideTimer = setTimeout(() => { commitProgressHideTimer = null; hideCommitProgress(); diff --git a/e2e/stu380-deploy.spec.js b/e2e/stu380-deploy.spec.js index 403dd54..c417c7d 100644 --- a/e2e/stu380-deploy.spec.js +++ b/e2e/stu380-deploy.spec.js @@ -235,6 +235,49 @@ test.describe("STU-380 deployment terminal outcomes", () => { await expect(page.locator("#commit-progress-overlay")).toBeHidden(); }); + test("a new deploy within the terminal hide delay keeps its progress overlay", async ({ page }) => { + await page.addInitScript(() => { + localStorage.setItem("hasSeenWalkthrough", "true"); + }); + await applyDefaultRoutes(page); + await page.goto("/"); + // The commitStateChange listener is attached at the end of the app's async + // startup, so wait for the real listener to answer: dispatching PREPARING + // opens the overlay. This is also deploy 1 starting. + await page.waitForFunction(() => { + window.dispatchEvent( + new CustomEvent("commitStateChange", { detail: { state: "PREPARING" } }), + ); + return document + .getElementById("commit-progress-overlay") + ?.classList.contains("open"); + }); + await expect(page.locator("#commit-progress-overlay")).toBeVisible(); + + // Deploy 1 ends: the terminal state schedules the overlay hide one second + // later, but the overlay is still up while that delay runs. + await page.evaluate(() => { + window.dispatchEvent( + new CustomEvent("commitStateChange", { detail: { state: "SUCCESS" } }), + ); + }); + + // Deploy 2 starts inside that second, exactly as a real confirm does by + // emitting PREPARING. The previous deploy's timer must not close the + // overlay that now belongs to the running deploy. + await page.evaluate(() => { + window.dispatchEvent( + new CustomEvent("commitStateChange", { detail: { state: "PREPARING" } }), + ); + }); + // Past the 1s hide delay and the overlay's 0.25s visibility transition: + // a stale timer has removed `.open` by now, so the class is the settled + // evidence — not a computed style that is mid-transition. + await page.waitForTimeout(1500); + await expect(page.locator("#commit-progress-overlay")).toHaveClass(/\bopen\b/); + await expect(page.locator("#commit-progress-overlay")).toBeVisible(); + }); + test("a partial/unconfirmed terminal keeps the deploy controls enabled (busy released)", async ({ page }) => { await render(page, { success: false, @@ -947,6 +990,57 @@ test.describe("STU-380 transport outcome regressions", () => { expect(exportCalls).toBe(2); }); + test("a gateway 502 keeps the project snapshot dirty instead of reporting a refusal", async ({ page }) => { + await loadDeployHooks(page); + + const zip = new JSZip(); + zip.file( + "pubspec.yaml", + "name: my_app\n\ndependencies:\n flutter:\n sdk: flutter\n", + ); + const projectZip = await zip.generateAsync({ type: "base64" }); + + let exportCalls = 0; + await page.route("**/exportCode", async (route) => { + exportCalls += 1; + await route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify({ value: { project_zip: projectZip } }), + }); + }); + // A gateway 502 can arrive after the provisioning request was forwarded — + // the runner may still be deploying classes, so this is not a refusal. + await page.route(ENDPOINTS.deployCustomClasses, async (route) => { + await route.fulfill({ status: 502, body: "bad gateway" }); + }); + + // Seed the project-source cache exactly like a deploy's pubspec merge does. + await page.evaluate(() => window.__CCC_RESOLVE_PROJECT_PUBSPEC__({})); + expect(exportCalls).toBe(1); + + const outcome = await settleAsOutcome( + page, + "__CCC_PROVISION_CUSTOM_CLASSES__", + [{ fileMap: newClassFileMap }], + ); + // The UI must not report a definitive "nothing was written". + expect(outcome.name).toBe("UnconfirmedDeployError"); + expect(outcome.outcome).toBe("unconfirmed"); + // The cause must be truthful: an HTTP 502 was actually received, so the + // message names the status and may not claim the connection dropped. The + // reconcile-before-retry advice stays in both cases. + expect(outcome.message).toContain("HTTP 502"); + expect(outcome.message).not.toContain("dropped"); + expect(outcome.message).toContain("reconcile"); + + // The possibly-written class keeps the snapshot dirty: the next deploy + // must re-export instead of planning against the cached pre-write snapshot + // and silently provisioning the class a second time. + await page.evaluate(() => window.__CCC_RESOLVE_PROJECT_PUBSPEC__({})); + expect(exportCalls).toBe(2); + }); + test("a runner failure after deploy began also drops the cached snapshot", async ({ page }) => { await loadDeployHooks(page); @@ -1214,12 +1308,18 @@ test.describe("STU-380 transport outcome regressions", () => { await page.evaluate(() => { const realFetch = window.fetch; let provisionCalls = 0; + window.__CCC_PROVISION_STREAM__ = null; window.fetch = (url, ...args) => { if (String(url).includes("deployCustomClasses")) { provisionCalls += 1; if (provisionCalls === 1) { + const body = new ReadableStream({ + start(controller) { + window.__CCC_PROVISION_STREAM__ = controller; + }, + }); return Promise.resolve( - new Response(new ReadableStream({ start() {} }), { + new Response(body, { status: 200, headers: { "Content-Type": "application/x-ndjson" }, }), @@ -1261,11 +1361,28 @@ test.describe("STU-380 transport outcome regressions", () => { await page.evaluate(() => window.__CCC_RESOLVE_PROJECT_PUBSPEC__({})); expect(exportCalls).toBe(2); await page.evaluate(() => window.__CCC_RESOLVE_PROJECT_PUBSPEC__({})); - // Second sighting has no verified credit left — it retires A's write and - // this export becomes the cached snapshot. + // The second sighting shows the same unchanged X: it is the same fact as + // the first, so it cannot retire A's uncertain write. While A can still + // land, no export may be cached. expect(exportCalls).toBe(3); await page.evaluate(() => window.__CCC_RESOLVE_PROJECT_PUBSPEC__({})); - expect(exportCalls).toBe(3); + expect(exportCalls).toBe(4); + + // A's own late result is what resolves its write: the uncertain credit + // converts to a verified one, and the next sighting consumes it — only + // then does caching resume. + await page.evaluate(() => { + const encoder = new TextEncoder(); + window.__CCC_PROVISION_STREAM__.enqueue( + encoder.encode('{"event":"result","success":true}\n'), + ); + window.__CCC_PROVISION_STREAM__.close(); + }); + await page.waitForTimeout(50); + await page.evaluate(() => window.__CCC_RESOLVE_PROJECT_PUBSPEC__({})); + expect(exportCalls).toBe(5); + await page.evaluate(() => window.__CCC_RESOLVE_PROJECT_PUBSPEC__({})); + expect(exportCalls).toBe(5); }); test("a push still in flight keeps exports uncached until it settles", async ({ page }) => { diff --git a/src/provisionStream.js b/src/provisionStream.js index 6fa09fa..a408871 100644 --- a/src/provisionStream.js +++ b/src/provisionStream.js @@ -93,13 +93,12 @@ export async function readProvisionResponse(response, handlers = {}) { return { success: false, finalResultReceived: false, - // An explicit non-2xx HTTP response is a definitive server refusal, not an - // unknown outcome. The caller must render it as FAILED; only an ok response - // whose stream dropped before a result leaves the remote state genuinely - // unknown (UNCONFIRMED). This holds for the deploy runner: once streaming - // begins it emits every post-work failure as a `result` event under the - // already-sent 200 status, so a non-2xx can only arrive before the first - // remote write. + // An explicit non-2xx HTTP response. A 4xx proves the request was turned + // away before any work, so the caller renders it as FAILED. A 5xx proves + // no such thing: a gateway can answer 502/503/504 after forwarding the + // request to the runner, which may already be deploying (or may have + // deployed) classes — so a 5xx is UNCONFIRMED, exactly like an ok response + // whose stream dropped before a result. httpRejected: !response.ok, httpStatus: response.status, runnerPhase: lastPhase, @@ -150,11 +149,13 @@ export function isPreWriteRunnerFailure(result) { * Maps a provisioning stream result to the single truthful terminal outcome. * * When the runner delivered a definitive result, the ordinary classifier - * applies. When it did not (`finalResultReceived === false`), the two cases the - * caller must never flatten are separated: an explicit HTTP rejection (403/5xx) - * is a definitive refusal — the write did not happen — so it is FAILED, while an - * ok response whose stream simply dropped (or a client wait expiry) leaves the - * remote outcome unknown and is UNCONFIRMED. + * applies. When it did not (`finalResultReceived === false`), the cases the + * caller must never flatten are separated: a 4xx response is a definitive + * refusal — the request was turned away before any write — so it is FAILED, + * while a 5xx is not proof of anything. A gateway can return 502/503/504 + * after forwarding the request to the runner, so classes may already be + * deploying; that outcome, an ok response whose stream simply dropped, and a + * client wait expiry all leave the remote outcome unknown (UNCONFIRMED). * * This is the decision `provisionMissingCodeFiles` uses in the real deploy * path, so a test of this function drives the same code that renders a 403 @@ -175,7 +176,15 @@ export function deployOutcomeOfStreamResult(result) { : outcome; } if (result?.httpRejected) { - return DeployOutcome.FAILED; + // Only a 4xx proves the write was refused before it began. A 5xx can come + // from a gateway after the request reached the runner, so it must stay + // UNCONFIRMED: reporting it as FAILED would tell the user nothing was + // written while the runner is still deploying. + return typeof result.httpStatus === "number" && + result.httpStatus >= 400 && + result.httpStatus < 500 + ? DeployOutcome.FAILED + : DeployOutcome.UNCONFIRMED; } return DeployOutcome.UNCONFIRMED; } diff --git a/src/provisionStream.test.js b/src/provisionStream.test.js index eff319c..5b5a5f6 100644 --- a/src/provisionStream.test.js +++ b/src/provisionStream.test.js @@ -103,12 +103,11 @@ test("treats an empty body as a failure", async () => { // --- STU-380: an explicit HTTP rejection is FAILED, never UNCONFIRMED -------- // These drive the real stream handler (`readProvisionResponse`) against a true -// Response object with a non-2xx status and no streamed "result" event — the -// exact shape the deploy runner produces when it refuses the request before -// doing any work (API key lacks write access, transient 5xx, …). The -// mislabelling bug lived here: that shape was previously thrown as an -// UnconfirmedDeployError, telling the user to reconcile in FlutterFlow when the -// server had plainly refused the write. +// Response object with a non-2xx status and no streamed "result" event. A 4xx +// is the shape the deploy runner produces when it refuses the request before +// doing any work (API key lacks write access, bad request, …): the write was +// turned away, so it is FAILED. A 5xx is not a refusal — a gateway can fail +// after forwarding the request to the runner — so it is UNCONFIRMED. function provisionAtStatus(chunks, status) { const body = new ReadableStream({ @@ -173,7 +172,7 @@ test("an explicit HTTP rejection (403) is a failure, not unconfirmed", async () assert.notEqual(DeployOutcome.FAILED, DeployOutcome.UNCONFIRMED); }); -test("a 5xx server error with no streamed result is also FAILED, not unconfirmed", async () => { +test("a 5xx server error with no streamed result is UNCONFIRMED — a gateway can fail after forwarding", async () => { const result = await readProvisionResponse( new Response( '{"event":"phase","phase":"connecting","message":"Connecting..."}\n', @@ -181,12 +180,34 @@ test("a 5xx server error with no streamed result is also FAILED, not unconfirmed ), ); assert.equal(result.httpRejected, true); + assert.equal(result.httpStatus, 502); + // A 502 can come from a gateway after the request reached the runner, which + // may be deploying classes already — so it cannot prove nothing was written. assert.equal( deployOutcomeOfStreamResult(result), - DeployOutcome.FAILED, + DeployOutcome.UNCONFIRMED, + ); + + // Every 5xx shares that uncertainty: the server failed, but none of them + // proves the request was turned away before a write. + const unavailable = await readProvisionResponse( + new Response("", { status: 503 }), + ); + assert.equal( + deployOutcomeOfStreamResult(unavailable), + DeployOutcome.UNCONFIRMED, ); }); +test("a 4xx with no streamed result stays a definitive refusal, not unconfirmed", async () => { + const result = await readProvisionResponse( + new Response("no", { status: 400 }), + ); + // A 4xx is validation/auth — the request was turned away before any work. + assert.equal(result.httpRejected, true); + assert.equal(deployOutcomeOfStreamResult(result), DeployOutcome.FAILED); +}); + test("a dropped stream on an OK response stays UNCONFIRMED (outcome truly unknown)", async () => { const result = await readProvisionResponse( provisionAtStatus(