Skip to content

Gate fixes: deploy truthfulness for gateway errors, repeated exports and progress overlay (PR #100 findings) - #101

Merged
sgardoll merged 5 commits into
milestone/stu-implement-updated-ui-uxfrom
sgardoll/gate-deploy-truthfulness
Sep 28, 2026
Merged

sgardoll merged 5 commits into
milestone/stu-implement-updated-ui-uxfrom
sgardoll/gate-deploy-truthfulness

Conversation

@sgardoll

Copy link
Copy Markdown
Owner

Findings addressed

G1 — Greptile P1, "Gateway errors hide uncertain writes" (src/provisionStream.js:177-179):

If a gateway returns 502 after forwarding the provisioning request but before delivering the runner's result, this code treats it as proof that nothing was written. The runner may still be deploying classes, yet the UI reports failure and keeps the cached project snapshot; a retry can provision those classes again. [Unknown remote outcomes must remain unconfirmed and require reconciliation before retrying.]

Fixed in deployOutcomeOfStreamResult: only a 4xx is still a definitive pre-write refusal (FAILED). Every 5xx is now UNCONFIRMED — a gateway can answer 502/503/504 after the request reached the runner. The caller's existing UNCONFIRMED path then invalidates the project-source cache and marks the files dirty (markProjectSourceDirty), so a retry must re-export and cannot silently re-provision.

G2 — Greptile P1, "Repeated exports clear pending writes" (app.js:3086-3089):

After a confirmed retry writes the same file content, one export uses its verified credit. A second export of that unchanged content then clears the earlier uncertain write, even if its request is still running. The snapshot can be cached, and a later write from the first request can leave the next deploy planning against stale files.

Fixed in resolveProjectPubspec's dirty-sighting loop: an export sighting that consumes a verified credit is marked explainedByVerified on the record. A later export of the same unchanged content is the same fact, not a new landing, so it can no longer retire the uncertain credit. The uncertain write is resolved only by its own late result (success → converts to a verified credit consumed by the next sighting; provable pre-write failure → retired), so the snapshot stays uncached while the write may still land.

D1 — Devin, "Previous deploy hides active progress" (app.js:6744-6746):

When another deploy starts within one second, the prior commitStateChange timer closes its active progress overlay. The remote write continues without a protected progress view.

Fixed in the commitStateChange listener: the terminal state's 1s hide timer is now tracked in commitProgressHideTimer and cleared when a new deploy reports PREPARING/VALIDATING/PUSHING, so a stale timer can no longer close the new deploy's overlay.

Fail → pass evidence (base head 6dd8edb)

The source files (app.js, src/provisionStream.js) were reverted to base head while the new tests were kept; the same commands were then re-run on the fixed head. No other files differ.

Base head — unit:

$ CI=1 npm test
not ok 295 - a 5xx server error with no streamed result is UNCONFIRMED — a gateway can fail after forwarding
# tests 355
# pass 354
# fail 1

Base head — e2e (--retries=0):

$ CI=1 CCC_TEST_PORT=3104 npx playwright test e2e/stu380-deploy.spec.js \
    -g "gateway 502 keeps|identical retry's success|new deploy within the terminal hide"
  ✘ 1 … a new deploy within the terminal hide delay keeps its progress overlay
      Error: expect(locator).toHaveClass(expected) failed
      Expected pattern: /\bopen\b/
      Received string:  "modal-overlay"          ← stale timer removed .open
  ✘ 2 … a gateway 502 keeps the project snapshot dirty instead of reporting a refusal
      Expected: "UnconfirmedDeployError"
      Received: "Error"                          ← base called it a definitive refusal
  ✘ 3 … an identical retry's success cannot retire the earlier uncertain write
      Expected: 4
      Received: 3                                ← base cached the snapshot on the unchanged sighting
  3 failed

Fixed head b7830b8:

$ CI=1 npm test
# tests 355
# pass 355
# fail 0

$ CI=1 CCC_TEST_PORT=3104 npx playwright test e2e/stu380-deploy.spec.js \
    -g "gateway 502 keeps|identical retry's success|new deploy within the terminal hide"
  ✓ 1 … a new deploy within the terminal hide delay keeps its progress overlay
  ✓ 2 … a gateway 502 keeps the project snapshot dirty instead of reporting a refusal
  ✓ 3 … an identical retry's success cannot retire the earlier uncertain write
  3 passed

The break each test catches:

  • G1 unit test: reverting deployOutcomeOfStreamResult to FAILED for all httpRejected fails the test (observed above).
  • G1 e2e: the 502 must produce UnconfirmedDeployError and force a fresh export (exportCalls 1→2); base throws a plain Error and leaves the cached snapshot.
  • G2 e2e: the second sighting of unchanged content must not cache (exportCalls 4, not 3); a late success from the first attempt then resolves it and caching resumes at 5.
  • D1 e2e: after SUCCESS + a new PREPARING inside the hide delay, #commit-progress-overlay still carries .open past the 1s timer and 0.25s visibility transition.

CI results on pushed head b7830b8

Command Result
CI=1 npm test 355 passed / 0 failed
CI=1 npm run build ✓ built
CI=1 CCC_TEST_PORT=3104 npm run test:ui 208 tests: 207 passed + 1 flaky on retry (stu389-hero-mark-field WebGL timing, unrelated; passes 3/3 in isolation with --retries=0 --repeat-each=3)
CI=1 npm run test:accessibility 7 shells passed
CI=1 npm run test:deploy-guard 9/9 passed
CI=1 npm run test:proxies 30/30 passed
CI=1 python3 scripts/scan_bundle_for_leaks.py clean

Functions edited (line ranges are post-change)

  • app.js resolveProjectPubspec — dirty-sighting loop at lines 3080-3104; and the projectSourceDirtyWrites model comment at lines 2673-2684 (same bookkeeping feature).
  • app.js DOMContentLoaded callback's commitStateChange listener — lines 6738-6772 (timer declaration + listener).
  • src/provisionStream.js deployOutcomeOfStreamResult — lines 148-190; httpRejected comment in readProvisionResponse — lines 93-104.
  • Tests: src/provisionStream.test.js (two tests updated/added), e2e/stu380-deploy.spec.js (one new G1 test, one new D1 test, G2 test extended).

No other region of app.js was touched. git status after restoring generated artifacts shows only these four files modified.

Not implemented / out of scope

  • The 5xx UNCONFIRMED message text in provisionMissingCodeFiles (app.js ~2940) is outside this brief's ownership. A gateway 502 now surfaces the existing unconfirmed wording ("connection … dropped before it reported a result … may still be finishing … reconcile before retrying"), which does not claim nothing was written, but it does not name the HTTP status.
  • src/deployOutcome.js was not touched: its FAILED doc already states that 5xx are UNCONFIRMED, which the fix now honours.
  • No mechanism retires an uncertain credit when an attempt never reports any result; by design the snapshot stays uncached for the session rather than caching a snapshot that ignores a possibly-written file.

Judgement calls / uncertainties

  • All 5xx, not just 502/503/504, classify as UNCONFIRMED: none of them proves the request was turned away before a write, and the codebase's stated rule is that unknown remote outcomes stay unconfirmed.
  • explainedByVerified is deliberately conservative: once a verified write has explained a content sighting, repeated sightings of that content can never retire the uncertain credit — even if the content later changes away and back. Only the attempt's own result resolves it.
  • The D1 test asserts the .open class rather than computed visibility because the overlay's visibility transition (0.25s) is still mid-flight at the 1s hide-delay boundary; the class is the settled evidence that the timer ran.

…thful

A 5xx from the provisioning endpoint no longer classifies as a definitive
pre-write refusal: a gateway can answer 502/503/504 after forwarding the
request, so the outcome stays UNCONFIRMED and the possibly-written files stay
dirty until an export proves them (only 4xx is still FAILED).

An export sighting already explained by a verified write is spent evidence: a
second export of the same unchanged content can no longer retire an earlier
uncertain write whose identical content may still land, so the snapshot stays
uncached until that attempt reports its own result.

A new deploy starting inside the terminal state's 1s hide delay cancels the
previous deploy's hide timer instead of having its progress overlay closed by
it.
@sgardoll
sgardoll marked this pull request as ready for review September 28, 2026 06:54
@greptile-apps

greptile-apps Bot commented Sep 28, 2026

Copy link
Copy Markdown
Contributor

This PR does not match any of the 1 configured review trigger rule.

devin-ai-integration[bot]

This comment was marked as resolved.

provisionMissingCodeFiles' no-final-result branch still described a
403/5xx `response.ok === false` as a definitive pre-write refusal.
Only a 4xx is FAILED now; a 5xx stays UNCONFIRMED because a gateway can
answer 502/503/504 after forwarding the request to the runner.
Comment-only: no behavior change.
…esult

provisionMissingCodeFiles reported a no-result 5xx as "the connection to
the FlutterFlow deploy runner dropped before it reported a result" — but
an HTTP 502 was actually received, and the status was discarded. The
cause sentence was false for exactly the case the 5xx handling exists
for.

The unconfirmed message now says the runner returned HTTP <status>
without reporting a result when a non-ok status arrived, and keeps the
dropped-connection wording only when an ok response stream ended with no
result. The reconcile-before-retry advice is unchanged in both.

The 502 e2e test now pins the truthful text: it must name HTTP 502 and
must not claim the connection dropped.
…-truthfulness

Resolve the app.js commitStateChange conflict in favour of the base
side, which owns the delayed-hide timer at module scope behind
cancelCommitProgressHide() (also called from commitProgress.start()).
The PR's function-local copy of the timer is dropped: it would shadow
the module-scope handle, so the start-path cancel and the terminal
branch would operate on two different variables.

Keep the PR's one-line timer clear on the terminal branch. Base only
cancels a pending hide when a commit *starts*, so two terminal states
in quick succession leave the first timer pending; it fires early,
nulls the shared handle, and a later cancelCommitProgressHide() can no
longer reach the second timer.

@devin-ai-integration devin-ai-integration Bot left a comment

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.

Devin Review found 2 new potential issues.

Devin Review

Comment thread app.js
Comment on lines +3111 to +3112
} else if (!record.explainedByVerified) {
record.uncertain -= 1;

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.

Comment thread app.js
Comment on lines +2955 to +2958
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.";

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.

@sgardoll
sgardoll merged commit 01a11be into milestone/stu-implement-updated-ui-ux Sep 28, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant