Skip to content

fix(deploy): finish an interrupted deploy whose classes landed with a pubspec-only push - #115

Merged
sgardoll merged 1 commit into
hardening/deploy-resiliencefrom
hardening/deploy-durability
Oct 4, 2026
Merged

sgardoll merged 1 commit into
hardening/deploy-resiliencefrom
hardening/deploy-durability

Conversation

@sgardoll

@sgardoll sgardoll commented Oct 4, 2026 •

Copy link
Copy Markdown
Owner

Summary

A deploy whose step-2 response is lost or expired used to leave the classes written to FlutterFlow but the pubspec.yaml push (step 3) never run, so the project could not build. Commit 5231e76 added reconcileUnconfirmedProvision, which re-reads the project after an unconfirmed provision and reports which classes landed. This PR extends that: when the reconcile proves every attempted class landed, the unconfirmed terminal now offers a "Finish deploy" action that runs step 3 only — the pubspec push — without re-running the class write.

Item 1 — the "Finish deploy" action

  • src/flutterFlowCodeFileProvisioning.js: two new pure helpers, isDeployFinishable(partition) (true only when every attempted class landed) and buildFinishPushFileMap(fileMap, landedEntries, serializedYaml) (builds the finish-push file map: the landed classes excluded, the merged pubspec added).
  • app.js:
    • reconcileUnconfirmedProvision returns the error carrying reconcilePartition (landed/notLanded), and the all-landed message now points at "Finish deploy".
    • provisionMissingCodeFiles's unconfirmed branch, when finishable, attaches finishDeploy — the deploy-time creds, the full file map, the already-merged serializedYaml (produced by resolveProjectPubspec — reused, not re-invented), and the landed entries.
    • executeCommit and executeBundleCommit merge that onto the unconfirmed result via mergeFinishDeployContext.
    • showCommitUnconfirmedModal offers a "Finish deploy" button in the terminal modal (same place as the existing failure guidance) when result.finishDeploy is present.
    • finishProvisionedDeploy runs the async finish: rebuilds the deploy-time API client, builds the pubspec-only push via buildFinishPushFileMap, reuses buildApiSyncMetadata / createZipFromFileMap / pushCodeWithUiTimeout, and renders the result through the normal renderCommitTerminal. The landed classes are excluded from the sync map, so they are never re-provisioned.
  • Tests: 3 new unit tests in src/flutterFlowCodeFileProvisioning.test.js drive the exact pure decision path app.js uses after a timed-out step 2 — re-read reports all landed → finishable → finish file map carries the merged pubspec and omits the landed classes.

Testability note (per the workstream contract): app.js is a Vite-browser monolith with no end-to-end unit path (DOM, fetch, the Cloud Run runner, and live FlutterFlow credentials), so I factored the finish decision + file-map construction into pure functions in src/ and unit-tested those. The app.js wiring is verified by node --check, a clean vite build, and the full node --test src/*.test.js suite (402 pass). I am not claiming end-to-end app.js coverage.

Item 2 — can the pubspec be pushed before/with the classes? (decided: leave the order alone)

Evidence based on reading the code:

  1. The class write (step 2, deployCustomClasses) and the pubspec write (step 3, syncCustomCodeChanges) go through different endpoints. They cannot share a single call; "together with" is not achievable.
  2. Pushing the pubspec first would mean issuing it as a separate, independent syncCustomCodeChanges request through its own retry-with-uncertain-outcome machinery (pushCodeWithUiTimeout). That request has its own lost-response/unconfirmed failure modes, so it would add a second write we cannot safely confirm — making the unconfirmed window worse, not better.
  3. The compile gate already sees the merged dependencies: provisionMissingCodeFiles sends the runner a verification manifest built from the merged serialized_yaml, so classes are compiled against the new dependencies regardless of push order. Reordering buys nothing for the compile gate.
  4. Pushing the pubspec first would also create the converse partial state (pubspec written, classes not yet), which is harder to reconcile.
  5. The failure window item 1 addresses is fully covered by "Finish deploy": on a lost step-2 response with classes landed, the user pushes the pubspec without re-deploying; a lost step-3 response is already handled by the normal unconfirmed reconcile + retry.

Conclusion: reordering is not safe enough to force. Item 1 is the mitigation, and this PR implements it.

Out of scope / notes

  • The legacy commitToFlutterFlow path (not used by the current UI, which routes through executeCommit/executeBundleCommit) has no targetIdentity, so it still returns the plain unconfirmed result without a Finish button; it degrades gracefully to the existing unconfirmed guidance.
  • The finish push passes an empty remote-files map to buildApiSyncMetadata, which is correct because after an all-landed reconcile the finish file map is pubspec-only in the normal single/multi-artifact fresh-deploy case (type "D" is excluded from the file_map), matching what a successful post-provision step-3 already sends.

Verification

  • node --test src/*.test.js → 402 passing (399 baseline + 3 new), 0 failing
  • node --check app.js → OK
  • npm run build (vite) → builds clean, 42 modules

Devin Review

…th a pubspec-only push

When step 2 (the class write) succeeds but its response is lost or expires,
the classes land in FlutterFlow but step 3 (the pubspec.yaml push) never
runs, leaving the project unable to build. Now that commit 5231e76 re-reads
the project after an unconfirmed provision, extend the terminal: when the
reconcile proves every attempted class landed, offer a "Finish deploy" action
that pushes the already-merged pubspec (reusing resolveProjectPubspec's output)
without re-running the class write. The landed classes are excluded from the
finish file map so they are never re-provisioned.
@greptile-apps

greptile-apps Bot commented Oct 4, 2026

Copy link
Copy Markdown
Contributor

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

@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 4 potential issues.

Devin Review

Comment thread app.js
// needs — the deploy-time credentials, the full file map, the already
// merged yaml, and the landed entry list — so the terminal action can
// push without re-provisioning the classes.
if (isDeployFinishable(reconciled.reconcilePartition)) {

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.

🔴 Failed reconciliation falsely marks deploy failed

When the project re-read fails, isDeployFinishable receives no partition and throws. The original unconfirmed outcome becomes a failed deploy, although classes can still land.

Suggested change
if (isDeployFinishable(reconciled.reconcilePartition)) {
if (reconciled.reconcilePartition && isDeployFinishable(reconciled.reconcilePartition)) {

Devin Review


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

Comment thread app.js
zipped_custom_code: zippedCustomCode,
uid: `web_${Date.now()}`,
branch_name: apiClient.branchName,
serialized_yaml: finish.serializedYaml,

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.

🔴 Delayed finish overwrites newer project dependencies

If dependencies change after reconciliation, finishProvisionedDeploy pushes its earlier complete YAML snapshot. Newer project dependencies disappear from FlutterFlow.

Learn more

The normal deploy merges generated dependencies with the current project manifest in resolveProjectPubspec. The finish action instead retains that earlier merged manifest and sends it as the complete serialized_yaml. The user can leave the terminal open while another deploy or project editor adds packages, and the finish push replaces that newer manifest. Its bundle file map can also contain non-class artifacts created before the delay.

Example: Deploy A prepares a manifest containing intl, then provisioning times out and the Finish button appears. Deploy B adds http; clicking Finish for A uploads A's older manifest and removes http.

Recommended fix: Before constructing the finish request, fetch the current project manifest and merge only the interrupted deploy's required dependencies into it. Also reconcile remaining bundle artifacts against the fresh project export before writing them.

Devin Review


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

Comment thread app.js
finish.landed,
finish.serializedYaml,
);
const syncMetadata = await buildApiSyncMetadata(finishFileMap, new Map());

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.

🔴 Existing bundle artifacts lose update metadata

For bundles with existing actions, widgets, or functions, buildApiSyncMetadata treats retained files as new. The empty remote map drops original checksums and marks existing functions for addition, so the finish push can fail.

Learn more

The finish map removes only classes that the runner attempted to create. Other bundle entries can already exist in FlutterFlow. buildFlutterFlowSyncMetadata consults remote files to attach original_checksum to updates and to avoid adding functions that already exist. Unlike the normal bundle push, Finish supplies an empty map, even when these remote entries are present.

Example: A bundle has a new model.dart and an existing read_tag.dart. After model.dart lands, Finish retains read_tag.dart but omits its original checksum; FlutterFlow receives a create-shaped update instead of an update.

Recommended fix: Fetch fresh remote files at finish time and pass them to buildApiSyncMetadata; reconcile their contents and handle concurrent edits before constructing the push.

Devin Review


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

Comment thread app.js
Comment on lines +7967 to +7974
} catch (error) {
renderCommitTerminal({
...finish.result,
success: false,
error: error.message,
unconfirmed: error instanceof UnconfirmedDeployError,
remoteRefusal: error.remoteRefusal === true,
});

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.

🔍 Failed finish obscures landed classes

A rejected finish push displays a total failure, although reconciliation already found the classes in FlutterFlow. The terminal tells users nothing was applied.

Devin Review


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

@sgardoll
sgardoll merged commit fbb4d83 into hardening/deploy-resilience Oct 4, 2026
1 of 2 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