Skip to content

fix(deploy): keep gateway 5xx, repeated sightings and new deploys truthful (r2) #107

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
sgardoll merged 1 commit into from
Sep 28, 2026
Merged

fix(deploy): keep gateway 5xx, repeated sightings and new deploys truthful (r2) #107

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
55 changes: 36 additions & 19 deletions app.js
View file
Open in desktop
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.";
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

@devin-ai-integration devin-ai-integration Bot Sep 28, 2026

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 clear pending class writes

With two uncertain writes of identical content, resolveProjectPubspec retires one on each unchanged export. The second write can land later, after its dirty entry has cleared and the stale snapshot was cached.

Learn more

The dirty ledger tracks uncertain provisioning attempts by file path and content. An export showing that content establishes at most one observable transition; repeated exports of the same snapshot do not establish that another pending attempt wrote it. The explainedByVerified flag only spends sightings attributed to verified writes. When no verified credit exists, repeated sightings can each decrement uncertain, emptying the ledger and enabling caching while a runner can still write.

Example: Two requests for class X time out. The first writes X; two consecutive exports both show X. Both uncertain credits disappear, and an export is cached. The second request then writes X over a newer edit, while the cache still describes the earlier project state.

Recommended fix: Track the last observed content or distinct landing evidence per path, including sightings consumed by uncertain writes. Never retire another uncertain credit on an unchanged export; preserve uncertainty where identical writes cannot be distinguished.

Devin Review

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

Comment on lines +3110 to +3112

@devin-ai-integration devin-ai-integration Bot Sep 28, 2026

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.

🟡 Returned class content keeps exports uncached

After a verified X sighting, explainedByVerified stays true when an export shows another version. If a pending write then restores X, its new sighting cannot retire the dirty entry, so every deploy re-exports.

Learn more

The project source cache is bypassed while any class write has an outstanding dirty entry. A verified write of X sets explainedByVerified on the X record, but observations of any other content do not reset it. If another edit replaces X with Y and the pending X write later restores X, the ledger rejects that distinct X sighting. It can remain dirty for the rest of the session, forcing a complete project export on each deploy.

Example: A timed-out provision of X is followed by a confirmed retry of X. An export sees X and consumes the confirmed credit. A later sync changes the class to Y, then the original provision writes X. Exports now show X again, but the pending X credit never clears.

Recommended fix: Track the last observed content per path and allow a new transition back to X to supply fresh evidence. Do not reset the evidence solely because time passes or the same snapshot is exported again.

Devin Review

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

}
if (record.uncertain <= 0 && record.verified <= 0) {
contents.delete(observed);
}
Expand Down
82 changes: 78 additions & 4 deletions e2e/stu380-deploy.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -947,6 +947,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 +1265,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 +1318,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
View file
Open in desktop
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;
}
return DeployOutcome.UNCONFIRMED;
}
37 changes: 29 additions & 8 deletions src/provisionStream.test.js
View file
Open in desktop
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