Repository navigation
feat(runner): record deploy runs and serve their status so a lost outcome is known - #118
Conversation
…come is known The last P0 of deploy resilience: when a deploy outlives the browser's wait bound or its connection drops, the only answer the browser had was to re-read the whole project and estimate whether classes landed. This makes the outcome known rather than guessed by giving the runner a per-run record and a status endpoint the client can ask. The client generates the run id and sends it with the deploy request, which is what makes "never arrived" distinguishable from "unknown": a 404 on the status endpoint for an id the client itself minted proves the request never reached the runner, so nothing was written and a retry is safe. On an unconfirmed provision the client polls the status endpoint under a bounded budget and, when the record is definitive (clean done, or a provably pre-write failure), reports the REAL outcome instead of the reconcile estimate; a 404, a 5xx or a malformed body falls back to the existing project re-read, so an older runner keeps working untouched. The record is stored in Firestore in project low-code-connect through the REST API with an OAuth token from the GCE metadata server over dart:io - no runtime pub dependency, given the image build is already slow. The API key is never stored, only its SHA-256 digest (dependence-free, pinned to FIPS-180-4 vectors), and the status endpoint requires the caller to present the same key, so a run id alone is never a bearer capability over --allow-unauthenticated. Writing the record can never fail a deploy: each Firestore call is bounded and its failure logged, not propagated; the terminal write is awaited under a short cap so a client that lost the response reliably sees the run finish. The status endpoint's auth rejection and 404 for an unknown id, the record codec, the never-throw recorder and the client's fallback when the endpoint is absent or 404s are all unit-tested without Firestore.
|
This PR does not match any of the 1 configured review trigger rule. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| void recordPhase(String phase, String message) { | ||
| _phase = phase; | ||
| _write({'status': 'running', 'phase': phase, 'message': message}); | ||
| } |
There was a problem hiding this comment.
🔴 Completed runs revert to running
When a phase write finishes after recordDone or recordFailed, it restores status: running. recordPhase sends independent, unawaited PATCH requests, so lost responses can remain unresolved after the deploy finishes.
Learn more
Each phase and terminal state updates the same Firestore document. recordPhase starts an independent asynchronous PATCH, while the terminal write only awaits its own PATCH. Firestore applies requests in arrival order, not invocation order. A delayed phase PATCH can therefore overwrite a finished run with running, and later status reads never recover the completed outcome.
Example: The deploy_start PATCH stalls, the CLI uploads GaugeWidget, and the done PATCH completes. The stalled PATCH then succeeds, returning the stored status to running even though GaugeWidget was deployed.
Recommended fix: Serialize writes per RunRecorder, or use an atomic store-side condition that refuses nonterminal updates once a terminal status is stored. Ensure terminal writes wait for or supersede every earlier phase write, including those that timed out locally.
Was this helpful? React with 👍 or 👎 to provide feedback.
| if (statusOutcome.definitive === "failed") { | ||
| // A provable pre-write refusal: nothing could have been written, so it | ||
| // is a real failure, not an unknown outcome. | ||
| runStatusResolved = true; | ||
| throw new Error( | ||
| statusOutcome.error || "The deploy failed before writing anything.", | ||
| ); |
There was a problem hiding this comment.
🟡 Pre-write failures leave project exports uncached
When a lost response resolves to failed, its attempted classes remain marked dirty despite never being written. resolveProjectPubspec cannot clear absent classes, so later deploys repeatedly export the project.
Learn more
The unconfirmed-error handler calls markProjectSourceDirty before querying status. The failed status is definitive only when the runner stopped before any class write, so that attempt cannot create a later file. The new failure branch leaves its uncertain credits in projectSourceDirtyWrites. resolveProjectPubspec removes uncertain credits only after observing matching content, so a file that was never written keeps project exports uncached indefinitely.
Example: A compile check rejects GaugeWidget, but its stream disconnects. Status confirms a pre-write failure. Future deploys see no GaugeWidget in FlutterFlow and fetch a new full export every time, instead of reusing the cached project source.
Recommended fix: On a definitive pre-write failure, decrement that attempt's uncertain credits with adjustProjectSourceDirty before throwing. Preserve credits from other concurrent attempts at the same path and content.
Was this helpful? React with 👍 or 👎 to provide feedback.
| return { | ||
| remoteFiles, | ||
| syncFileMap: excludeProvisionedCodeFiles(fileMap, missingCodeFiles), | ||
| unverified, | ||
| approximate, | ||
| provisionSucceeded: true, | ||
| }; |
There was a problem hiding this comment.
🟡 Recovered deploys lose compile-check warnings
When verification fails to run and the provision response is lost, statusOutcome reports success without the runner's verificationSkipped warning. The deploy finishes without telling the user their classes were not compiled.
Learn more
The runner returns verificationSkipped when the compile check cannot run. The normal provisioning result appends that warning to unverified before returning it. The recovered-success branch bypasses the result and returns only the pre-request warnings, while recordDone does not persist the skip reason.
Example: Flutter package resolution fails, then GaugeWidget deploys successfully but its stream drops. A done status recovers the deploy, yet the user receives no warning that GaugeWidget was never compiled.
Recommended fix: Persist the runner's verification outcome in its run record and return it through the status endpoint. Add any recovered skip reason to unverified just as the normal result path does.
Was this helpful? React with 👍 or 👎 to provide feedback.
| // Never allow a run-record write that follows the catch to take down the | ||
| // response - it is best effort, and no runId is parsed for a request that | ||
| // threw before it (request body/format errors). |
There was a problem hiding this comment.
🟡 Unexpected runner failures leave runs active
If the CLI fails to start, the catch returns an error but never marks the recorded run terminal. A lost response leaves status: running after the runner has stopped.
Learn more
After request validation, the deploy path records deploy_start before starting the CLI process. An exception from Process.start, script creation, or another later operation reaches the outer catch, which sends HTTP 500 but never calls recordFailed. If the response is lost, the stored record remains running forever, even though the request is over.
Example: The FlutterFlow executable is missing, so starting the deploy process throws. The HTTP 500 response disconnects before the browser reads it; status polling sees running until its budget expires.
Recommended fix: Keep the recorder accessible in the outer catch and write a terminal failure for every exception after recording begins. Preserve the phase so failures after deploy_start stay unconfirmed rather than being mislabeled pre-write.
Was this helpful? React with 👍 or 👎 to provide feedback.
| /// The Firestore collection (a sub-path of `databases/(default)/documents`) | ||
| /// that holds deploy-run records, keyed by the client-generated run id. | ||
| const runStatusCollection = 'deploy_runs'; |
There was a problem hiding this comment.
🔍 Confirm run-record retention
Each deploy creates a Firestore record, with no cleanup in this change. Check whether an external TTL policy bounds collection growth.
Was this helpful? React with 👍 or 👎 to provide feedback.
| /// Semantics the browser relies on: | ||
| /// - 404 for an id it generated itself proves the request never reached the | ||
| /// runner, so nothing was written and a retry is safe. | ||
| /// - 401/403 when the key is absent or does not match. | ||
| /// - 503 when Firestore itself cannot be read (the browser treats that as | ||
| /// "unknown" and falls back to re-reading the project). |
There was a problem hiding this comment.
🔍 Clarify what a missing record proves
A 404 does not prove the runner never received a request: record writes can fail independently. The client treats it as unknown, so update the stated status contract before other callers rely on that claim.
Was this helpful? React with 👍 or 👎 to provide feedback.
| final query = request.uri.queryParameters['key']?.trim(); | ||
| if (query != null && query.isNotEmpty) return query; |
There was a problem hiding this comment.
🟥 Status URLs can expose project API keys
When callers supply key in the status URL, proxy access logs can capture the FlutterFlow API key. Anyone with that key can access the corresponding project.
Was this helpful? React with 👍 or 👎 to provide feedback.
| final statusPath = RegExp( | ||
| r'^/runStatus/([A-Za-z0-9_-]+)$', | ||
| ).firstMatch(request.uri.path); | ||
| if (request.method == 'GET' && statusPath != null) { | ||
| await _handleRunStatus(request, statusPath.group(1)!); |
There was a problem hiding this comment.
🟨 Status lookup accepts unbounded run identifiers
A GET request can supply an arbitrarily long runId despite the POST's 120-byte limit. This forces unnecessary parsing and Firestore requests on an unauthenticated service.
Was this helpful? React with 👍 or 👎 to provide feedback.
Summary
Closes the last P0 of deploy resilience: when a deploy outlives the browser wait bound or its connection drops, the browser could only re-read the whole project and guess whether classes landed. This gives the runner a per-run record + GET /runStatus/ so a lost outcome is known rather than estimated.
How it works
runId) with the deploy request. This is what makes "never arrived" provable: a 404 on the status endpoint for an id the client itself minted means the request never reached the runner, so nothing was written and a retry is safe.low-code-connect, REST API + OAuth token from the GCE metadata server overdart:io— no new runtime pub dependency). The record is updated as the run progresses (workspace_init/verifying/deploy_start/ phase markers / terminaldone|failed, per-class results, error string).x-ff-api-key, query fallback) and compares digests, so a run id alone is never a bearer capability over--allow-unauthenticated.done(or provably pre-writefailed) reports the REAL outcome; a 404 / 5xx / malformed body (older runner) falls back to the existing project re-read unchanged, so an older runner keeps working.Tests
dart analyzeclean,dart test):sha256vs FIPS vectors;fetchRunStatusauth rejection (missing key 401 / wrong key 403), 404 for an unknown id, 503 on Firestore failure; theRunRecordernever throws even when every write is rejected; Firestore field codec round trip.node --test src/*.test.js: 416 pass, +14 new): id plumbing, response→outcome mapping (pre-write vs post-write failure), and the fallback when the endpoint is absent, 404s, or returns a malformed body.e2e/deploy-timeout-reconcile.spec.jspasses with the new wiring (reconcile fallback intact).Not tested / notes
cloud-run/Dockerfilealready copieslib/(line:COPY lib/ ./lib/), so the new runner libraries deploy with no Dockerfile change.stu380dirty-cache export-count tests such as993/1091/1171/1282) fail intermittently on the base branch too in this harness, independent of this change; they are out of scope here.