Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 37 additions & 19 deletions app.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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.";
Comment on lines +2955 to +2958

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 Gateway errors retain timeout wording

An immediate HTTP 502 now reports its status, but showCommitUnconfirmedModal still says the browser stopped waiting. The terminal title reads like a timeout despite a received response.

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

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.",
);
}
Expand Down Expand Up @@ -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;
Comment on lines +3111 to +3112

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Repeated exports erase pending retries

When two identical provisions remain unconfirmed, resolveProjectPubspec retires both after repeated exports show one landing. The unchanged content consumes an uncertain credit on every export, so a later retry can overwrite a newer edit behind a cached snapshot.

Learn more

Provisioning attempts that time out stay tracked as uncertain because the runner can still write after the browser stops waiting. markProjectSourceDirty increments the uncertain count for every attempt, including retries of the same file content. If two such attempts exist, a single remote write produces one observable content transition. Every fresh export of that unchanged content then decrements the count again, even though the other attempt can still run. resolveProjectPubspec caches the export once the count reaches zero, hiding a later write from subsequent deploy planning.

Example: Attempts A and B both time out while provisioning gauge_model.dart with content X. A writes X; two exports showing X clear both credits. A new edit Y is deployed, but B later writes X, and a subsequent deploy reads the cached snapshot instead of X.

Recommended fix: Track whether an observed path/content transition has already been used to retire an uncertain attempt, not only whether a verified write explained it. Allow another retirement only after a distinct observable transition or a definitive late result for that attempt. Preserve the existing verified credit handling for confirmed retries.

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

}
if (record.uncertain <= 0 && record.verified <= 0) {
contents.delete(observed);
}
Expand Down Expand Up @@ -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();
Expand Down
125 changes: 121 additions & 4 deletions e2e/stu380-deploy.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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" },
}),
Expand Down Expand Up @@ -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 }) => {
Expand Down
35 changes: 22 additions & 13 deletions src/provisionStream.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand All @@ -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;
Comment thread
devin-ai-integration[bot] marked this conversation as resolved.
Resolved
}
return DeployOutcome.UNCONFIRMED;
}
37 changes: 29 additions & 8 deletions src/provisionStream.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down Expand Up @@ -173,20 +172,42 @@ 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',
{ status: 502 },
),
);
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(
Expand Down
Loading