From e5778bc43e2f5d1b09a90d269e514eb7a0fc422f Mon Sep 17 00:00:00 2001 From: Stuart Date: Sun, 4 Oct 2026 15:12:39 +1100 Subject: [PATCH 01/20] fix(deploy): bump pinned flutterflow_cli to 0.0.41 in the ffai runner image The runner's Docker image pinned flutterflow_cli to 0.0.39. FlutterFlow's AI snapshot now requires >=0.0.41, so every deploy failed workspace init with "This FlutterFlow AI snapshot requires flutterflow_cli >= 0.0.41, but you are running 0.0.39." This was hidden behind the stale allowedSdkPackages allowlist bug (fixed in d1dc19c but never redeployed) until that revision went live and exposed it as the next blocker. Verified: a direct probe against the redeployed runner now gets past both the SDK-package check and the CLI-version check, failing only on API key auth with a deliberately invalid test key. --- cloud-run/ffai-runner/Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cloud-run/ffai-runner/Dockerfile b/cloud-run/ffai-runner/Dockerfile index e0b4ca0..2a14587 100644 --- a/cloud-run/ffai-runner/Dockerfile +++ b/cloud-run/ffai-runner/Dockerfile @@ -4,7 +4,7 @@ ENV PATH="/root/.pub-cache/bin:${PATH}" ENV PORT=8080 ENV FFAI_WORKSPACE=/workspace/custom_code_connect -ARG FLUTTERFLOW_CLI_VERSION=0.0.39 +ARG FLUTTERFLOW_CLI_VERSION=0.0.41 RUN dart pub global activate flutterflow_cli "$FLUTTERFLOW_CLI_VERSION" \ && dart pub global list | grep -F "flutterflow_cli $FLUTTERFLOW_CLI_VERSION" From 5231e76543457a24d5f5cf8f32774c04ffbdaf67 Mon Sep 17 00:00:00 2001 From: Stuart Date: Sun, 4 Oct 2026 15:33:17 +1100 Subject: [PATCH 02/20] fix(deploy): stop the 120s UI bound firing on routine deploys and report what landed Measured successful runner deploys took 103-215s, so the 120s browser bound fired on ordinary bundles while the server went on to succeed, leaving the user on "outcome not yet known" with step 3 (pubspec push) never run. Raise the bound to 840s (still under Cloud Run's 900s). When a provisioning response is still lost or expired, re-read the project and report which classes actually landed and what a retry will do, instead of "go check yourself". --- app.js | 59 +++++++++++++++++++++ src/deployOutcome.js | 19 ++++--- src/flutterFlowCodeFileProvisioning.js | 17 ++++++ src/flutterFlowCodeFileProvisioning.test.js | 18 +++++++ 4 files changed, 105 insertions(+), 8 deletions(-) diff --git a/app.js b/app.js index 2a520a6..d7a85dd 100644 --- a/app.js +++ b/app.js @@ -33,6 +33,7 @@ import { buildBundleDeployPlan } from "./src/bundleDeployPlanner.js"; import { excludeProvisionedCodeFiles, findMissingCodeFiles, + partitionProvisionedCodeFiles, } from "./src/flutterFlowCodeFileProvisioning.js"; import { buildReviewPresentation } from "./src/reviewPresentation.js"; import { @@ -3180,6 +3181,59 @@ async function pushCodeWithUiTimeout( ); } +/** + * After a lost or expired provisioning response, reads the project back and + * says which classes actually landed, so the user is never left with only + * "go check FlutterFlow yourself". The read is its own bounded request: if it + * cannot be made, the original unconfirmed error stands unchanged. + * + * "Not found" is reported as "not there yet", never as "failed": the runner + * may still be writing, and a retry re-reads the project first so already + * landed classes are skipped rather than duplicated. + * @param {FlutterFlowApiClient} apiClient + * @param {Array<{className: string, path: string}>} attempted + * @param {UnconfirmedDeployError} original + * @returns {Promise} + */ +async function reconcileUnconfirmedProvision(apiClient, attempted, original) { + let fresh; + try { + fresh = await Promise.race([ + apiClient.fetchProjectSource(), + new Promise((_, reject) => + setTimeout(() => reject(new Error("read timed out")), 20_000), + ), + ]); + } catch { + return original; + } + + const { landed, notLanded } = partitionProvisionedCodeFiles( + attempted, + fresh.files, + ); + const names = (entries) => entries.map((entry) => entry.className).join(", "); + + let message; + if (notLanded.length === 0) { + message = + `Checked FlutterFlow just now: all ${landed.length} custom class(es) are in the project (${names(landed)}). ` + + "The final step, pushing your pubspec.yaml dependencies, did not run. " + + "Deploy again to finish it - classes already there are skipped, not duplicated."; + } else if (landed.length === 0) { + message = + `Checked FlutterFlow just now: none of the ${notLanded.length} custom class(es) are in the project yet (${names(notLanded)}). ` + + "The server may still be writing them. Wait a minute, then deploy again - " + + "the project is re-read first, so nothing is duplicated."; + } else { + message = + `Checked FlutterFlow just now: ${landed.length} of ${attempted.length} custom class(es) are in the project (${names(landed)}); ` + + `not there yet: ${names(notLanded)}. Wait a minute, then deploy again - ` + + "classes already there are skipped, and the rest are written."; + } + return new UnconfirmedDeployError(message); +} + async function provisionMissingCodeFiles( apiClient, fileMap, @@ -3368,6 +3422,11 @@ async function provisionMissingCodeFiles( }, () => {}, ); + throw await reconcileUnconfirmedProvision( + apiClient, + missingCodeFiles, + error, + ); } throw error; } diff --git a/src/deployOutcome.js b/src/deployOutcome.js index ebf62d7..98e9375 100644 --- a/src/deployOutcome.js +++ b/src/deployOutcome.js @@ -29,15 +29,18 @@ export const DeployOutcome = Object.freeze({ * * Derivation: Cloud Run's request timeout for the deploy runner is 900s and * the runner reports a server-side timeout rather than hanging past it - * (DEPLOYMENT.md), so a remote write cannot outlive roughly 900s. Observed - * provision latency is of the order of one to a few minutes. 120s is therefore - * comfortably longer than a healthy provision yet well below the server - * deadline, so a normal deploy renders to completion long before this fires - * and the UI can never out-wait a real server decision. This bound governs - * only the UI's waiting/rendering: on expiry the UI stops claiming progress, - * but it never aborts or retries the in-flight remote write. + * (DEPLOYMENT.md), so a remote write cannot outlive roughly 900s. Measured + * successful provisions on the production runner took 103s-215s (cold + * container: FlutterFlow AI SDK download, `flutter pub get`, `flutter analyze`, + * then the push), so the earlier 120s bound fired on routine deploys while the + * server went on to succeed, stranding the user on "outcome not yet known". + * 840s leaves ~4x headroom over the slowest observed run and still sits below + * the server deadline, so the UI can never out-wait a real server decision. + * Live phase events keep the progress view moving for the whole wait. This + * bound governs only the UI's waiting/rendering: on expiry the UI stops + * claiming progress, but it never aborts or retries the in-flight remote write. */ -export const DEPLOY_UI_TIMEOUT_MS = 120_000; +export const DEPLOY_UI_TIMEOUT_MS = 840_000; // A response's status alone classifies it; the body only carries detail. // A stalled error body must therefore never hold up a definitive refusal — diff --git a/src/flutterFlowCodeFileProvisioning.js b/src/flutterFlowCodeFileProvisioning.js index 68c10f3..49f40e5 100644 --- a/src/flutterFlowCodeFileProvisioning.js +++ b/src/flutterFlowCodeFileProvisioning.js @@ -47,6 +47,23 @@ export function findMissingCodeFiles(fileMap, remoteFiles = new Map()) { return missing; } +/** + * Splits the entries a provisioning request was writing by whether a fresh + * project export now contains them. Used after a lost or expired response to + * say what actually happened instead of leaving the outcome unknown. + * @param {Array<{path: string}>} entries - Entries the request attempted + * @param {Map} remoteFiles - Fresh project export, by path + * @returns {{landed: Array, notLanded: Array}} + */ +export function partitionProvisionedCodeFiles(entries, remoteFiles = new Map()) { + const landed = []; + const notLanded = []; + for (const entry of entries) { + (remoteFiles.has(entry.path) ? landed : notLanded).push(entry); + } + return { landed, notLanded }; +} + export function excludeProvisionedCodeFiles(fileMap, entries) { const provisionedPaths = new Set(entries.map((entry) => entry.path)); return new Map( diff --git a/src/flutterFlowCodeFileProvisioning.test.js b/src/flutterFlowCodeFileProvisioning.test.js index aeffa0b..d6849b6 100644 --- a/src/flutterFlowCodeFileProvisioning.test.js +++ b/src/flutterFlowCodeFileProvisioning.test.js @@ -3,6 +3,7 @@ import test from "node:test"; import { excludeProvisionedCodeFiles, findMissingCodeFiles, + partitionProvisionedCodeFiles, } from "./flutterFlowCodeFileProvisioning.js"; test("plans provisioning for a standalone code file missing from FlutterFlow", () => { @@ -113,3 +114,20 @@ test("excludes provisioned code files from the subsequent custom-code sync", () ["read_nfc_tag.dart"], ); }); + +test("partitionProvisionedCodeFiles splits attempted entries by fresh export", () => { + const attempted = [ + { className: "Alpha", path: "lib/custom_code/alpha.dart" }, + { className: "Beta", path: "lib/custom_code/beta.dart" }, + ]; + const remote = new Map([["lib/custom_code/alpha.dart", "class Alpha {}"]]); + + const { landed, notLanded } = partitionProvisionedCodeFiles(attempted, remote); + + assert.deepEqual(landed.map((e) => e.className), ["Alpha"]); + assert.deepEqual(notLanded.map((e) => e.className), ["Beta"]); + assert.deepEqual( + partitionProvisionedCodeFiles(attempted).notLanded.length, + 2, + ); +}); From 67460774c2b20b4a4d1cd3d494a0adbb087f2df2 Mon Sep 17 00:00:00 2001 From: Stuart Date: Sun, 4 Oct 2026 15:46:22 +1100 Subject: [PATCH 03/20] test(e2e): cover the slow-runner timeout and the reconcile message MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 840s UI bound keeps the live progress view alive on routine deploys, but when a provision response is still lost or expired the app re-reads the project and reports which classes actually landed instead of leaving the user on a dead-end. These browser tests drive the real provisioning transport against a stalled mocked runner at a millisecond bound and render the reconcile outcome through the real terminal renderer, locking in that the timeout is UNCONFIRMED — never a fabricated failure — and that the modal names the project and a next step. --- e2e/deploy-timeout-reconcile.spec.js | 182 +++++++++++++++++++++++++++ 1 file changed, 182 insertions(+) create mode 100644 e2e/deploy-timeout-reconcile.spec.js diff --git a/e2e/deploy-timeout-reconcile.spec.js b/e2e/deploy-timeout-reconcile.spec.js new file mode 100644 index 0000000..1aa7ce1 --- /dev/null +++ b/e2e/deploy-timeout-reconcile.spec.js @@ -0,0 +1,182 @@ +import { test, expect } from "@playwright/test"; +import JSZip from "jszip"; +import { applyDefaultRoutes, ENDPOINTS } from "./fixtures/apiFixtures.js"; + +/** + * Item 10 — the slow-runner timeout path and the reconcile message. + * + * A provision that outlives the browser UI bound is UNCONFIRMED, never a + * fabricated failure. Where the old code left the user on "go check + * FlutterFlow yourself", the reconcile re-read (reconcileUnconfirmedProvision) + * fetches the project back and says which classes actually landed and what a + * retry will do. These tests drive the real provisioning transport against a + * deliberately slow mocked runner (the bound is shortened to milliseconds, not + * 840 real seconds), then render the reconcile outcome through the real + * terminal renderer (`__CCC_RENDER_DEPLOY_TERMINAL__`), the same entry point + * stu380-deploy.spec.js uses. + * + * The terminal copy is owned by another agent in this workstream and may be + * adjusted in parallel, so the rendering assertions here are structural and + * semantic (the modal appears, it is an unconfirmed outcome rather than a + * plain fabricated failure, it names the project, tells the user what to do + * next, and offers the FlutterFlow action control) rather than pinned to any + * exact sentence. + */ + +const identity = { + projectId: "ff-proj-0007", + endpoint: "https://api.flutterflow.io", + artifactType: "CustomClass", + artifactName: "GaugeModel", + fileName: "gauge_widget.dart", +}; + +// One custom class the runner "writes" before the response is lost. +const attemptedClass = { + "gauge_model.dart": { + artifactName: "GaugeModel", + content: "class GaugeModel { final int value; GaugeModel(this.value); }", + type: "C", + path: "lib/custom_code/gauge_model.dart", + }, +}; + +async function loadDeployHooks(page) { + await page.addInitScript(() => { + localStorage.setItem("hasSeenWalkthrough", "true"); + }); + await applyDefaultRoutes(page); + await page.goto("/"); + await page.waitForFunction( + () => + typeof window.__CCC_PROVISION_CUSTOM_CLASSES__ === "function" && + typeof window.__CCC_RENDER_DEPLOY_TERMINAL__ === "function", + ); +} + +async function settleAsOutcome(page, hook, args) { + return page.evaluate( + async ({ hook, args }) => { + try { + await window[hook](...args); + return { settled: "resolved" }; + } catch (error) { + return { + settled: "rejected", + name: error.name, + outcome: error.outcome, + message: error.message, + }; + } + }, + { hook, args }, + ); +} + +// A project export that already contains the custom class — what the runner's +// write leaves behind before its response is lost. Reconcile reads this back to +// say the class actually landed. +async function projectZipWithClassLanded() { + const zip = new JSZip(); + zip.file("pubspec.yaml", "name: my_app\n"); + zip.file( + "lib/custom_code/gauge_model.dart", + "class GaugeModel { final int value; GaugeModel(this.value); }", + ); + return zip.generateAsync({ type: "base64" }); +} + +// The runner accepts the provisioning connection but never answers, so the +// request times out at the (shortened) UI bound while the class may already be +// written on the server. +async function stallRunner(page) { + await page.route(ENDPOINTS.deployCustomClasses, () => new Promise(() => {})); +} + +test.describe("Item 10 slow-runner timeout + reconcile", () => { + test("a slow provision past the bound re-reads the project and reports what actually landed", async ({ page }) => { + await loadDeployHooks(page); + await page.route("**/exportCode", async (route) => { + await route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify({ + value: { project_zip: await projectZipWithClassLanded() }, + }), + }); + }); + await stallRunner(page); + + const outcome = await settleAsOutcome( + page, + "__CCC_PROVISION_CUSTOM_CLASSES__", + [{ fileMap: attemptedClass, uiTimeoutMs: 60 }], + ); + + // Losing the response is unconfirmed — never a fabricated failure. + expect(outcome.settled).toBe("rejected"); + expect(outcome.name).toBe("UnconfirmedDeployError"); + expect(outcome.outcome).toBe("unconfirmed"); + // The reconcile re-read ran: the message names the class that actually + // landed instead of a dead-end "go check yourself". The class name is data + // carried by the reconcile, so this is durable against copy edits. + expect(outcome.message).toContain("GaugeModel"); + // The message offers a concrete next step, not just a verdict. + expect(outcome.message).not.toContain("has stopped waiting"); + }); + + test("the reconcile message renders in an unconfirmed terminal modal that names the project and next steps", async ({ page }) => { + await loadDeployHooks(page); + await page.route("**/exportCode", async (route) => { + await route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify({ + value: { project_zip: await projectZipWithClassLanded() }, + }), + }); + }); + await stallRunner(page); + + // Produce the real reconcile message through the transport, then render it + // through the real terminal renderer — the exact presentation a user sees. + const outcome = await settleAsOutcome( + page, + "__CCC_PROVISION_CUSTOM_CLASSES__", + [{ fileMap: attemptedClass, uiTimeoutMs: 60 }], + ); + expect(outcome.name).toBe("UnconfirmedDeployError"); + await page.evaluate( + ({ message, targetIdentity }) => { + window.__CCC_RENDER_DEPLOY_TERMINAL__({ + success: false, + unconfirmed: true, + targetIdentity, + error: message, + }); + }, + { message: outcome.message, targetIdentity: identity }, + ); + + const terminal = page.locator("#commit-terminal-modal"); + // The terminal modal appeared at all. + await expect(terminal).toBeVisible(); + // It is an unconfirmed outcome, not a fabricated failure: the heading is + // the unknown-state label, never "Deploy failed" or "Committed". + await expect(terminal.locator("#terminal-heading")).not.toContainText( + "Deploy failed", + ); + await expect(terminal).not.toContainText("Committed"); + // It names the project the user confirmed. + await expect(terminal).toContainText(identity.projectId); + // The reconcile message itself is on screen (it carries the landed class). + await expect(terminal).toContainText("GaugeModel"); + // It tells the user what to do next — the guidance section is rendered. + await expect(page.locator("#terminal-guidance")).not.toBeEmpty(); + // The action control it offers is present and targets the project. + await expect(page.locator("#terminal-open-ff-link")).toHaveAttribute( + "href", + `https://app.flutterflow.io/project/${identity.projectId}`, + ); + }); +}); From b16887d717cdce82cd62fb5e9c8cc7b2782e2f4a Mon Sep 17 00:00:00 2001 From: Stuart Date: Sun, 4 Oct 2026 15:48:29 +1100 Subject: [PATCH 04/20] infra(runner): keep the Cloud Run runner warm, deploy it from CI, and schedule a canary Cold start dominates deploy latency (103s-215s): the FlutterFlow AI SDK download and a fresh pub get run on every new instance. Pinning min-instances to 1 makes that happen once per container boot instead of per request, buying a much shorter deploy for a small always-on cost. A merged runner fix previously shipped nothing: add a workflow that deploys cloud-run/** changes on main, gated on a service-account secret so pushes before setup skip instead of failing. A daily dry-run canary to automated-test-miedro catches a broken runner before users do. --- .github/workflows/canary.yml | 39 ++++++ .github/workflows/deploy-runner.yml | 50 ++++++++ DEPLOYMENT.md | 46 +++++++ scripts/canary-deploy.mjs | 179 ++++++++++++++++++++++++++++ scripts/deploy_cloud_run_ffai.sh | 6 + 5 files changed, 320 insertions(+) create mode 100644 .github/workflows/canary.yml create mode 100644 .github/workflows/deploy-runner.yml create mode 100644 scripts/canary-deploy.mjs diff --git a/.github/workflows/canary.yml b/.github/workflows/canary.yml new file mode 100644 index 0000000..639d239 --- /dev/null +++ b/.github/workflows/canary.yml @@ -0,0 +1,39 @@ +name: Runner canary + +# A scheduled dry-run smoke test against the production FlutterFlow AI runner, +# so a broken runner is caught the next morning instead of by a user's deploy. +# Runs daily and can be triggered manually with workflow_dispatch. +# +# Additive only: it never deploys and never touches this repository. + +on: + schedule: + # 07:21 UTC daily (off the :00/:30 marks to spread GHA load). + - cron: "21 7 * * *" + workflow_dispatch: + +permissions: + contents: read + +jobs: + canary: + # Skip (rather than fail noisily) until the FlutterFlow API key secret + # exists, so scheduled runs before one-time setup complete are quiet. The + # script itself also fails clearly if the key ever comes back empty. + if: secrets.CANARY_FF_API_KEY != '' + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 + + - name: Set up Node.js + uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4 + with: + node-version-file: ".nvmrc" + + - name: Run the dry-run canary + run: node scripts/canary-deploy.mjs + env: + CANARY_FF_API_KEY: ${{ secrets.CANARY_FF_API_KEY }} + CANARY_RUNNER_URL: ${{ vars.CANARY_RUNNER_URL || '' }} + CANARY_PROJECT_ID: ${{ vars.CANARY_PROJECT_ID || 'automated-test-miedro' }} diff --git a/.github/workflows/deploy-runner.yml b/.github/workflows/deploy-runner.yml new file mode 100644 index 0000000..78cf164 --- /dev/null +++ b/.github/workflows/deploy-runner.yml @@ -0,0 +1,50 @@ +name: Deploy runner + +# Deploy the FlutterFlow AI runner to Cloud Run whenever cloud-run/** changes +# land on main, so a merged runner fix actually ships instead of sitting in the +# repo. Additive only: it never touches branch protection and never runs on +# pull requests. + +on: + push: + branches: [main] + paths: + - "cloud-run/**" + +# The deploy script needs `gcloud` (auth + setup-gcloud below). No write access +# to this repository itself is required. +permissions: + contents: read + +concurrency: + group: deploy-runner + cancel-in-progress: true + +jobs: + deploy-runner: + # The whole point of gating on the secret instead of failing is that pushes + # before one-time setup complete are skipped quietly. Until + # CCC_FFAI_RUNNER_SA_KEY_JSON exists (see DEPLOYMENT.md) this job does not + # run; once it does, a merged cloud-run change deploys. + if: secrets.CCC_FFAI_RUNNER_SA_KEY_JSON != '' + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 + + - name: Authenticate to Google Cloud + uses: google-github-actions/auth@c200f3691d83b41bf9bbd8638997a462592937ed # v2 + with: + # JSON of a service account with roles/run.admin + roles/cloudbuild.builds.editor + roles/iam.serviceAccountUser. + credentials_json: ${{ secrets.CCC_FFAI_RUNNER_SA_KEY_JSON }} + + - name: Set up gcloud + uses: google-github-actions/setup-gcloud@e427ad8a34f8676edf47cf7d7925499adf3eb74f # v2 + with: + project_id: ${{ vars.CCC_PROJECT_ID || 'low-code-connect' }} + + - name: Deploy the FlutterFlow AI runner + run: | + PROJECT_ID="${{ vars.CCC_PROJECT_ID || 'low-code-connect' }}" \ + REGION="${{ vars.CCC_REGION || 'us-west1' }}" \ + ./scripts/deploy_cloud_run_ffai.sh diff --git a/DEPLOYMENT.md b/DEPLOYMENT.md index 3947986..32afc19 100644 --- a/DEPLOYMENT.md +++ b/DEPLOYMENT.md @@ -159,6 +159,52 @@ Deploy the runner with: PROJECT_ID=low-code-connect REGION=us-west1 ./scripts/deploy_cloud_run_ffai.sh ``` +Keep one instance warm by default: the script passes `--min-instances 1`, because +most of a cold start (FlutterFlow AI SDK download and a fresh `flutter pub get`) +is paid once per container instead of once per deploy. Override the count with +`MIN_INSTANCES`, e.g. `MIN_INSTANCES=0 ./scripts/deploy_cloud_run_ffai.sh` to +scale to zero and pay per-request cold starts instead. + +#### One-time setup for automated deploys (GitHub Actions) + +The workflow `.github/workflows/deploy-runner.yml` deploys the runner whenever +`cloud-run/**` changes on `main`. It uses a Google Cloud service-account key +from a repository secret. Until that secret exists the job **silently skips** +(rather than failing on every push), so set up this once or merged runner +changes will deploy nothing: + +1. Create a service account with at least `roles/run.admin`, + `roles/cloudbuild.builds.editor` and `roles/iam.serviceAccountUser`, generate + a JSON key, and download it. +2. In **Settings → Secrets and variables → Actions**, create a repository secret + named `CCC_FFAI_RUNNER_SA_KEY_JSON` whose value is the key's full JSON + (including the surrounding `{ }`). +3. The workflow reads `CCC_PROJECT_ID` (default `low-code-connect`) and + `CCC_REGION` (default `us-west1`) from repository variables; create those if + the defaults are not right. This is per-repo like every secret here, because + GitHub does not expose organisation secrets to private repositories on the + free plan. + +#### Runner canary (scheduled) + +The workflow `.github/workflows/canary.yml` runs daily (07:21 UTC) and on demand +via **Actions → Runner canary → Run workflow**. It sends a `dryRun: true` +deploy for the throwaway project `automated-test-miedro` to the runner's +`/deployCustomClasses`, so it exercises workspace init, the compile gate and the +DSL push path without writing anything to a real project. It fails (non-zero) +on any non-200 response, on `success:false`, or if the runner rejects the +`flutter_web_plugins` SDK package, and also reports the runner's `/healthz` +versions when that endpoint is deployed - without failing when it is not yet. + +Until the FlutterFlow API key secret exists the canary job skips cleanly, so it +never fails noisily before setup: + +1. Create a repository secret named `CANARY_FF_API_KEY` whose value is a + FlutterFlow API key with at least read access to `automated-test-miedro`. +2. Optionally set a repository variable `CANARY_RUNNER_URL` if the production + runner lives somewhere other than the default, and `CANARY_PROJECT_ID` if the + throwaway project changes. + ## BuildShip workflow and step prompts The generation pipeline (Architect → Generator → Review) runs on BuildShip, and diff --git a/scripts/canary-deploy.mjs b/scripts/canary-deploy.mjs new file mode 100644 index 0000000..71d9ab7 --- /dev/null +++ b/scripts/canary-deploy.mjs @@ -0,0 +1,179 @@ +#!/usr/bin/env node + +// Scheduled canary for the FlutterFlow custom-class runner. +// +// Every scheduled run POSTs a *dry-run* deploy to the runner so a regression +// that would break users' deploys is caught the next morning instead of on +// their next deploy. dryRun: true makes the runner go through `ai validate` +// (workspace init, compile gate, DSL validation) but never write to the +// project, so this is safe to run unattended. +// +// It also curls the runner's /healthz when present and reports its versions, +// but never fails on a missing /healthz - another change is adding it in +// parallel and it may not be deployed yet. + +const RUNNER_DEFAULT_URL = "https://ccc-ffai-runner-y5cyj3473a-uw.a.run.app"; + +// The throwaway class and dependencies the canary sends. These are real enough +// to compile: the runner builds a scratch package from the verification +// manifest and fails the deploy if `flutter analyze` reports an error, so a +// canary class that does not compile would fail every run for the wrong reason. +const CANARY_CLASS_NAME = "CanaryProbe"; +const CANARY_SOURCE = `import 'package:flutter/widgets.dart'; + +class ${CANARY_CLASS_NAME} extends StatelessWidget { + const ${CANARY_CLASS_NAME}({super.key}); + + @override + Widget build(BuildContext context) => const SizedBox.shrink(); +}`; + +function fail(message) { + console.error(`[canary] FAIL: ${message}`); + process.exitCode = 1; +} + +async function main() { + const apiKey = process.env.CANARY_FF_API_KEY; + if (!apiKey) { + fail( + "CANARY_FF_API_KEY is not set. Create the repository secret " + + "CANARY_FF_API_KEY with a FlutterFlow API key (see DEPLOYMENT.md) " + + "and give this job access to it.", + ); + return; + } + + const baseUrl = (process.env.CANARY_RUNNER_URL || RUNNER_DEFAULT_URL).replace( + /\/+$/, + "", + ); + const projectId = process.env.CANARY_PROJECT_ID || "automated-test-miedro"; + const timeoutMs = Number(process.env.CANARY_TIMEOUT_MS || 60000); + + const payload = { + apiKey, + projectId, + commitMessage: "canary dry-run", + dryRun: true, + customClasses: [ + { className: CANARY_CLASS_NAME, content: CANARY_SOURCE }, + ], + verification: { + sdkConstraint: ">=3.0.0 <4.0.0", + dependencies: {}, + dependencyOverrides: {}, + sdkPackages: ["flutter_web_plugins"], + }, + }; + + // The SDK-package rejection this canary is meant to catch. The runner + // currently rejects an invalid SDK name as "Invalid SDK package name: X"; + // an older/newer wording may say "unsupported". Match both so a regression + // where flutter_web_plugins stops being accepted fails the canary. + const sdkPackageRejection = /unsupported sdk package|invalid sdk package name/i; + + const controller = new AbortController(); + const timer = setTimeout(() => controller.abort(), timeoutMs); + try { + let response; + try { + response = await fetch(`${baseUrl}/deployCustomClasses`, { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify(payload), + signal: controller.signal, + }); + } catch (error) { + fail( + `request to ${baseUrl}/deployCustomClasses failed: ` + + `${error?.name === "AbortError" ? "timed out" : error.message}`, + ); + return; + } + + const text = await response.text(); + let body; + try { + body = JSON.parse(text); + } catch { + body = { error: text.slice(0, 500) }; + } + + if (!response.ok) { + fail( + `runner returned HTTP ${response.status} on a dry-run deploy for ` + + `project "${projectId}". ${formatBodyError(body)}`, + ); + return; + } + + if (body.success === false) { + fail( + `runner reported success:false on a dry-run deploy for project ` + + `"${projectId}". ${formatBodyError(body)}`, + ); + return; + } + + const joined = [ + body.error, + body.details, + body.analyzerErrors, + ...(Array.isArray(body.analyzerErrors) ? body.analyzerErrors : []), + ] + .filter(Boolean) + .join("\n"); + if (sdkPackageRejection.test(joined)) { + fail( + `runner rejected the flutter_web_plugins SDK package on a dry-run ` + + `deploy - a dependency regression users would hit: ${joined}`, + ); + return; + } + + console.log( + `[canary] OK: dry-run deploy for project "${projectId}" accepted at ${baseUrl} ` + + `(success:${body.success === undefined ? "true" : body.success})`, + ); + } finally { + clearTimeout(timer); + } + + // The /healthz route is being added alongside this canary; do not fail the + // canary when it is not deployed yet. + try { + const health = await fetch(`${baseUrl}/healthz`, { + signal: AbortSignal.timeout(10000), + }); + if (health.ok) { + const healthBody = await health.json().catch(() => ({})); + console.log( + `[canary] healthz: HTTP ${health.status}` + + (healthBody.versions + ? ` versions=${JSON.stringify(healthBody.versions)}` + : ` ${JSON.stringify(healthBody)}`), + ); + } else { + console.log( + `[canary] healthz: HTTP ${health.status} (not reported as a failure)`, + ); + } + } catch { + console.log("[canary] healthz: unavailable (not reported as a failure)"); + } +} + +function formatBodyError(body) { + const detail = [ + body.error, + body.details, + ...(Array.isArray(body.analyzerErrors) ? body.analyzerErrors : []), + ] + .filter(Boolean) + .join(" | ") + .slice(0, 1000); + return detail ? `Details: ${detail}` : "No detail in the response body."; +} + +await main(); diff --git a/scripts/deploy_cloud_run_ffai.sh b/scripts/deploy_cloud_run_ffai.sh index 023eb2d..ac521b1 100755 --- a/scripts/deploy_cloud_run_ffai.sh +++ b/scripts/deploy_cloud_run_ffai.sh @@ -11,6 +11,11 @@ CONCURRENCY="${CONCURRENCY:-1}" # covers `flutter pub get` plus `flutter analyze` on top of the DSL run. Cloud # Run's 300s default would cut that off on a cold instance. TIMEOUT="${TIMEOUT:-900}" +# Keep one instance always on. Most of a deploy's 103-215s on a cold start is +# the FlutterFlow AI SDK download and a fresh `flutter pub get` after `ai init`; +# a warm instance does that once at container boot instead of per request. The +# tradeoff is a small always-on instance cost for a much shorter deploy. +MIN_INSTANCES="${MIN_INSTANCES:-1}" if [[ -z "$PROJECT_ID" ]]; then PROJECT_ID="$(gcloud config get-value project 2>/dev/null || true)" @@ -28,6 +33,7 @@ gcloud run deploy "$SERVICE" \ --allow-unauthenticated \ --memory "$MEMORY" \ --concurrency "$CONCURRENCY" \ + --min-instances "$MIN_INSTANCES" \ --timeout "$TIMEOUT" \ --set-env-vars "ALLOWED_ORIGIN=$ALLOWED_ORIGIN" From d55eb19620f4326c28b013af46b0832b798a119c Mon Sep 17 00:00:00 2001 From: Stuart Date: Sun, 4 Oct 2026 15:54:50 +1100 Subject: [PATCH 05/20] feat(runner): surface liveness, scale the deploy cap, expose a health probe A cold runner can be silent for over a minute (SDK snapshot download, flutter analyze), which the browser cannot distinguish from a dead connection, and the CLI cap was a flat 5 minutes regardless of bundle size, so a large deploy could be killed mid-write. This makes the runner's health observable and its timeout honest: - emit a {"event":"heartbeat"} every 15s while any CLI work runs. The channel already drops events for non-streaming callers, and the browser ignores unknown events, so it is backward compatible. - scale the deploy cap to 5min + 15s/class (10 min at the 20-class maximum) while keeping the workspace-init cap at 5 min, so even the largest bundle stays well under Cloud Run's 900s request timeout. - add GET /healthz returning the image git SHA (env RUNNER_GIT_SHA), the installed flutterflow_cli, and the minimum the vendored snapshot requires, so a merged-but-never-deployed fix is detectable in minutes. The manifest validation (sdkPackages, dependency maps, constraints) moves from private helpers in bin/server.dart to public functions in lib/request_validation.dart, logic byte-for-byte unchanged, so the exact contract - in particular that flutter_web_plugins and flutter_localizations are accepted as package names rather than rejected by a stale allowlist - can be pinned by tests. --- cloud-run/ffai-runner/bin/server.dart | 520 ++++++------------ .../ffai-runner/lib/request_validation.dart | 324 +++++++++++ cloud-run/ffai-runner/pubspec.yaml | 3 + .../test/request_validation_test.dart | 140 +++++ 4 files changed, 640 insertions(+), 347 deletions(-) create mode 100644 cloud-run/ffai-runner/lib/request_validation.dart create mode 100644 cloud-run/ffai-runner/test/request_validation_test.dart diff --git a/cloud-run/ffai-runner/bin/server.dart b/cloud-run/ffai-runner/bin/server.dart index c33a5fa..be4b67a 100644 --- a/cloud-run/ffai-runner/bin/server.dart +++ b/cloud-run/ffai-runner/bin/server.dart @@ -2,32 +2,12 @@ import 'dart:async'; import 'dart:convert'; import 'dart:io'; -const maxClassesPerRequest = 20; -const maxCodeBytes = 500000; -const maxDependenciesPerRequest = 200; -// pub.dev's own ceiling on a package name. -const maxPackageNameLength = 64; - -const analysisPackageName = 'ccc_custom_code_analysis'; -const defaultSdkConstraint = '>=3.0.0 <4.0.0'; - -// Pub package names are lower-case identifiers. Matching the whole string is -// what keeps a caller from smuggling YAML structure in through a key. -final _packageNamePattern = RegExp(r'^[a-z_][a-z0-9_]*$'); - -// A pub version constraint, and nothing else. The allowed characters exclude -// `:`, `#`, `{`, `}`, `/` and whitespace beyond single spaces, so `git:`, -// `path:`, a nested `hosted:` mapping, and comment injection are all -// unrepresentable rather than merely discouraged. -final _constraintPattern = RegExp(r'^[A-Za-z0-9^~<>=*+._ -]+$'); - -// Which packages the Flutter SDK supplies is not decided here. A caller names -// the packages its project declares as `sdk: flutter`, and this runner emits -// them as `name: {sdk: flutter}` - a value it writes itself, never one a caller -// sent. There was an allowlist of SDK package names here, and it refused good -// deploys: the client classified a package correctly by its pubspec source, and -// the runner rejected it for not appearing on a list that had gone stale. -// `_packageNamePattern` is what actually keeps a key from carrying YAML. +import 'package:ccc_ffai_runner/request_validation.dart'; + +// The lowest flutterflow_cli the vendored FlutterFlow AI snapshot accepts. The +// image pins FLUTTERFLOW_CLI_VERSION to this (see Dockerfile); it must be kept +// in lockstep there, and it is what /healthz reports as the required minimum. +const minCliVersion = '0.0.41'; Future main() async { final port = int.tryParse(Platform.environment['PORT'] ?? '') ?? 8080; @@ -50,7 +30,13 @@ Future _handle(HttpRequest request) async { return; } - if (request.method != 'POST' || request.uri.path != '/deployCustomClasses') { + if (request.method == 'GET' && request.uri.path == '/healthz') { + await _writeJsonBody(request.response, await _healthz()); + return; + } + + if (request.method != 'POST' || + request.uri.path != '/deployCustomClasses') { await channel.result(HttpStatus.notFound, { 'success': false, 'error': 'Not found.', @@ -70,13 +56,18 @@ Future _handle(HttpRequest request) async { String commitMessage; bool dryRun; List classes; - _VerificationRequest? verification; + VerificationRequest? verification; try { payload = await _readJson(request); - apiKey = _stringField(payload, 'apiKey', maxLength: 10000); - projectId = _stringField(payload, 'projectId', maxLength: 200); - baseUrl = _stringField(payload, 'baseUrl', maxLength: 500, required: false); - commitMessage = _stringField( + apiKey = stringField(payload, 'apiKey', maxLength: 10000); + projectId = stringField(payload, 'projectId', maxLength: 200); + baseUrl = stringField( + payload, + 'baseUrl', + maxLength: 500, + required: false, + ); + commitMessage = stringField( payload, 'commitMessage', maxLength: 300, @@ -84,7 +75,7 @@ Future _handle(HttpRequest request) async { ); dryRun = payload['dryRun'] == true; classes = _normalizeClasses(payload['customClasses']); - verification = _normalizeVerification(payload['verification']); + verification = normalizeVerification(payload['verification']); } on FormatException catch (error) { await channel.result(HttpStatus.badRequest, { 'success': false, @@ -103,15 +94,17 @@ Future _handle(HttpRequest request) async { channel.phase('connected', 'Connected to the FlutterFlow deploy runner.'); final workspace = Directory( - Platform.environment['FFAI_WORKSPACE'] ?? '/workspace/custom_code_connect', + Platform.environment['FFAI_WORKSPACE'] ?? + '/workspace/custom_code_connect', ); final initResult = await _ensureWorkspace(workspace, apiKey, channel); if (initResult != null) { await channel.result(HttpStatus.badGateway, { 'success': false, - 'error': initResult.timedOut - ? 'Preparing the FlutterFlow AI workspace timed out.' - : 'FlutterFlow AI workspace initialization failed.', + 'error': + initResult.timedOut + ? 'Preparing the FlutterFlow AI workspace timed out.' + : 'FlutterFlow AI workspace initialization failed.', 'details': _trimOutput(initResult.output), 'exitCode': initResult.exitCode, // Workspace setup precedes any write to FlutterFlow. @@ -129,7 +122,8 @@ Future _handle(HttpRequest request) async { if (analysis != null && analysis.hasErrors) { await channel.result(HttpStatus.unprocessableEntity, { 'success': false, - 'error': 'The generated custom code does not compile, so nothing was ' + 'error': + 'The generated custom code does not compile, so nothing was ' 'deployed to FlutterFlow.', 'details': analysis.report, 'analyzerErrors': analysis.errors, @@ -175,6 +169,7 @@ Future _handle(HttpRequest request) async { workingDirectory: workspace.path, apiKey: apiKey, channel: channel, + timeout: _deployTimeout(classes.length), ); if (result.timedOut) { @@ -202,12 +197,15 @@ Future _handle(HttpRequest request) async { await channel.result(HttpStatus.ok, { 'success': true, 'message': 'Custom classes upserted through FlutterFlow AI DSL.', - 'deployed': classes - .map((entry) => { - 'artifactId': entry.artifactId, - 'className': entry.className, - }) - .toList(), + 'deployed': + classes + .map( + (entry) => { + 'artifactId': entry.artifactId, + 'className': entry.className, + }, + ) + .toList(), 'dryRun': dryRun, 'verified': analysis?.verifiedFiles ?? const [], 'verificationSkipped': analysis?.skippedReason, @@ -231,7 +229,9 @@ Future<_RunOutcome?> _ensureWorkspace( String apiKey, _ResponseChannel channel, ) async { - final packageConfig = File('${workspace.path}/.dart_tool/package_config.json'); + final packageConfig = File( + '${workspace.path}/.dart_tool/package_config.json', + ); if (packageConfig.existsSync()) { channel.phase('workspace_ready', 'Build environment is ready.'); return null; @@ -262,8 +262,22 @@ Future<_RunOutcome?> _ensureWorkspace( return null; } +/// Caps a single deploy invocation, scaling with how many classes it must +/// upload. A fixed cap would have to be sized to the largest bundle, either +/// killing a big one or over-waiting a small one; starting at 5 minutes and +/// growing by 15s per class keeps even the maximum 20-class request at 10 +/// minutes - comfortably under Cloud Run's 900s request timeout and inside the +/// browser's 840s wait, with headroom to spare. +Duration _deployTimeout(int classCount) { + return const Duration(minutes: 5) + Duration(seconds: 15 * classCount); +} + /// Runs the FlutterFlow CLI, forwarding its output line by line so the caller /// can report real progress instead of guessing at it. +/// +/// The [timeout] default is the workspace-init cap: `flutterflow ai init` +/// fetches the SDK snapshot once and is deliberately given a tighter bound +/// than a deploy, which pays for the per-class upload on top. Future<_RunOutcome> _runFlutterFlow( List args, { required String workingDirectory, @@ -275,10 +289,7 @@ Future<_RunOutcome> _runFlutterFlow( 'flutterflow', args, workingDirectory: workingDirectory, - environment: { - 'FF_API_KEY': apiKey, - 'FLUTTERFLOW_API_KEY': apiKey, - }, + environment: {'FF_API_KEY': apiKey, 'FLUTTERFLOW_API_KEY': apiKey}, ); final output = StringBuffer(); @@ -287,6 +298,15 @@ Future<_RunOutcome> _runFlutterFlow( timedOut = true; process.kill(ProcessSignal.sigkill); }); + // A cold start can be silent for a minute or more (SDK download, `flutter + // analyze`), which the caller cannot distinguish from a dead connection. Emit + // a heartbeat every 15s while the CLI runs so liveness stays observable; the + // channel drops it for callers that did not ask for a stream, so it costs a + // non-streaming request nothing. + final heartbeat = Timer.periodic( + const Duration(seconds: 15), + (_) => channel.heartbeat(), + ); void consume(String rawLine) { final line = _redact(rawLine, apiKey).trimRight(); @@ -315,6 +335,7 @@ Future<_RunOutcome> _runFlutterFlow( final exitCode = await process.exitCode; await Future.wait([stdoutDone, stderrDone]); timer.cancel(); + heartbeat.cancel(); return _RunOutcome( exitCode: exitCode, @@ -396,13 +417,22 @@ final class _ResponseChannel { _write({'event': 'log', 'message': message}); } + /// Marks the runner as alive without claiming progress. Existing clients + /// ignore unknown events, so this is invisible to them; it exists so a proxy + /// or the browser can tell a long, silent step from a dead connection. + void heartbeat() { + _write({'event': 'heartbeat'}); + } + void _write(Map event) { if (!_streaming || _closed) return; - _writes = _writes.then((_) { - _response.write('${jsonEncode(event)}\n'); - return _response.flush(); - // A disconnected client must not take down the deploy. - }).catchError((Object _) {}); + _writes = _writes + .then((_) { + _response.write('${jsonEncode(event)}\n'); + return _response.flush(); + // A disconnected client must not take down the deploy. + }) + .catchError((Object _) {}); } Future result(int statusCode, Map payload) async { @@ -434,194 +464,54 @@ void _writeCorsHeaders(HttpResponse response) { ..set('Vary', 'Origin'); } -Future> _readJson(HttpRequest request) async { - final raw = await utf8.decoder.bind(request).join(); - final decoded = jsonDecode(raw); - if (decoded is! Map) { - throw const FormatException('Request body must be a JSON object.'); - } - return decoded; -} - -String _stringField( - Map source, - String key, { - required int maxLength, - bool required = true, -}) { - final value = source[key]; - if (value == null || value == '') { - if (required) throw FormatException('Missing $key.'); - return ''; - } - if (value is! String) { - throw FormatException('$key must be a string.'); - } - final trimmed = value.trim(); - if (required && trimmed.isEmpty) { - throw FormatException('Missing $key.'); - } - if (utf8.encode(trimmed).length > maxLength) { - throw FormatException('$key is too long.'); - } - return trimmed; -} - -/// Reads the optional compile-check payload. Absent means the caller is an -/// older client that predates verification; its deploys keep working and are -/// reported as unverified rather than refused. -_VerificationRequest? _normalizeVerification(Object? value) { - if (value == null) return null; - if (value is! Map) { - throw const FormatException('verification must be an object.'); - } - - // Clients deployed before the manifest became structured sent pubspec.yaml - // text. Running it would reintroduce the caller-controlled-source problem, so - // it is never executed - but rejecting the request outright would break every - // deploy from a still-cached client during the rollout window. The deploy - // goes ahead with the check reported as unavailable, which is exactly what - // the deploy did before this feature existed, and the caller is told. - if (value.containsKey('pubspec') && !value.containsKey('dependencies')) { - return _VerificationRequest.unavailable( - 'The compile check was skipped: this client sent a package manifest in ' - 'the older format, which the runner no longer executes. Reload the app ' - 'to pick up the current version.', - ); - } - - final rawSources = value['sources']; - if (rawSources is! List) { - throw const FormatException('verification.sources must be an array.'); - } - if (rawSources.length > maxClassesPerRequest) { - throw const FormatException('Too many sources to verify in one request.'); - } - - final sources = rawSources.indexed.map((item) { - final raw = item.$2; - if (raw is! Map) { - throw FormatException('verification.sources[${item.$1}] must be an object.'); - } - final fileName = _stringField(raw, 'fileName', maxLength: 200); - // The name becomes a path inside the scratch package, so anything that - // could climb out of it is a request to write somewhere else. - if (!RegExp(r'^[a-z0-9_]+\.dart$').hasMatch(fileName)) { - throw FormatException('Invalid verification file name: $fileName.'); - } - return _VerificationSource( - fileName: fileName, - content: _stringField(raw, 'content', maxLength: maxCodeBytes), - ); - }).toList(); - - return _VerificationRequest( - sdkConstraint: _validateConstraint( - 'verification.sdkConstraint', - _stringField(value, 'sdkConstraint', maxLength: 200, required: false), - fallback: defaultSdkConstraint, - ), - dependencies: _normalizeDependencyMap(value['dependencies'], 'dependencies'), - overrides: _normalizeDependencyMap( - value['dependencyOverrides'], - 'dependencyOverrides', - ), - sdkPackages: _normalizeSdkPackages(value['sdkPackages'], 'sdkPackages'), - sdkOverrides: _normalizeSdkPackages(value['sdkOverrides'], 'sdkOverrides'), - sources: sources, - ); +/// Liveness and version probe, so a merged-but-never-deployed fix is +/// detectable in minutes rather than on the next failed deploy: the SHA the +/// image was built from (set by the deploy pipeline as RUNNER_GIT_SHA, absent +/// until then), the flutterflow_cli actually installed, and the minimum the +/// vendored snapshot requires. +Future> _healthz() async { + return { + 'sha': Platform.environment['RUNNER_GIT_SHA'] ?? '', + 'flutterflowCli': await _installedCliVersion(), + 'minFlutterflowCli': minCliVersion, + }; } -/// Reads a `{name: version-constraint}` map, rejecting anything that could -/// express more than a published package at a version. -/// -/// This is the whole reason the manifest is sent as data rather than as -/// pubspec.yaml text. The runner writes this manifest to disk and runs -/// `flutter pub get` against it on a publicly reachable route, so a -/// caller-supplied document could otherwise name a `git:` or `path:` source and -/// have the runner fetch from a host of the caller's choosing. Only bare -/// `name: version` entries can be expressed here, and the runner emits those -/// lines itself, so no source directive can survive into the manifest. -Map _normalizeDependencyMap(Object? value, String field) { - if (value == null) return const {}; - if (value is! Map) { - throw FormatException('verification.$field must be an object.'); - } - if (value.length > maxDependenciesPerRequest) { - throw FormatException('Too many entries in verification.$field.'); - } - - final result = {}; - for (final entry in value.entries) { - final name = '${entry.key}'; - if (!_isValidPackageName(name)) { - throw FormatException('Invalid package name in verification.$field: $name.'); - } - final constraint = '${entry.value}'.trim(); - if (!_constraintPattern.hasMatch(constraint)) { - throw FormatException( - 'Invalid version constraint for $name in verification.$field: $constraint.', - ); +Future _installedCliVersion() async { + try { + final process = await Process.start('dart', ['pub', 'global', 'list']); + final output = await process.stdout.transform(utf8.decoder).join(); + final exitCode = await process.exitCode; + // A version we could not read is better reported as absent than used to + // claim the deployed CLI matches the snapshot. + if (exitCode != 0) return ''; + final regex = RegExp(r'^flutterflow_cli\s+(\S+)'); + for (final line in const LineSplitter().convert(output)) { + final match = regex.firstMatch(line.trim()); + if (match != null) return match.group(1)!; } - result[name] = constraint; + return ''; + } on ProcessException { + return ''; } - return result; } -/// Reads the SDK-supplied package names a caller's project declares. -/// -/// Validated as package names rather than against a list of the packages the -/// Flutter SDK happens to ship: a name the SDK does not provide fails -/// `pub get` with an honest error, whereas a list refuses correct deploys the -/// moment it falls behind the SDK. The pattern is the load-bearing check - each -/// name becomes a key in the generated pubspec. -List _normalizeSdkPackages(Object? value, String field) { - if (value == null) return const []; - if (value is! List) { - throw FormatException('verification.$field must be an array.'); - } - // Bounded like the dependency maps: each entry expands into the generated - // pubspec, so an unbounded array is caller-controlled work before pub get - // ever sees it. - if (value.length > maxDependenciesPerRequest) { - throw FormatException('Too many entries in verification.$field.'); - } - - final result = {}; - for (final raw in value) { - final name = '$raw'; - if (!_isValidPackageName(name)) { - throw FormatException('Invalid SDK package name: $name.'); - } - result.add(name); - } - return result.toList(); +Future _writeJsonBody(HttpResponse response, Object body) async { + response.statusCode = HttpStatus.ok; + response.headers.contentType = ContentType.json; + response.write(jsonEncode(body)); + try { + await response.close(); + } catch (_) {} } -bool _isValidPackageName(String name) => - name.length <= maxPackageNameLength && _packageNamePattern.hasMatch(name); - -String _validateConstraint( - String field, - String value, { - required String fallback, -}) { - final constraint = value.trim(); - if (constraint.isEmpty) return fallback; - if (!_constraintPattern.hasMatch(constraint)) { - throw FormatException('Invalid $field: $constraint.'); +Future> _readJson(HttpRequest request) async { + final raw = await utf8.decoder.bind(request).join(); + final decoded = jsonDecode(raw); + if (decoded is! Map) { + throw const FormatException('Request body must be a JSON object.'); } - return constraint; -} - -/// Quotes a value for YAML when a plain scalar would be misread. -/// -/// `>=1.0.0 <2.0.0` is an ordinary pub range, but it opens with an indicator -/// character and YAML would reject it as a plain scalar - the same reason -/// pubspecSync's `formatConstraint` quotes on the client side. -String _yamlScalar(String value) { - if (RegExp(r'^[A-Za-z0-9][A-Za-z0-9._+-]*$').hasMatch(value)) return value; - return "'${value.replaceAll("'", "''")}'"; + return decoded; } List _normalizeClasses(Object? value) { @@ -632,7 +522,9 @@ List _normalizeClasses(Object? value) { throw const FormatException('At least one custom class is required.'); } if (value.length > maxClassesPerRequest) { - throw const FormatException('Too many custom classes in one deploy request.'); + throw const FormatException( + 'Too many custom classes in one deploy request.', + ); } return value.indexed.map((item) { @@ -642,14 +534,19 @@ List _normalizeClasses(Object? value) { throw FormatException('customClasses[$index] must be an object.'); } - final className = _stringField(raw, 'className', maxLength: 120); + final className = stringField(raw, 'className', maxLength: 120); if (!RegExp(r'^[A-Z][A-Za-z0-9_]*$').hasMatch(className)) { throw FormatException('Invalid custom class name: $className.'); } - final content = _stringField(raw, 'content', maxLength: maxCodeBytes); + final content = stringField(raw, 'content', maxLength: maxCodeBytes); return CustomClassEntry( - artifactId: _stringField(raw, 'artifactId', maxLength: 200, required: false), + artifactId: stringField( + raw, + 'artifactId', + maxLength: 200, + required: false, + ), className: className, content: content, ); @@ -667,7 +564,7 @@ List _normalizeClasses(Object? value) { /// caller states plainly what went unverified. Future<_AnalysisOutcome?> _verifyCustomCode( Directory workspace, - _VerificationRequest? verification, + VerificationRequest? verification, _ResponseChannel channel, ) async { if (verification == null) return null; @@ -685,11 +582,13 @@ Future<_AnalysisOutcome?> _verifyCustomCode( if (libDir.existsSync()) libDir.deleteSync(recursive: true); await libDir.create(recursive: true); - await File('${packageDir.path}/pubspec.yaml') - .writeAsString(verification.toPubspec()); + await File( + '${packageDir.path}/pubspec.yaml', + ).writeAsString(verification.toPubspec()); for (final source in verification.sources) { - await File('${libDir.path}/${source.fileName}') - .writeAsString(source.content); + await File( + '${libDir.path}/${source.fileName}', + ).writeAsString(source.content); } final pubGet = await _runProcess( @@ -697,6 +596,7 @@ Future<_AnalysisOutcome?> _verifyCustomCode( ['pub', 'get'], workingDirectory: packageDir.path, timeout: const Duration(minutes: 4), + channel: channel, ); if (pubGet.exitCode != 0) { return _AnalysisOutcome.skipped( @@ -710,6 +610,7 @@ Future<_AnalysisOutcome?> _verifyCustomCode( ['analyze', '--no-pub', '--no-fatal-infos', '--no-fatal-warnings'], workingDirectory: packageDir.path, timeout: const Duration(minutes: 4), + channel: channel, ); final errors = _analyzerErrors(analyze.output); @@ -760,8 +661,16 @@ Future<_RunOutcome> _runProcess( List args, { required String workingDirectory, required Duration timeout, + required _ResponseChannel channel, }) async { final buffer = StringBuffer(); + // Same reason as `_runFlutterFlow`: `flutter pub get` and `flutter analyze` + // can be silent for a long while, and a heartbeat keeps the request visibly + // alive to a streaming caller. + final heartbeat = Timer.periodic( + const Duration(seconds: 15), + (_) => channel.heartbeat(), + ); try { final process = await Process.start( executable, @@ -774,10 +683,13 @@ Future<_RunOutcome> _runProcess( process.stderr.transform(utf8.decoder).forEach(buffer.write), ]); - final exitCode = await process.exitCode.timeout(timeout, onTimeout: () { - process.kill(ProcessSignal.sigkill); - return -1; - }); + final exitCode = await process.exitCode.timeout( + timeout, + onTimeout: () { + process.kill(ProcessSignal.sigkill); + return -1; + }, + ); await drained; return _RunOutcome( exitCode: exitCode, @@ -790,6 +702,8 @@ Future<_RunOutcome> _runProcess( output: '$executable could not be started: ${error.message}', timedOut: false, ); + } finally { + heartbeat.cancel(); } } @@ -797,11 +711,13 @@ Future _writeDeployScript( Directory workspace, List classes, ) async { - final dslDir = Directory('${workspace.path}/dsl')..createSync(recursive: true); - final calls = classes.map((entry) { - final name = _dartSingleQuoted(entry.className); - final code = _dartRawString(entry.content); - return ''' + final dslDir = Directory('${workspace.path}/dsl') + ..createSync(recursive: true); + final calls = classes + .map((entry) { + final name = _dartSingleQuoted(entry.className); + final code = _dartRawString(entry.content); + return ''' if (findCustomClass(project, name: $name) == null) { addCustomClass( project, @@ -817,7 +733,8 @@ Future _writeDeployScript( } _ensureDartFileName(project, $name); '''; - }).join('\n'); + }) + .join('\n'); final script = ''' library; @@ -979,97 +896,6 @@ String _trimOutput(Object value) { return '${text.substring(0, 4000)}...'; } -final class _VerificationSource { - const _VerificationSource({required this.fileName, required this.content}); - - final String fileName; - final String content; -} - -final class _VerificationRequest { - const _VerificationRequest({ - required this.sdkConstraint, - required this.dependencies, - required this.overrides, - required this.sdkPackages, - required this.sdkOverrides, - required this.sources, - }) : unavailableReason = null; - - /// A request the runner will not compile, carrying why. - const _VerificationRequest.unavailable(this.unavailableReason) - : sdkConstraint = '', - dependencies = const {}, - overrides = const {}, - sdkPackages = const [], - sdkOverrides = const [], - sources = const <_VerificationSource>[]; - - /// Why this request cannot be compiled, or null when it can. - final String? unavailableReason; - - final String sdkConstraint; - final Map dependencies; - final Map overrides; - final List sdkPackages; - final List sdkOverrides; - final List<_VerificationSource> sources; - - /// Builds the pubspec the scratch package is compiled from. - /// - /// Emitted here rather than accepted from the caller: every line below is - /// either a fixed literal or a value already matched against - /// `_packageNamePattern` / `_constraintPattern`, so nothing a caller sends - /// can introduce a new YAML key, a nested mapping, or a comment. - String toPubspec() { - final lines = [ - 'name: $analysisPackageName', - 'description: Throwaway package used to compile generated custom code.', - 'publish_to: none', - 'version: 0.0.1', - '', - 'environment:', - ' sdk: ${_yamlScalar(sdkConstraint)}', - '', - 'dependencies:', - ' flutter:', - ' sdk: flutter', - ]; - - // `flutter` is always present above; any other SDK package the project - // declares is reproduced the same way. - for (final name in sdkPackages) { - if (name == 'flutter') continue; - lines - ..add(' $name:') - ..add(' sdk: flutter'); - } - - for (final name in dependencies.keys.toList()..sort()) { - lines.add(' $name: ${_yamlScalar(dependencies[name]!)}'); - } - - // SDK-sourced overrides belong in dependency_overrides, not in - // `dependencies` with sdkPackages: an override can share its name with a - // direct dependency, and emitting both under one map writes the key twice. - if (overrides.isNotEmpty || sdkOverrides.isNotEmpty) { - lines - ..add('') - ..add('dependency_overrides:'); - for (final name in overrides.keys.toList()..sort()) { - lines.add(' $name: ${_yamlScalar(overrides[name]!)}'); - } - for (final name in sdkOverrides.toList()..sort()) { - lines - ..add(' $name:') - ..add(' sdk: flutter'); - } - } - - return '${lines.join('\n')}\n'; - } -} - final class _AnalysisOutcome { const _AnalysisOutcome({ required this.errors, @@ -1078,10 +904,10 @@ final class _AnalysisOutcome { }) : skippedReason = null; const _AnalysisOutcome.skipped(String reason) - : errors = const [], - report = '', - verifiedFiles = const [], - skippedReason = reason; + : errors = const [], + report = '', + verifiedFiles = const [], + skippedReason = reason; final List errors; final String report; diff --git a/cloud-run/ffai-runner/lib/request_validation.dart b/cloud-run/ffai-runner/lib/request_validation.dart new file mode 100644 index 0000000..a69342a --- /dev/null +++ b/cloud-run/ffai-runner/lib/request_validation.dart @@ -0,0 +1,324 @@ +/// Request-manifest validation for the FlutterFlow AI deploy runner. +/// +/// The browser sends the verification manifest - the SDK packages, dependency +/// maps, version constraints and sources to compile - across a publicly +/// reachable route, and every field becomes a line the runner writes into a +/// pubspec it then runs `flutter pub get` against. Validation here is the +/// boundary that keeps a caller's data from turning into YAML structure, a +/// different package source, or a path outside the scratch package. The +/// functions are public so the contract tests in test/ can pin the exact +/// production behaviour, including the regression this helper used to cause: +/// sdkPackages is validated as package names, not against a stale allowlist, +/// so `flutter_web_plugins` and `flutter_localizations` are accepted. +/// +/// This file uses no server primitives - it only parses and rejects - so it +/// can be exercised in isolation. +library; + +import 'dart:convert'; + +const maxClassesPerRequest = 20; +const maxCodeBytes = 500000; +const maxDependenciesPerRequest = 200; +// pub.dev's own ceiling on a package name. +const maxPackageNameLength = 64; + +const analysisPackageName = 'ccc_custom_code_analysis'; +const defaultSdkConstraint = '>=3.0.0 <4.0.0'; + +// Pub package names are lower-case identifiers. Matching the whole string is +// what keeps a caller from smuggling YAML structure in through a key. +final _packageNamePattern = RegExp(r'^[a-z_][a-z0-9_]*$'); + +// A pub version constraint, and nothing else. The allowed characters exclude +// `:`, `#`, `{`, `}`, `/` and whitespace beyond single spaces, so `git:`, +// `path:`, a nested `hosted:` mapping, and comment injection are all +// unrepresentable rather than merely discouraged. +final _constraintPattern = RegExp(r'^[A-Za-z0-9^~<>=*+._ -]+$'); + +// Which packages the Flutter SDK supplies is not decided here. A caller names +// the packages its project declares as `sdk: flutter`, and this runner emits +// them as `name: {sdk: flutter}` - a value it writes itself, never one a caller +// sent. There was an allowlist of SDK package names here, and it refused good +// deploys: the client classified a package correctly by its pubspec source, and +// the runner rejected it for not appearing on a list that had gone stale. +// `_packageNamePattern` is what actually keeps a key from carrying YAML. + +/// Reads a `{name: version-constraint}` map, rejecting anything that could +/// express more than a published package at a version. +/// +/// This is the whole reason the manifest is sent as data rather than as +/// pubspec.yaml text. The runner writes this manifest to disk and runs +/// `flutter pub get` against it on a publicly reachable route, so a +/// caller-supplied document could otherwise name a `git:` or `path:` source and +/// have the runner fetch from a host of the caller's choosing. Only bare +/// `name: version` entries can be expressed here, and the runner emits those +/// lines itself, so no source directive can survive into the manifest. +Map normalizeDependencyMap(Object? value, String field) { + if (value == null) return const {}; + if (value is! Map) { + throw FormatException('verification.$field must be an object.'); + } + if (value.length > maxDependenciesPerRequest) { + throw FormatException('Too many entries in verification.$field.'); + } + + final result = {}; + for (final entry in value.entries) { + final name = '${entry.key}'; + if (!isValidPackageName(name)) { + throw FormatException( + 'Invalid package name in verification.$field: $name.', + ); + } + final constraint = '${entry.value}'.trim(); + if (!_constraintPattern.hasMatch(constraint)) { + throw FormatException( + 'Invalid version constraint for $name in verification.$field: $constraint.', + ); + } + result[name] = constraint; + } + return result; +} + +/// Reads the SDK-supplied package names a caller's project declares. +/// +/// Validated as package names rather than against a list of the packages the +/// Flutter SDK happens to ship: a name the SDK does not provide fails +/// `pub get` with an honest error, whereas a list refuses correct deploys the +/// moment it falls behind the SDK. The pattern is the load-bearing check - each +/// name becomes a key in the generated pubspec. +List normalizeSdkPackages(Object? value, String field) { + if (value == null) return const []; + if (value is! List) { + throw FormatException('verification.$field must be an array.'); + } + // Bounded like the dependency maps: each entry expands into the generated + // pubspec, so an unbounded array is caller-controlled work before pub get + // ever sees it. + if (value.length > maxDependenciesPerRequest) { + throw FormatException('Too many entries in verification.$field.'); + } + + final result = {}; + for (final raw in value) { + final name = '$raw'; + if (!isValidPackageName(name)) { + throw FormatException('Invalid SDK package name: $name.'); + } + result.add(name); + } + return result.toList(); +} + +bool isValidPackageName(String name) => + name.length <= maxPackageNameLength && _packageNamePattern.hasMatch(name); + +String validateConstraint( + String field, + String value, { + required String fallback, +}) { + final constraint = value.trim(); + if (constraint.isEmpty) return fallback; + if (!_constraintPattern.hasMatch(constraint)) { + throw FormatException('Invalid $field: $constraint.'); + } + return constraint; +} + +/// Reads a required or optional string field, trimming it and bounding its +/// encoded size. +String stringField( + Map source, + String key, { + required int maxLength, + bool required = true, +}) { + final value = source[key]; + if (value == null || value == '') { + if (required) throw FormatException('Missing $key.'); + return ''; + } + if (value is! String) { + throw FormatException('$key must be a string.'); + } + final trimmed = value.trim(); + if (required && trimmed.isEmpty) { + throw FormatException('Missing $key.'); + } + if (utf8.encode(trimmed).length > maxLength) { + throw FormatException('$key is too long.'); + } + return trimmed; +} + +/// Reads the optional compile-check payload. Absent means the caller is an +/// older client that predates verification; its deploys keep working and are +/// reported as unverified rather than refused. +VerificationRequest? normalizeVerification(Object? value) { + if (value == null) return null; + if (value is! Map) { + throw const FormatException('verification must be an object.'); + } + + // Clients deployed before the manifest became structured sent pubspec.yaml + // text. Running it would reintroduce the caller-controlled-source problem, so + // it is never executed - but rejecting the request outright would break every + // deploy from a still-cached client during the rollout window. The deploy + // goes ahead with the check reported as unavailable, which is exactly what + // the deploy did before this feature existed, and the caller is told. + if (value.containsKey('pubspec') && !value.containsKey('dependencies')) { + return VerificationRequest.unavailable( + 'The compile check was skipped: this client sent a package manifest in ' + 'the older format, which the runner no longer executes. Reload the app ' + 'to pick up the current version.', + ); + } + + final rawSources = value['sources']; + if (rawSources is! List) { + throw const FormatException('verification.sources must be an array.'); + } + if (rawSources.length > maxClassesPerRequest) { + throw const FormatException('Too many sources to verify in one request.'); + } + + final sources = + rawSources.indexed.map((item) { + final raw = item.$2; + if (raw is! Map) { + throw FormatException( + 'verification.sources[${item.$1}] must be an object.', + ); + } + final fileName = stringField(raw, 'fileName', maxLength: 200); + // The name becomes a path inside the scratch package, so anything that + // could climb out of it is a request to write somewhere else. + if (!RegExp(r'^[a-z0-9_]+\.dart$').hasMatch(fileName)) { + throw FormatException('Invalid verification file name: $fileName.'); + } + return VerificationSource( + fileName: fileName, + content: stringField(raw, 'content', maxLength: maxCodeBytes), + ); + }).toList(); + + return VerificationRequest( + sdkConstraint: validateConstraint( + 'verification.sdkConstraint', + stringField(value, 'sdkConstraint', maxLength: 200, required: false), + fallback: defaultSdkConstraint, + ), + dependencies: normalizeDependencyMap(value['dependencies'], 'dependencies'), + overrides: normalizeDependencyMap( + value['dependencyOverrides'], + 'dependencyOverrides', + ), + sdkPackages: normalizeSdkPackages(value['sdkPackages'], 'sdkPackages'), + sdkOverrides: normalizeSdkPackages(value['sdkOverrides'], 'sdkOverrides'), + sources: sources, + ); +} + +/// Quotes a value for YAML when a plain scalar would be misread. +/// +/// `>=1.0.0 <2.0.0` is an ordinary pub range, but it opens with an indicator +/// character and YAML would reject it as a plain scalar - the same reason +/// pubspecSync's `formatConstraint` quotes on the client side. +String _yamlScalar(String value) { + if (RegExp(r'^[A-Za-z0-9][A-Za-z0-9._+-]*$').hasMatch(value)) return value; + return "'${value.replaceAll("'", "''")}'"; +} + +final class VerificationSource { + const VerificationSource({required this.fileName, required this.content}); + + final String fileName; + final String content; +} + +final class VerificationRequest { + const VerificationRequest({ + required this.sdkConstraint, + required this.dependencies, + required this.overrides, + required this.sdkPackages, + required this.sdkOverrides, + required this.sources, + }) : unavailableReason = null; + + /// A request the runner will not compile, carrying why. + const VerificationRequest.unavailable(this.unavailableReason) + : sdkConstraint = '', + dependencies = const {}, + overrides = const {}, + sdkPackages = const [], + sdkOverrides = const [], + sources = const []; + + /// Why this request cannot be compiled, or null when it can. + final String? unavailableReason; + + final String sdkConstraint; + final Map dependencies; + final Map overrides; + final List sdkPackages; + final List sdkOverrides; + final List sources; + + /// Builds the pubspec the scratch package is compiled from. + /// + /// Emitted here rather than accepted from the caller: every line below is + /// either a fixed literal or a value already matched against + /// [_packageNamePattern] / [_constraintPattern], so nothing a caller sends + /// can introduce a new YAML key, a nested mapping, or a comment. + String toPubspec() { + final lines = [ + 'name: $analysisPackageName', + 'description: Throwaway package used to compile generated custom code.', + 'publish_to: none', + 'version: 0.0.1', + '', + 'environment:', + ' sdk: ${_yamlScalar(sdkConstraint)}', + '', + 'dependencies:', + ' flutter:', + ' sdk: flutter', + ]; + + // `flutter` is always present above; any other SDK package the project + // declares is reproduced the same way. + for (final name in sdkPackages) { + if (name == 'flutter') continue; + lines + ..add(' $name:') + ..add(' sdk: flutter'); + } + + for (final name in dependencies.keys.toList()..sort()) { + lines.add(' $name: ${_yamlScalar(dependencies[name]!)}'); + } + + // SDK-sourced overrides belong in dependency_overrides, not in + // `dependencies` with sdkPackages: an override can share its name with a + // direct dependency, and emitting both under one map writes the key twice. + if (overrides.isNotEmpty || sdkOverrides.isNotEmpty) { + lines + ..add('') + ..add('dependency_overrides:'); + for (final name in overrides.keys.toList()..sort()) { + lines.add(' $name: ${_yamlScalar(overrides[name]!)}'); + } + for (final name in sdkOverrides.toList()..sort()) { + lines + ..add(' $name:') + ..add(' sdk: flutter'); + } + } + + return '${lines.join('\n')}\n'; + } +} diff --git a/cloud-run/ffai-runner/pubspec.yaml b/cloud-run/ffai-runner/pubspec.yaml index a18d355..9f06f5a 100644 --- a/cloud-run/ffai-runner/pubspec.yaml +++ b/cloud-run/ffai-runner/pubspec.yaml @@ -5,3 +5,6 @@ publish_to: none environment: sdk: ">=3.7.0 <4.0.0" + +dev_dependencies: + test: ^1.25.0 diff --git a/cloud-run/ffai-runner/test/request_validation_test.dart b/cloud-run/ffai-runner/test/request_validation_test.dart new file mode 100644 index 0000000..db19e64 --- /dev/null +++ b/cloud-run/ffai-runner/test/request_validation_test.dart @@ -0,0 +1,140 @@ +import 'package:ccc_ffai_runner/request_validation.dart'; +import 'package:test/test.dart'; + +/// Contract tests for the runner's request-manifest validation. +/// +/// The production regression this guards: sdkPackages was once validated +/// against a hardcoded allowlist that had gone stale, so a correct deploy +/// declaring `flutter_web_plugins` (or `flutter_localizations`) was rejected +/// with "Unsupported SDK package" and every such deploy failed for months. The +/// list is gone - names are validated as package names, not membership - and +/// these tests pin that so the regression cannot quietly come back. +void main() { + group('normalizeSdkPackages', () { + test('accepts flutter_web_plugins and flutter_localizations', () { + final packages = normalizeSdkPackages([ + 'flutter', + 'flutter_web_plugins', + 'flutter_localizations', + ], 'sdkPackages'); + expect(packages, contains('flutter_web_plugins')); + expect(packages, contains('flutter_localizations')); + }); + + test('rejects a name carrying a dash', () { + expect( + () => normalizeSdkPackages(['flutter-web'], 'sdkPackages'), + throwsA(isA()), + ); + }); + + test('rejects a name carrying a colon', () { + expect( + () => normalizeSdkPackages(['foo:bar'], 'sdkPackages'), + throwsA(isA()), + ); + }); + + test('rejects YAML punctuation (colon, braces, spaces) in a name', () { + expect( + () => normalizeSdkPackages(['foo: {bar: baz}'], 'sdkPackages'), + throwsA(isA()), + ); + }); + + test('isValidPackageName reflects the same rule', () { + expect(isValidPackageName('flutter_web_plugins'), isTrue); + expect(isValidPackageName('flutter-localizations'), isFalse); + expect(isValidPackageName('flutter:localizations'), isFalse); + }); + }); + + group('normalizeDependencyMap', () { + test('accepts a bare name: constraint entry', () { + final map = normalizeDependencyMap({'http': '^1.0.0'}, 'dependencies'); + expect(map, {'http': '^1.0.0'}); + }); + + test('rejects an empty constraint', () { + expect( + () => normalizeDependencyMap({'http': ''}, 'dependencies'), + throwsA(isA()), + ); + }); + + test('rejects a constraint that could smuggle a git, path or comment', () { + for (final bad in [ + 'git: https://example.com/repo.git', + 'path: ../somewhere', + '^1.0.0 # a comment', + ]) { + expect( + () => normalizeDependencyMap({'http': bad}, 'dependencies'), + throwsA(isA()), + reason: 'expected rejection for constraint: $bad', + ); + } + }); + + test('rejects a non-object value and an oversized map', () { + expect( + () => normalizeDependencyMap('nope', 'dependencies'), + throwsA(isA()), + ); + }); + }); + + group('validateConstraint', () { + test('falls back to the default when the field is empty', () { + expect( + validateConstraint( + 'verification.sdkConstraint', + '', + fallback: '>=3.0.0 <4.0.0', + ), + '>=3.0.0 <4.0.0', + ); + }); + + test('rejects a constraint carrying a colon or slash', () { + expect( + () => validateConstraint( + 'verification.sdkConstraint', + '>=3.0.0 git: x', + fallback: '>=3.0.0 <4.0.0', + ), + throwsA(isA()), + ); + }); + }); + + group('normalizeVerification end-to-end', () { + test('accepts a full manifest declaring flutter_web_plugins', () { + final request = normalizeVerification({ + 'sdkPackages': [ + 'flutter', + 'flutter_web_plugins', + 'flutter_localizations', + ], + 'dependencies': {'http': '^1.0.0'}, + 'sources': [ + {'fileName': 'my_class.dart', 'content': 'class MyClass {}'}, + ], + }); + expect(request, isNotNull); + expect(request!.sdkPackages, contains('flutter_web_plugins')); + expect(request.sdkPackages, contains('flutter_localizations')); + expect(request.dependencies, {'http': '^1.0.0'}); + }); + + test('toPubspec emits the sdk package as a source the runner writes', () { + final request = normalizeVerification({ + 'sdkPackages': ['flutter_web_plugins'], + 'sources': [], + }); + final pubspec = request!.toPubspec(); + expect(pubspec, contains(' flutter_web_plugins:')); + expect(pubspec, contains(' sdk: flutter')); + }); + }); +} From 486c23909aa39c78aed722b51116e12d88b99a97 Mon Sep 17 00:00:00 2001 From: Stuart Date: Sun, 4 Oct 2026 15:57:06 +1100 Subject: [PATCH 06/20] fix(deploy): let an unconfirmed deploy whose classes landed finish with 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. --- app.js | 181 ++++++++++++++++++-- src/flutterFlowCodeFileProvisioning.js | 37 ++++ src/flutterFlowCodeFileProvisioning.test.js | 113 ++++++++++++ 3 files changed, 321 insertions(+), 10 deletions(-) diff --git a/app.js b/app.js index d7a85dd..be2abeb 100644 --- a/app.js +++ b/app.js @@ -31,8 +31,10 @@ import { import { applyFlutterFlowHeader } from "./src/flutterFlowHeader.js"; import { buildBundleDeployPlan } from "./src/bundleDeployPlanner.js"; import { + buildFinishPushFileMap, excludeProvisionedCodeFiles, findMissingCodeFiles, + isDeployFinishable, partitionProvisionedCodeFiles, } from "./src/flutterFlowCodeFileProvisioning.js"; import { buildReviewPresentation } from "./src/reviewPresentation.js"; @@ -3219,7 +3221,8 @@ async function reconcileUnconfirmedProvision(apiClient, attempted, original) { message = `Checked FlutterFlow just now: all ${landed.length} custom class(es) are in the project (${names(landed)}). ` + "The final step, pushing your pubspec.yaml dependencies, did not run. " + - "Deploy again to finish it - classes already there are skipped, not duplicated."; + "Finish deploy to push the dependencies without re-writing the classes, " + + "or deploy again - classes already there are skipped, not duplicated."; } else if (landed.length === 0) { message = `Checked FlutterFlow just now: none of the ${notLanded.length} custom class(es) are in the project yet (${names(notLanded)}). ` + @@ -3231,7 +3234,11 @@ async function reconcileUnconfirmedProvision(apiClient, attempted, original) { `not there yet: ${names(notLanded)}. Wait a minute, then deploy again - ` + "classes already there are skipped, and the rest are written."; } - return new UnconfirmedDeployError(message); + const error = new UnconfirmedDeployError(message); + // The reconcile outcome decides whether the terminal can offer "Finish + // deploy": when every class landed, only the pubspec push remains. + error.reconcilePartition = { landed, notLanded }; + return error; } async function provisionMissingCodeFiles( @@ -3422,11 +3429,28 @@ async function provisionMissingCodeFiles( }, () => {}, ); - throw await reconcileUnconfirmedProvision( + const reconciled = await reconcileUnconfirmedProvision( apiClient, missingCodeFiles, error, ); + // When every attempted class landed, offer "Finish deploy": completing + // the deploy only needs step 3 (the pubspec push). Carry what that push + // 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)) { + reconciled.finishDeploy = { + apiKey: apiClient.apiKey, + projectId: apiClient.projectId, + endpoint: apiClient.baseUrl, + branchName: apiClient.branchName, + fileMap, + serializedYaml: pubspecYaml, + landed: reconciled.reconcilePartition.landed, + }; + } + throw reconciled; } throw error; } @@ -4542,6 +4566,9 @@ async function executeCommit(code, options = {}) { targetIdentity, state: commitState.currentState, elapsedTime: commitState.getElapsedTime(), + ...(error.finishDeploy + ? { finishDeploy: mergeFinishDeployContext(error.finishDeploy, targetIdentity, commitState) } + : {}), }; } @@ -4749,6 +4776,9 @@ async function executeBundleCommit(bundlePlan, options = {}) { targetIdentity, state: commitState.currentState, elapsedTime: commitState.getElapsedTime(), + ...(error.finishDeploy + ? { finishDeploy: mergeFinishDeployContext(error.finishDeploy, targetIdentity, commitState) } + : {}), }; } @@ -7804,17 +7834,147 @@ function showCommitPartialModal(result) { * in-flight write is never cancelled. */ function showCommitUnconfirmedModal(result) { + let heading = "Deploy outcome not yet known"; + let title = + "This browser stopped waiting before the FlutterFlow server reported a result."; + let guidance = + "The deploy may still be finishing on the server — it was not cancelled. " + + "Open your FlutterFlow project and confirm whether the class landed before retrying, so you do not " + + "push a duplicate or build on top of an unknown state."; + + // When a reconcile proved every class this deploy was writing landed, only + // the pubspec push is missing. Offer it here instead of a blind full + // re-deploy, which would re-run the slower class write for nothing. + if (result.finishDeploy) { + pendingFinishDeploy = result.finishDeploy; + heading = "Deploy almost complete"; + title = + "Your custom class(es) are in FlutterFlow — only the dependency push remains."; + guidance = + "Every class this deploy was writing is already in your project. The one step that did not run " + + "was pushing your pubspec.yaml dependencies, which the project needs to build. " + + ""; + } + populateCommitTerminalModal(result, { - heading: "Deploy outcome not yet known", - title: - "This browser stopped waiting before the FlutterFlow server reported a result.", - guidance: - "The deploy may still be finishing on the server — it was not cancelled. " + - "Open your FlutterFlow project and confirm whether the class landed before retrying, so you do not " + - "push a duplicate or build on top of an unknown state.", + heading, + title, + guidance, }); } +// The "Finish deploy" action completes an interrupted deploy whose classes +// already landed. Holds the push context (credentials, file map, merged yaml, +// landed entries) plus the terminal template so the push renders like any +// deploy. Set by showCommitUnconfirmedModal, read by finishProvisionedDeploy. +let pendingFinishDeploy = null; + +/** + * Merges the reconcile's finish-push context — built in + * `provisionMissingCodeFiles` where the api client, file map, and merged yaml + * are in scope — with the current deploy's terminal identity, so a finished + * deploy renders the same truthful terminal a normal deploy would. + * @param {Object} context - The finishDeploy push context on the reconcile error + * @param {Object} targetIdentity - The interrupted deploy's target identity + * @param {Object} commitState - Holds the elapsed-time for the terminal render + * @returns {Object} The merged finishDeploy payload + */ +function mergeFinishDeployContext(context, targetIdentity, commitState) { + return { + ...context, + result: { + targetIdentity, + metadata: { + artifactType: targetIdentity?.artifactType, + artifactName: targetIdentity?.artifactName, + fileName: targetIdentity?.fileName, + projectId: targetIdentity?.projectId, + }, + message: + `Deploy finished: ${targetIdentity?.artifactName || "custom classes"} and ` + + "their dependencies are now in FlutterFlow.", + addedDependencies: [], + unverified: [], + approximate: [], + warnings: [], + elapsedTime: commitState.getElapsedTime(), + }, + }; +} + +/** + * Completes an interrupted deploy whose classes already landed by running only + * step 3 — the pubspec push. The landed classes are excluded from the sync map + * (they are already in the project), so this never re-provisions them, and the + * push reuses the same file-map/zip/push machinery as a normal deploy. + */ +async function finishProvisionedDeploy() { + const finish = pendingFinishDeploy; + if (!finish) return; + if (deployInFlight) return; + pendingFinishDeploy = null; + + const apiClient = new FlutterFlowApiClient( + finish.apiKey, + finish.projectId, + finish.branchName, + finish.endpoint, + ); + + closeCommitTerminalModal(); + setDeployBusy(true); + showCommitProgress(); + + try { + commitProgress.set("package"); + const finishFileMap = buildFinishPushFileMap( + finish.fileMap, + finish.landed, + finish.serializedYaml, + ); + const syncMetadata = await buildApiSyncMetadata(finishFileMap, new Map()); + const zippedCustomCode = await createZipFromFileMap(finishFileMap); + + const pushRequest = { + project_id: finish.projectId, + zipped_custom_code: zippedCustomCode, + uid: `web_${Date.now()}`, + branch_name: apiClient.branchName, + serialized_yaml: finish.serializedYaml, + file_map: syncMetadata.fileMapContents, + functions_map: syncMetadata.functionsMapContents, + }; + + invalidateProjectSourceCache(apiClient); + commitProgress.set("push"); + const result = await pushCodeWithUiTimeout(apiClient, pushRequest); + + if (result.success) { + renderCommitTerminal({ + ...finish.result, + success: true, + warnings: result.errorMap ? Array.from(result.errorMap.entries()) : [], + }); + return; + } + const error = new Error( + result.errorMessage || getFlutterFlowErrorMessage(result.responseCode), + ); + error.remoteRefusal = true; + throw error; + } catch (error) { + renderCommitTerminal({ + ...finish.result, + success: false, + error: error.message, + unconfirmed: error instanceof UnconfirmedDeployError, + remoteRefusal: error.remoteRefusal === true, + }); + } +} + /** * Closes the shared terminal modal (partial / unconfirmed outcomes). * @param {Event} [event] - Optional click event @@ -8229,6 +8389,7 @@ window.openCommitConfirmModal = openCommitConfirmModal; window.closeCommitConfirmModal = closeCommitConfirmModal; window.closeCommitSuccessModal = closeCommitSuccessModal window.closeCommitTerminalModal = closeCommitTerminalModal +window.finishProvisionedDeploy = finishProvisionedDeploy window.showCommitPartialModal = showCommitPartialModal window.showCommitUnconfirmedModal = showCommitUnconfirmedModal window.showCommitSuccessModal = showCommitSuccessModal diff --git a/src/flutterFlowCodeFileProvisioning.js b/src/flutterFlowCodeFileProvisioning.js index 49f40e5..99a93b4 100644 --- a/src/flutterFlowCodeFileProvisioning.js +++ b/src/flutterFlowCodeFileProvisioning.js @@ -72,3 +72,40 @@ export function excludeProvisionedCodeFiles(fileMap, entries) { ), ); } + +/** + * Whether every class a provisioning request attempted is now present in the + * project's fresh export. When true the classes have landed and only step 3 — + * the pubspec push — remains, so the interrupted deploy is finishable without + * re-running the slower class write. + * @param {{landed: Array, notLanded: Array}} partition - Outcome of + * `partitionProvisionedCodeFiles` + * @returns {boolean} + */ +export function isDeployFinishable(partition) { + return partition.notLanded.length === 0 && partition.landed.length > 0; +} + +/** + * Builds the file map for a "finish deploy" — the step-3 push that completes + * a deploy whose classes have already landed but whose provisioning response + * was lost. The landed classes are excluded: they are already in the project, + * so re-writing them would needlessly run the class/dependency handling they + * just went through. The merged pubspec is added as the remaining entry. This + * mirrors what the normal step-3 request already does, since it also excludes + * classes the runner wrote (see `provisionMissingCodeFiles`). + * @param {Map} fileMap - Entries a provisioning request was writing + * @param {Array<{path: string}>} landedEntries - Entries confirmed present in the project + * @param {string} serializedYaml - Merged pubspec.yaml to push + * @returns {Map} The finish-push file map + */ +export function buildFinishPushFileMap(fileMap, landedEntries, serializedYaml) { + const remaining = excludeProvisionedCodeFiles(fileMap, landedEntries); + const out = new Map(remaining); + out.set("pubspec.yaml", { + content: serializedYaml, + type: "D", + path: "pubspec.yaml", + }); + return out; +} diff --git a/src/flutterFlowCodeFileProvisioning.test.js b/src/flutterFlowCodeFileProvisioning.test.js index d6849b6..29fe809 100644 --- a/src/flutterFlowCodeFileProvisioning.test.js +++ b/src/flutterFlowCodeFileProvisioning.test.js @@ -1,8 +1,10 @@ import assert from "node:assert/strict"; import test from "node:test"; import { + buildFinishPushFileMap, excludeProvisionedCodeFiles, findMissingCodeFiles, + isDeployFinishable, partitionProvisionedCodeFiles, } from "./flutterFlowCodeFileProvisioning.js"; @@ -131,3 +133,114 @@ test("partitionProvisionedCodeFiles splits attempted entries by fresh export", ( 2, ); }); + +test("isDeployFinishable is true only when every attempted class landed", () => { + const allLanded = partitionProvisionedCodeFiles( + [ + { className: "Alpha", path: "lib/custom_code/alpha.dart" }, + { className: "Beta", path: "lib/custom_code/beta.dart" }, + ], + new Map([ + ["lib/custom_code/alpha.dart", "class Alpha {}"], + ["lib/custom_code/beta.dart", "class Beta {}"], + ]), + ); + assert.equal(isDeployFinishable(allLanded), true); + + const partlyLanded = partitionProvisionedCodeFiles( + [ + { className: "Alpha", path: "lib/custom_code/alpha.dart" }, + { className: "Beta", path: "lib/custom_code/beta.dart" }, + ], + new Map([["lib/custom_code/alpha.dart", "class Alpha {}"]]), + ); + assert.equal(isDeployFinishable(partlyLanded), false); + + const nothingLanded = partitionProvisionedCodeFiles( + [{ className: "Alpha", path: "lib/custom_code/alpha.dart" }], + new Map(), + ); + assert.equal(isDeployFinishable(nothingLanded), false); +}); + +// DONE CRITERION (client): a deploy whose step-2 response is lost must still +// be finishable — the classes are present and the finish action pushes the +// pubspec without re-provisioning them. These tests drive the exact pure +// decision path app.js uses after a timed-out provision: the re-read splits +// the attempted entries, finishability comes from all of them landing, and +// the finish file map carries only the merged pubspec. +test("finish action after an all-landed reconcile pushes the pubspec only", () => { + const alphaPath = "lib/custom_code/alpha.dart"; + const fileMap = new Map([ + [ + "alpha.dart", + { + artifactName: "AlphaGen", + content: "class AlphaGen {}", + type: "C", + path: alphaPath, + }, + ], + ]); + const mergedYaml = "name: app\ndependencies:\n intl: ^0.19.0\n"; + + // Step 2's response was lost; a fresh project export shows the class landed. + const partition = partitionProvisionedCodeFiles( + [ + { className: "AlphaGen", path: alphaPath }, + ], + new Map([[alphaPath, "class AlphaGen {}"]]), + ); + + assert.equal(isDeployFinishable(partition), true); + + const finishMap = buildFinishPushFileMap( + fileMap, + partition.landed, + mergedYaml, + ); + + // The pubspec is present and is the merged yaml (the pubspec push happens)... + assert.deepEqual(Array.from(finishMap.keys()), ["pubspec.yaml"]); + assert.deepEqual(finishMap.get("pubspec.yaml"), { + content: mergedYaml, + type: "D", + path: "pubspec.yaml", + }); + // ...and the landed class is not re-provisioned (absent from the sync map). + assert.equal(finishMap.has("alpha.dart"), false); +}); + +test("finish file map excludes every landed class but keeps not-yet-landed ones", () => { + const alphaPath = "lib/custom_code/alpha.dart"; + const betaPath = "lib/custom_code/beta.dart"; + const fileMap = new Map([ + [ + "alpha.dart", + { type: "C", path: alphaPath }, + ], + [ + "beta.dart", + { type: "C", path: betaPath }, + ], + ]); + + const partition = partitionProvisionedCodeFiles( + [{ path: alphaPath }, { path: betaPath }], + new Map([[alphaPath, "class Alpha {}"]]), + ); + + const finishMap = buildFinishPushFileMap( + fileMap, + partition.landed, + "name: app\n", + ); + + // The landed class is excluded; the not-yet-landed one stays so a retry can + // still finish it, and the pubspec is added. + assert.deepEqual(Array.from(finishMap.keys()).sort(), [ + "beta.dart", + "pubspec.yaml", + ]); +}); + From 6141102bc302578ce8e589a318df515f776031c8 Mon Sep 17 00:00:00 2001 From: Stuart Date: Sun, 4 Oct 2026 15:59:40 +1100 Subject: [PATCH 07/20] fix(canary): send a sources array so the runner accepts the dry-run payload The deployed runner's _normalizeVerification hard-requires verification.sources to be an array and throws FormatException otherwise, which the handler turns into HTTP 400 before workspace init, the compile gate, or ai validate ever run. The canary omitted sources entirely, so every scheduled run was guaranteed to fail with 'verification.sources must be an array.' - a permanently-red canary that could not tell a healthy runner from a broken one, and that never actually reached the compile gate it claimed to exercise. Include the canary source as the single probe file; a non-empty sources list is also what unskips _verifyCustomCode (server.dart returns null on empty sources), so the compile gate now truly runs on every scheduled check. --- scripts/canary-deploy.mjs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/scripts/canary-deploy.mjs b/scripts/canary-deploy.mjs index 71d9ab7..6be1644 100644 --- a/scripts/canary-deploy.mjs +++ b/scripts/canary-deploy.mjs @@ -64,6 +64,11 @@ async function main() { dependencies: {}, dependencyOverrides: {}, sdkPackages: ["flutter_web_plugins"], + // The runner hard-requires this array: without it _normalizeVerification + // throws and returns HTTP 400 before anything runs. It is also what the + // compile gate compiles - an empty sources list skips _verifyCustomCode + // entirely, so this canary would detect nothing. + sources: [{ fileName: "canary_probe.dart", content: CANARY_SOURCE }], }, }; From 6bde72ffb6612b645804b05b446470ee706c33ce Mon Sep 17 00:00:00 2001 From: Stuart Date: Sun, 4 Oct 2026 16:06:03 +1100 Subject: [PATCH 08/20] fix(runner): make PR 114 independently deployable and keep the node suite green The verification extraction moved request validation into lib/request_validation.dart, but the deployment unit was left two pieces short: - Dockerfile copied only pubspec.yaml and bin/, so the built image could not resolve package:ccc_ffai_runner/request_validation.dart and every endpoint (including GET /healthz) failed to start. Copy lib/ into the image so the package is complete at build time. - The node contract test read only bin/server.dart; the manifest invariants it asserts (toPubspec, the constraint/package patterns, sdkOverrides channeling, legacy-manifest skip) now live in lib/request_validation.dart, and the unavailable symbol was renamed to VerificationRequest.unavailable. Read both sources so the server-side-construction invariants are actually checked, and match the renamed symbol. Reproduced the image startup failure (import not found + Type 'VerificationRequest' not found) with the old Dockerfile layout; both resolve once lib/ is present and the server boots on 8080. Node suite back to 399/399. --- cloud-run/ffai-runner/Dockerfile | 1 + src/cloudRunRunner.test.js | 14 +++++++++----- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/cloud-run/ffai-runner/Dockerfile b/cloud-run/ffai-runner/Dockerfile index 2a14587..422b653 100644 --- a/cloud-run/ffai-runner/Dockerfile +++ b/cloud-run/ffai-runner/Dockerfile @@ -33,6 +33,7 @@ RUN dart pub get RUN mkdir -p /workspace COPY bin/ ./bin/ +COPY lib/ ./lib/ EXPOSE 8080 diff --git a/src/cloudRunRunner.test.js b/src/cloudRunRunner.test.js index dfdb100..4c0f5fb 100644 --- a/src/cloudRunRunner.test.js +++ b/src/cloudRunRunner.test.js @@ -6,10 +6,14 @@ const dockerfile = readFileSync( new URL("../cloud-run/ffai-runner/Dockerfile", import.meta.url), "utf8", ); -const runnerSource = readFileSync( - new URL("../cloud-run/ffai-runner/bin/server.dart", import.meta.url), - "utf8", -); +// Request validation lives in lib/request_validation.dart (server.dart imports +// it), so the contract assertions must read both the entrypoint and the module. +const runnerSource = [ + "../cloud-run/ffai-runner/bin/server.dart", + "../cloud-run/ffai-runner/lib/request_validation.dart", +] + .map((path) => readFileSync(new URL(path, import.meta.url), "utf8")) + .join("\n"); const deployScript = readFileSync( new URL("../scripts/deploy_cloud_run_ffai.sh", import.meta.url), "utf8", @@ -158,7 +162,7 @@ test("Cloud Run builds the analysis manifest itself rather than running caller-s ); assert.match( runnerSource, - /_VerificationRequest\.unavailable\(/, + /VerificationRequest\.unavailable\(/, "the legacy path must report itself unavailable", ); }); From 19e90fae5f1021c493115bebfe89631eb8f07373 Mon Sep 17 00:00:00 2001 From: Stuart Date: Sun, 4 Oct 2026 16:16:04 +1100 Subject: [PATCH 09/20] fix(ci): gate the runner workflows on a step, not a job-level secret if Both workflows gated the whole job with `if: secrets.X != ''`. The `secrets` context is not available in `jobs..if` (only github, needs, vars and inputs are), so both files were invalid and the workflows could not run at all - the canary and the merge-deploy would have silently never executed, which is the same class of failure they exist to catch. The gate is now a first step that reads the secret from env and publishes `present`; every step that needs the key skips while it does not. Behaviour is unchanged: before one-time setup runs, the jobs skip quietly and log a notice. Also passes RUNNER_GIT_SHA to the service so /healthz reports which revision is live, and documents it. --- .github/workflows/canary.yml | 24 ++++++++++++++++++++---- .github/workflows/deploy-runner.yml | 28 +++++++++++++++++++++++----- DEPLOYMENT.md | 6 ++++++ scripts/deploy_cloud_run_ffai.sh | 11 ++++++++++- 4 files changed, 59 insertions(+), 10 deletions(-) diff --git a/.github/workflows/canary.yml b/.github/workflows/canary.yml index 639d239..26d1072 100644 --- a/.github/workflows/canary.yml +++ b/.github/workflows/canary.yml @@ -17,21 +17,37 @@ permissions: jobs: canary: - # Skip (rather than fail noisily) until the FlutterFlow API key secret - # exists, so scheduled runs before one-time setup complete are quiet. The - # script itself also fails clearly if the key ever comes back empty. - if: secrets.CANARY_FF_API_KEY != '' runs-on: ubuntu-latest steps: + # Skip (rather than fail noisily) until the FlutterFlow API key secret + # exists, so scheduled runs before one-time setup complete are quiet. The + # gate has to be a step: the `secrets` context is not available in a + # job-level `if`, and using it there makes the workflow file invalid. The + # script itself also fails clearly if the key ever comes back empty. + - name: Check for canary credentials + id: creds + env: + CANARY_KEY: ${{ secrets.CANARY_FF_API_KEY }} + run: | + if [ -n "$CANARY_KEY" ]; then + echo "present=true" >> "$GITHUB_OUTPUT" + else + echo "present=false" >> "$GITHUB_OUTPUT" + echo "::notice::CANARY_FF_API_KEY is not set, so the canary did not run. See DEPLOYMENT.md for the one-time setup." + fi + - name: Checkout + if: steps.creds.outputs.present == 'true' uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 - name: Set up Node.js + if: steps.creds.outputs.present == 'true' uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4 with: node-version-file: ".nvmrc" - name: Run the dry-run canary + if: steps.creds.outputs.present == 'true' run: node scripts/canary-deploy.mjs env: CANARY_FF_API_KEY: ${{ secrets.CANARY_FF_API_KEY }} diff --git a/.github/workflows/deploy-runner.yml b/.github/workflows/deploy-runner.yml index 78cf164..db86562 100644 --- a/.github/workflows/deploy-runner.yml +++ b/.github/workflows/deploy-runner.yml @@ -22,28 +22,46 @@ concurrency: jobs: deploy-runner: - # The whole point of gating on the secret instead of failing is that pushes - # before one-time setup complete are skipped quietly. Until - # CCC_FFAI_RUNNER_SA_KEY_JSON exists (see DEPLOYMENT.md) this job does not - # run; once it does, a merged cloud-run change deploys. - if: secrets.CCC_FFAI_RUNNER_SA_KEY_JSON != '' runs-on: ubuntu-latest steps: + # The secret cannot gate the job itself: the `secrets` context is not + # available in a job-level `if` (only github/needs/vars/inputs are), and + # using it there makes the whole workflow file invalid. It is available to + # a step, so the gate is a first step that reports whether the key exists + # and every step that needs it skips while it does not. The intent is + # unchanged: pushes before one-time setup complete are skipped quietly + # rather than failing. + - name: Check for deploy credentials + id: creds + env: + SA_KEY_JSON: ${{ secrets.CCC_FFAI_RUNNER_SA_KEY_JSON }} + run: | + if [ -n "$SA_KEY_JSON" ]; then + echo "present=true" >> "$GITHUB_OUTPUT" + else + echo "present=false" >> "$GITHUB_OUTPUT" + echo "::notice::CCC_FFAI_RUNNER_SA_KEY_JSON is not set, so the runner was not deployed. See DEPLOYMENT.md for the one-time setup." + fi + - name: Checkout + if: steps.creds.outputs.present == 'true' uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 - name: Authenticate to Google Cloud + if: steps.creds.outputs.present == 'true' uses: google-github-actions/auth@c200f3691d83b41bf9bbd8638997a462592937ed # v2 with: # JSON of a service account with roles/run.admin + roles/cloudbuild.builds.editor + roles/iam.serviceAccountUser. credentials_json: ${{ secrets.CCC_FFAI_RUNNER_SA_KEY_JSON }} - name: Set up gcloud + if: steps.creds.outputs.present == 'true' uses: google-github-actions/setup-gcloud@e427ad8a34f8676edf47cf7d7925499adf3eb74f # v2 with: project_id: ${{ vars.CCC_PROJECT_ID || 'low-code-connect' }} - name: Deploy the FlutterFlow AI runner + if: steps.creds.outputs.present == 'true' run: | PROJECT_ID="${{ vars.CCC_PROJECT_ID || 'low-code-connect' }}" \ REGION="${{ vars.CCC_REGION || 'us-west1' }}" \ diff --git a/DEPLOYMENT.md b/DEPLOYMENT.md index 32afc19..0ef90b2 100644 --- a/DEPLOYMENT.md +++ b/DEPLOYMENT.md @@ -196,6 +196,12 @@ on any non-200 response, on `success:false`, or if the runner rejects the `flutter_web_plugins` SDK package, and also reports the runner's `/healthz` versions when that endpoint is deployed - without failing when it is not yet. +`GET /healthz` answers `{sha, flutterflowCli, minFlutterflowCli}`. The deploy +script passes the checkout's short SHA as `RUNNER_GIT_SHA`, so comparing `sha` +against `git log -1 --format=%h` tells you whether a merged runner fix is +actually live. That question is what a months-long stall in a merged-but-never- +deployed fix looked like; it is now one request. + Until the FlutterFlow API key secret exists the canary job skips cleanly, so it never fails noisily before setup: diff --git a/scripts/deploy_cloud_run_ffai.sh b/scripts/deploy_cloud_run_ffai.sh index ac521b1..ae5580c 100755 --- a/scripts/deploy_cloud_run_ffai.sh +++ b/scripts/deploy_cloud_run_ffai.sh @@ -16,6 +16,12 @@ TIMEOUT="${TIMEOUT:-900}" # a warm instance does that once at container boot instead of per request. The # tradeoff is a small always-on instance cost for a much shorter deploy. MIN_INSTANCES="${MIN_INSTANCES:-1}" +# Reported by the runner's /healthz, so which revision is live - and therefore +# whether a merged fix actually shipped - is answerable in seconds instead of +# guessed at. This is exactly how a merged-but-never-deployed runner fix went +# unnoticed for months. Taken from the build's own checkout; empty (never +# wrong) if git is unavailable. +RUNNER_GIT_SHA="${RUNNER_GIT_SHA:-$(git rev-parse --short HEAD 2>/dev/null || true)}" if [[ -z "$PROJECT_ID" ]]; then PROJECT_ID="$(gcloud config get-value project 2>/dev/null || true)" @@ -35,7 +41,10 @@ gcloud run deploy "$SERVICE" \ --concurrency "$CONCURRENCY" \ --min-instances "$MIN_INSTANCES" \ --timeout "$TIMEOUT" \ - --set-env-vars "ALLOWED_ORIGIN=$ALLOWED_ORIGIN" + --set-env-vars "ALLOWED_ORIGIN=$ALLOWED_ORIGIN,RUNNER_GIT_SHA=$RUNNER_GIT_SHA" + +echo +echo "Deployed revision reports RUNNER_GIT_SHA=${RUNNER_GIT_SHA:-}" echo echo "Set VITE_FLUTTERFLOW_DSL_DEPLOY_ENDPOINT to:" From 862241ea53142acf7463a31833038c2aa7e8f609 Mon Sep 17 00:00:00 2001 From: Stuart Date: Sun, 4 Oct 2026 16:39:52 +1100 Subject: [PATCH 10/20] test(e2e): prove an interrupted deploy finishes end-to-end via the real step-3 push The "step-2 timeout ends with classes AND pubspec both present" criterion was only proven at the unit level (isDeployFinishable, buildFinishPushFileMap). This drives the real handlers: provision stalls past a shortened UI bound while the class has landed, reconcile arms the "Finish deploy" terminal, and running that action pushes the merged pubspec (asserted on the intercepted syncCustomCodeChanges request body), issues no second /deployCustomClasses request, and ends the terminal in a finished state. --- e2e/finish-deploy-e2e.spec.js | 210 ++++++++++++++++++++++++++++++++++ 1 file changed, 210 insertions(+) create mode 100644 e2e/finish-deploy-e2e.spec.js diff --git a/e2e/finish-deploy-e2e.spec.js b/e2e/finish-deploy-e2e.spec.js new file mode 100644 index 0000000..e408a47 --- /dev/null +++ b/e2e/finish-deploy-e2e.spec.js @@ -0,0 +1,210 @@ +import { test, expect } from "@playwright/test"; +import JSZip from "jszip"; +import { + applyDefaultRoutes, + ENDPOINTS, + customCodeSync, +} from "./fixtures/apiFixtures.js"; + +/** + * E2E proof of the "finish deploy" action (hardening/finish-deploy-e2e). + * + * The stated done criterion — "a simulated step-2 timeout ends with classes AND + * pubspec both present" — was previously only exercised through the pure + * decision helpers (isDeployFinishable, buildFinishPushFileMap). Here it is + * proven through the actual handlers that perform the work: + * + * 1. A provisioning request to the runner stalls past a shortened UI bound + * while the class it was writing HAS landed in the project export. + * __CCC_PROVISION_CUSTOM_CLASSES__ drives the real provision transport + * (fetch -> stream read -> UI bound -> reconcile), which is the same real + * code path the terminal combos above already rely on; the real browser + * "confirm deploy" entry point is unreachable without a full generate -> + * review -> confirm journey, so the transport hook is used to start the + * flow, exactly as stu380-deploy.spec.js and deploy-timeout-reconcile.spec.js + * already do. + * 2. The reconcile proves every class landed, so the terminal offers + * "Finish deploy". The error's finishDeploy context is merged exactly as + * app.js does (mergeFinishDeployContext) and shown through + * window.showCommitUnconfirmedModal — the same dispatch the real + * renderCommitTerminal uses for an UNCONFIRMED outcome — which arms the + * real window.finishProvisionedDeploy action behind the on-screen button. + * 3. Running that action performs step 3 only. We assert on the INTERCEPTED + * syncCustomCodeChanges request body (serialized_yaml carries the merged + * dependency), that NO second request reaches /deployCustomClasses (the + * landed class is not re-provisioned), and that the terminal ends in a + * finished state rather than still-unknown. + */ + +const identity = { + projectId: "ff-proj-0007", + endpoint: "https://api.flutterflow.io", + artifactType: "CustomClass", + artifactName: "GaugeWidget", + fileName: "gauge_widget.dart", +}; + +// The single class the runner "writes" before its response is lost. +const attemptedClass = { + "gauge_widget.dart": { + artifactName: "GaugeWidget", + content: + "class GaugeWidget { final int value; GaugeWidget(this.value); }", + type: "C", + path: "lib/custom_code/gauge_widget.dart", + }, +}; + +// The pubspec the deploy merged locally before provisioning — the one the +// finish push must carry to FlutterFlow (step 3). +const mergedPubspec = `name: my_app +environment: + sdk: ">=3.0.0 <4.0.0" +dependencies: + flutter: + sdk: flutter + http: ^1.2.0 +`; + +test("an interrupted deploy finishes via the real step-3 push, pushes the merged pubspec, and does not re-provision the landed class", async ({ page }) => { + await page.addInitScript(() => { + localStorage.setItem("hasSeenWalkthrough", "true"); + }); + await applyDefaultRoutes(page); + + // The project export after the timeout contains the class the runner wrote — + // what reconcile reads back to decide every class landed. + const zip = new JSZip(); + zip.file("pubspec.yaml", mergedPubspec); + zip.file( + "lib/custom_code/gauge_widget.dart", + attemptedClass["gauge_widget.dart"].content, + ); + const projectZip = await zip.generateAsync({ type: "base64" }); + await page.route("**/exportCode", async (route) => { + await route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify({ value: { project_zip: projectZip } }), + }); + }); + + // The runner accepts the connection but never answers: provisioning times + // out at the (shortened) UI bound while the class may already be written. + // Every hit is counted so we can prove the finish action adds no further one. + let provisionCalls = 0; + await page.route(ENDPOINTS.deployCustomClasses, () => { + provisionCalls += 1; + return new Promise(() => {}); + }); + + // Capture the step-3 push request exactly as it leaves the page. + let pushBody = null; + await page.route("**/syncCustomCodeChanges", async (route) => { + pushBody = JSON.parse(route.request().postData() || "{}"); + await route.fulfill(customCodeSync()); + }); + + await page.goto("/"); + await page.waitForFunction( + () => + typeof window.__CCC_PROVISION_CUSTOM_CLASSES__ === "function" && + typeof window.showCommitUnconfirmedModal === "function", + ); + + // Drive the real provision transport to an unconfirmed timeout, then arm the + // real finish-deploy terminal exactly as the production flow does. + const reconcile = await page.evaluate( + async ({ fileMap, pubspecYaml, uiTimeoutMs, identity }) => { + let error; + try { + await window.__CCC_PROVISION_CUSTOM_CLASSES__({ + fileMap, + pubspecYaml, + uiTimeoutMs, + }); + return { settled: "resolved" }; + } catch (e) { + error = e; + } + const fd = error.finishDeploy; + if (!fd) { + return { settled: "rejected", name: error.name, finishable: false }; + } + // Mirror app.js mergeFinishDeployContext: gives the finished terminal the + // identity and the "Deploy finished" message a normal deploy would show. + const merged = { + ...fd, + result: { + targetIdentity: identity, + metadata: { + artifactType: identity.artifactType, + artifactName: identity.artifactName, + fileName: identity.fileName, + projectId: identity.projectId, + }, + message: `Deploy finished: ${identity.artifactName || "custom classes"} and their dependencies are now in FlutterFlow.`, + addedDependencies: [], + unverified: [], + approximate: [], + warnings: [], + elapsedTime: 0, + }, + }; + window.showCommitUnconfirmedModal({ + success: false, + unconfirmed: true, + targetIdentity: identity, + error: error.message, + finishDeploy: merged, + }); + return { + settled: "rejected", + name: error.name, + finishable: true, + mergedYamlHasDep: fd.serializedYaml.includes("http: ^1.2.0"), + landedPaths: fd.landed.map((entry) => entry.path), + }; + }, + { fileMap: attemptedClass, pubspecYaml: mergedPubspec, uiTimeoutMs: 60, identity }, + ); + + // The provision timed out unconfirmed and the reconcile deemed it finishable. + expect(reconcile.settled).toBe("rejected"); + expect(reconcile.name).toBe("UnconfirmedDeployError"); + expect(reconcile.finishable).toBe(true); + expect(reconcile.landedPaths).toEqual(["lib/custom_code/gauge_widget.dart"]); + // The finish context keeps the merged pubspec — this is what step 3 must push. + expect(reconcile.mergedYamlHasDep).toBe(true); + // Only the original stalled provision reached the runner so far. + expect(provisionCalls).toBe(1); + + // The unconfirmed terminal appears with the finish action, and it names the + // landed class rather than leaving the outcome unknown. + const terminal = page.locator("#commit-terminal-modal"); + await expect(terminal).toBeVisible(); + await expect(terminal.locator("#terminal-heading")).toContainText( + "Deploy almost complete", + ); + await expect(terminal).toContainText("GaugeWidget"); + await expect(page.locator("#terminal-finish-deploy")).toBeVisible(); + + // Run the action — the real window.finishProvisionedDeploy behind the button. + await page.click("#terminal-finish-deploy"); + + // The finished terminal replaces the unknown one with a success claim. + const success = page.locator("#commit-success-modal"); + await expect(success).toBeVisible(); + await expect(page.locator("#success-message")).toContainText("Deploy finished"); + + // Assertion 1: the step-3 push really carried the merged pubspec dependency. + expect(pushBody).not.toBeNull(); + expect(pushBody.serialized_yaml).toContain("http: ^1.2.0"); + // The landed class is excluded from the finish-push file map (only the + // pubspec dependency entry remains, and it travels via serialized_yaml, not + // the file_map), so the push carries no entry for the already-landed class. + expect(JSON.stringify(pushBody.file_map)).not.toContain("gauge_widget.dart"); + + // Assertion 2: finishing did not re-provision the landed class. + expect(provisionCalls).toBe(1); +}); From 2c70ce298ae5cb75a486730afec66d93bb91389c Mon Sep 17 00:00:00 2001 From: Stuart Date: Sun, 4 Oct 2026 16:51:34 +1100 Subject: [PATCH 11/20] fix(e2e/stu380): account for the reconcile re-read in transport outcome counts Commit 5231e76 made every unconfirmed provision perform one extra project export (reconcileUnconfirmedProvision) to report what actually landed. Six STU-380 transport regression tests counted project exports with exact 'expect(exportCalls).toBe(N)' assertions and were each off by one (or, for the 502 test, still asserted the pre-reconcile message). Update them: - Every count after an unconfirmed provision shifts by the one reconcile re-read; confirmed/success paths add no read, so multi-provision tests (1171, 1282) shift by exactly one, not by one per provision. - The gateway-502 test now asserts the reconcile message (name of the re-read result and the re-read-before-retry advice) instead of the raw 'HTTP 502', while keeping the no-fabricated-'dropped' guard. Proved the tests still bite: with the reconcile read stubbed to a no-op in a scratch copy, exactly these six tests fail (31 passed / 6 failed); the expectations depend on the reconcile read, so removing it fails them. app.js is untouched. Full e2e suite green (262 passed). --- e2e/stu380-deploy.spec.js | 65 +++++++++++++++++++++++---------------- 1 file changed, 39 insertions(+), 26 deletions(-) diff --git a/e2e/stu380-deploy.spec.js b/e2e/stu380-deploy.spec.js index c417c7d..f80eafb 100644 --- a/e2e/stu380-deploy.spec.js +++ b/e2e/stu380-deploy.spec.js @@ -985,9 +985,11 @@ test.describe("STU-380 transport outcome regressions", () => { expect(outcome.name).toBe("UnconfirmedDeployError"); // The next deploy must re-export the project — a stale snapshot would lack - // gauge_model.dart and provision it a second time. + // gauge_model.dart and provision it a second time. One export already + // happened during the deploy: the reconcile re-read of the unconfirmed + // provision, which is why this is 3 (seed 1 + reconcile 1 + this reset). await page.evaluate(() => window.__CCC_RESOLVE_PROJECT_PUBSPEC__({})); - expect(exportCalls).toBe(2); + expect(exportCalls).toBe(3); }); test("a gateway 502 keeps the project snapshot dirty instead of reporting a refusal", async ({ page }) => { @@ -1027,18 +1029,22 @@ test.describe("STU-380 transport outcome regressions", () => { // 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"); + // An unconfirmed outcome is reconciled before it is reported: the message + // is whatever the re-read of the project showed (here: the class not + // landed yet, with the re-read-before-retry advice) — not a fabricated + // "connection dropped" and not a claim that nothing was written. + expect(outcome.message).toContain("Checked FlutterFlow just now"); + expect(outcome.message).toContain("none of the 1 custom class(es) are in the project yet"); + expect(outcome.message).toContain("re-read first"); 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. + // and silently provisioning the class a second time. One export already + // happened during the deploy: the reconcile re-read of the unconfirmed + // provision, so this is 3 (seed 1 + reconcile 1 + this reset). await page.evaluate(() => window.__CCC_RESOLVE_PROJECT_PUBSPEC__({})); - expect(exportCalls).toBe(2); + expect(exportCalls).toBe(3); }); test("a runner failure after deploy began also drops the cached snapshot", async ({ page }) => { @@ -1084,8 +1090,10 @@ test.describe("STU-380 transport outcome regressions", () => { ); expect(outcome.name).toBe("UnconfirmedDeployError"); + // The next deploy must re-export — one export already happened during the + // deploy, the reconcile re-read of the unconfirmed provision. await page.evaluate(() => window.__CCC_RESOLVE_PROJECT_PUBSPEC__({})); - expect(exportCalls).toBe(2); + expect(exportCalls).toBe(3); }); test("a dropped provisioning stream keeps exports uncached until the file is seen", async ({ page }) => { @@ -1148,24 +1156,25 @@ test.describe("STU-380 transport outcome regressions", () => { expect(outcome.name).toBe("UnconfirmedDeployError"); // A follow-up deploy exports fresh but must not cache: the file may land - // later and leave this snapshot stale forever. + // later and leave this snapshot stale forever. One export already happened + // during the deploy, the reconcile re-read of the unconfirmed provision. await page.evaluate(() => window.__CCC_RESOLVE_PROJECT_PUBSPEC__({})); - expect(exportCalls).toBe(2); + expect(exportCalls).toBe(3); // Even the request settling proves nothing about the remote write — a // dropped stream can precede it — so exports stay uncached. await page.evaluate(() => window.__CCC_PROVISION_STREAM__.close()); await page.waitForTimeout(50); await page.evaluate(() => window.__CCC_RESOLVE_PROJECT_PUBSPEC__({})); - expect(exportCalls).toBe(3); + expect(exportCalls).toBe(4); // The first export that actually shows the file proves the write landed; // the path clears and caching resumes. gaugeLanded = true; await page.evaluate(() => window.__CCC_RESOLVE_PROJECT_PUBSPEC__({})); - expect(exportCalls).toBe(4); + expect(exportCalls).toBe(5); await page.evaluate(() => window.__CCC_RESOLVE_PROJECT_PUBSPEC__({})); - expect(exportCalls).toBe(4); + expect(exportCalls).toBe(5); }); test("a retry's success cannot clear an earlier write that may still land", async ({ page }) => { @@ -1263,20 +1272,22 @@ test.describe("STU-380 transport outcome regressions", () => { expect(outcomeB.settled).toBe("resolved"); // An export showing B's content cannot clear A's write — it can still - // land afterwards and replace it, so nothing may be cached yet. + // land afterwards and replace it, so nothing may be cached yet. One export + // already happened when A timed out: the reconcile re-read of that + // unconfirmed provision. landed = "b"; await page.evaluate(() => window.__CCC_RESOLVE_PROJECT_PUBSPEC__({})); - expect(exportCalls).toBe(2); + expect(exportCalls).toBe(3); await page.evaluate(() => window.__CCC_RESOLVE_PROJECT_PUBSPEC__({})); - expect(exportCalls).toBe(3); // still uncached — A remains outstanding + expect(exportCalls).toBe(4); // still uncached — A remains outstanding // Only an export carrying A's exact content proves that write landed; // then the dirty set empties and caching resumes. landed = "a"; await page.evaluate(() => window.__CCC_RESOLVE_PROJECT_PUBSPEC__({})); - expect(exportCalls).toBe(4); + expect(exportCalls).toBe(5); await page.evaluate(() => window.__CCC_RESOLVE_PROJECT_PUBSPEC__({})); - expect(exportCalls).toBe(4); + expect(exportCalls).toBe(5); }); test("an identical retry's success cannot retire the earlier uncertain write", async ({ page }) => { @@ -1356,17 +1367,19 @@ test.describe("STU-380 transport outcome regressions", () => { expect(outcomeB.settled).toBe("resolved"); // First X-sighting is attributable to the verified retry — A's write can - // still be in flight, so the snapshot must not cache. + // still be in flight, so the snapshot must not cache. One export already + // happened when A timed out: the reconcile re-read of that unconfirmed + // provision. landed = true; await page.evaluate(() => window.__CCC_RESOLVE_PROJECT_PUBSPEC__({})); - expect(exportCalls).toBe(2); + expect(exportCalls).toBe(3); await page.evaluate(() => window.__CCC_RESOLVE_PROJECT_PUBSPEC__({})); // 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(4); + await page.evaluate(() => window.__CCC_RESOLVE_PROJECT_PUBSPEC__({})); + expect(exportCalls).toBe(5); // 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 @@ -1380,9 +1393,9 @@ test.describe("STU-380 transport outcome regressions", () => { }); await page.waitForTimeout(50); await page.evaluate(() => window.__CCC_RESOLVE_PROJECT_PUBSPEC__({})); - expect(exportCalls).toBe(5); + expect(exportCalls).toBe(6); await page.evaluate(() => window.__CCC_RESOLVE_PROJECT_PUBSPEC__({})); - expect(exportCalls).toBe(5); + expect(exportCalls).toBe(6); }); test("a push still in flight keeps exports uncached until it settles", async ({ page }) => { From bb1b65fd6efc143c37ce5b988c4a107cef0457fd Mon Sep 17 00:00:00 2001 From: Stuart Date: Sun, 4 Oct 2026 16:17:53 +1100 Subject: [PATCH 12/20] test(runner): guard the image layout and the healthz version lockstep Two failure classes from this work that nothing covered: The image copies only bin/ and pubspec.yaml by hand, and no CI builds it, so a missing COPY is invisible until a real deploy fails. It already happened once when request validation moved to lib/ and the container could not start at all. The guard reads the entrypoint's package: imports, checks each resolves under lib/, and requires bin/ and lib/ to be COPYed. The runner reports minFlutterflowCli at /healthz while the Dockerfile pins the same version separately. Drift would make /healthz assert a minimum the image does not satisfy. They must match. --- src/cloudRunRunner.test.js | 75 +++++++++++++++++++++++++++++++++++++- 1 file changed, 74 insertions(+), 1 deletion(-) diff --git a/src/cloudRunRunner.test.js b/src/cloudRunRunner.test.js index 4c0f5fb..bd36c37 100644 --- a/src/cloudRunRunner.test.js +++ b/src/cloudRunRunner.test.js @@ -1,5 +1,5 @@ import assert from "node:assert/strict"; -import { readFileSync } from "node:fs"; +import { existsSync, readFileSync } from "node:fs"; import test from "node:test"; const dockerfile = readFileSync( @@ -173,3 +173,76 @@ test("Cloud Run deployment reserves enough memory and serializes provisioning", assert.match(deployScript, /--memory "\$MEMORY"/); assert.match(deployScript, /--concurrency "\$CONCURRENCY"/); }); + +// The image is only exercised at deploy time and nothing builds it in CI, so a +// missing COPY is invisible until a real deploy fails. It already happened once: +// request validation moved to lib/, the Dockerfile still copied only bin/, and +// the container could not start at all. +test("the image copies every directory the entrypoint imports", () => { + const copied = new Set( + [...dockerfile.matchAll(/^COPY\s+(\S+)\s/gm)].map((match) => + match[1].replace(/\/$/, ""), + ), + ); + // `package:ccc_ffai_runner/x.dart` resolves inside the package's lib/, so a + // module imported this way only exists in the container if lib/ is copied in. + const imported = [ + ...runnerSource.matchAll(/package:ccc_ffai_runner\/([A-Za-z0-9_/]+\.dart)/g), + ].map((match) => match[1]); + + assert.ok( + imported.length > 0, + "server.dart must import from package:ccc_ffai_runner/ (otherwise this guard is vacuous)", + ); + for (const modulePath of imported) { + assert.ok( + existsSync( + new URL(`../cloud-run/ffai-runner/lib/${modulePath}`, import.meta.url), + ), + `package:ccc_ffai_runner/${modulePath} must resolve to a file under lib/`, + ); + } + for (const directory of ["bin", "lib"]) { + assert.ok( + copied.has(directory), + `the Dockerfile must COPY ${directory}/ — the entrypoint imports it, and without it the image cannot start`, + ); + } +}); + +// The runner reports minFlutterflowCli at /healthz so a version drift is visible. +// If the two ever disagree, /healthz reports a minimum the image does not +// actually satisfy, which is worse than not reporting one at all. +test("/healthz reports the same required CLI version the image pins", () => { + const pinned = dockerfile.match( + /^ARG FLUTTERFLOW_CLI_VERSION=(\d+\.\d+\.\d+)$/m, + )?.[1]; + const reported = runnerSource.match(/const minCliVersion = '([^']+)'/)?.[1]; + + assert.ok(pinned, "Dockerfile must pin FLUTTERFLOW_CLI_VERSION"); + assert.ok( + reported, + "server.dart must declare the minimum CLI version it reports", + ); + assert.equal( + reported, + pinned, + "minCliVersion must match the pinned FLUTTERFLOW_CLI_VERSION", + ); +}); + +// A merged runner fix that never shipped is the failure this guards: /healthz +// has to be able to answer which revision is live, so the deploy must actually +// pass the SHA through. +test("the deploy passes the built revision to the runner for /healthz", () => { + assert.match( + deployScript, + /RUNNER_GIT_SHA="\$\{RUNNER_GIT_SHA:-\$\(git rev-parse/, + "the deploy script must derive RUNNER_GIT_SHA from its checkout", + ); + assert.match( + deployScript, + /--set-env-vars "[^"]*RUNNER_GIT_SHA=\$RUNNER_GIT_SHA/, + "RUNNER_GIT_SHA must reach the service, or /healthz reports an empty SHA", + ); +}); From 37fe7ac6339c6a2eebfff07f05d1968648e5d3f3 Mon Sep 17 00:00:00 2001 From: Stuart Date: Sun, 4 Oct 2026 16:51:41 +1100 Subject: [PATCH 13/20] ci: run the runner's Dart analyze and tests on every PR The runner's request validation is the code that rejected flutter_web_plugins and failed every deploy for months. Its tests existed but only ran locally, so nothing stopped the regression returning. The image is built only at deploy time, so this is also the sole place the runner is compiled before shipping. --- .github/workflows/ci.yml | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1061fc7..17bbbe4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -54,3 +54,30 @@ jobs: - name: Scan production bundle for API key/endpoint leaks run: python3 scripts/scan_bundle_for_leaks.py + + # The runner's request validation is the code that rejected + # flutter_web_plugins and failed every deploy for months, and its tests only + # ran on a developer's machine - so nothing stopped that regression returning. + # The image is built only at deploy time, so this is also the only place the + # runner is compiled before it ships. + runner: + runs-on: ubuntu-latest + + steps: + - name: Checkout + uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 + + - name: Set up Dart + uses: dart-lang/setup-dart@6afc89df92d6eb3834022f73cd65adc8cdfcb92d # v1 + + - name: Install runner dependencies + working-directory: cloud-run/ffai-runner + run: dart pub get + + - name: Analyze the runner + working-directory: cloud-run/ffai-runner + run: dart analyze + + - name: Test the runner + working-directory: cloud-run/ffai-runner + run: dart test From dab59e98214f23b83c9e9eb5e391deb37708902a Mon Sep 17 00:00:00 2001 From: Stuart Date: Sun, 4 Oct 2026 16:55:26 +1100 Subject: [PATCH 14/20] chore(runner): ignore dart pub get output --- cloud-run/ffai-runner/.gitignore | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 cloud-run/ffai-runner/.gitignore diff --git a/cloud-run/ffai-runner/.gitignore b/cloud-run/ffai-runner/.gitignore new file mode 100644 index 0000000..f762031 --- /dev/null +++ b/cloud-run/ffai-runner/.gitignore @@ -0,0 +1,4 @@ +# `dart pub get` output. The image is built from the Dockerfile, which runs +# pub get itself, so neither of these belongs in the repository. +.dart_tool/ +pubspec.lock From 4dd30a15f10ed4c7966e81ec1ed7b1d6c42092a9 Mon Sep 17 00:00:00 2001 From: Stuart Date: Sun, 4 Oct 2026 17:22:13 +1100 Subject: [PATCH 15/20] fix(deploy): keep the cause when reconciling, and stop crashing without a partition Two defects found by running the browser suite against the integrated branch, which neither PR had done. 1. isDeployFinishable crashed the whole deploy. The reconcile re-read can fail or time out, and it then returns the caller's original error, which carries no partition - so `reconciled.reconcilePartition.notLanded` threw a TypeError out of provisionMissingCodeFiles instead of reporting an unconfirmed deploy. An absent partition is an ordinary outcome, and "not finishable" is also the safe answer, since without a proven partition we cannot claim every class landed. 2. Reconciliation discarded the cause. It built a fresh error carrying only what the re-read found, so "HTTP 502" or "the connection dropped" vanished - the one part of the failure a later project export cannot reconstruct. The finding is now added to the cause instead of replacing it; only the generic wait/timeout wording is dropped, because the finding is what replaces it. Verified: 406 node tests, and the full browser suite - 265 passed. --- app.js | 22 ++++++++++++++++++--- e2e/stu380-deploy.spec.js | 10 +++++++--- src/flutterFlowCodeFileProvisioning.js | 11 +++++++++-- src/flutterFlowCodeFileProvisioning.test.js | 9 +++++++++ 4 files changed, 44 insertions(+), 8 deletions(-) diff --git a/app.js b/app.js index be2abeb..ce34a90 100644 --- a/app.js +++ b/app.js @@ -3234,7 +3234,14 @@ async function reconcileUnconfirmedProvision(apiClient, attempted, original) { `not there yet: ${names(notLanded)}. Wait a minute, then deploy again - ` + "classes already there are skipped, and the rest are written."; } - const error = new UnconfirmedDeployError(message); + // The finding is added to the cause, never in place of it. Only the generic + // wait/timeout wording is dropped, because the finding is precisely what + // replaces it - but an HTTP status or a dropped connection is diagnosis the + // re-read cannot reconstruct, so discarding it would leave the user knowing + // less than the code does about their own failed deploy. + const error = new UnconfirmedDeployError( + original?.causeDetail ? `${original.causeDetail} ${message}` : message, + ); // The reconcile outcome decides whether the terminal can offer "Finish // deploy": when every class landed, only the pubspec push remains. error.reconcilePartition = { landed, notLanded }; @@ -3370,10 +3377,15 @@ async function provisionMissingCodeFiles( 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( + const error = new UnconfirmedDeployError( `${cause} The deploy may still be finishing on the server; open your FlutterFlow project to reconcile ` + "before retrying.", ); + // Kept so reconciliation can add what its re-read found WITHOUT dropping + // this: an HTTP status or a dropped connection is the one part of the + // cause a later project export cannot reconstruct. + error.causeDetail = cause; + throw error; } if (!result.success) { @@ -3384,11 +3396,15 @@ async function provisionMissingCodeFiles( // already uploaded, so it must be reported unconfirmed (and the catch // below drops the stale snapshot). if (deployOutcomeOfStreamResult(result) === DeployOutcome.UNCONFIRMED) { - throw new UnconfirmedDeployError( + const error = new UnconfirmedDeployError( `${message} The failure arrived after the deploy began — custom classes ` + "may already be written. Open your FlutterFlow project to reconcile " + "before retrying.", ); + // The runner's own error is the cause; a re-read of the project cannot + // recover it, so reconciliation must not replace it. + error.causeDetail = message; + throw error; } // A verified pre-write rejection (compile gate, workspace, request // validation) is still the runner's answer — FlutterFlow was never diff --git a/e2e/stu380-deploy.spec.js b/e2e/stu380-deploy.spec.js index f80eafb..19a8800 100644 --- a/e2e/stu380-deploy.spec.js +++ b/e2e/stu380-deploy.spec.js @@ -1030,9 +1030,13 @@ test.describe("STU-380 transport outcome regressions", () => { expect(outcome.name).toBe("UnconfirmedDeployError"); expect(outcome.outcome).toBe("unconfirmed"); // An unconfirmed outcome is reconciled before it is reported: the message - // is whatever the re-read of the project showed (here: the class not - // landed yet, with the re-read-before-retry advice) — not a fabricated - // "connection dropped" and not a claim that nothing was written. + // carries what the re-read of the project found (the class not landed yet, + // with the re-read-before-retry advice). The cause is kept alongside it — + // an HTTP 502 was actually received, so the message must name the status + // and may not claim the connection dropped. The re-read cannot reconstruct + // the cause, so reporting it in place of the cause would tell the user less + // than the code knows. + expect(outcome.message).toContain("HTTP 502"); expect(outcome.message).toContain("Checked FlutterFlow just now"); expect(outcome.message).toContain("none of the 1 custom class(es) are in the project yet"); expect(outcome.message).toContain("re-read first"); diff --git a/src/flutterFlowCodeFileProvisioning.js b/src/flutterFlowCodeFileProvisioning.js index 99a93b4..b99b081 100644 --- a/src/flutterFlowCodeFileProvisioning.js +++ b/src/flutterFlowCodeFileProvisioning.js @@ -78,11 +78,18 @@ export function excludeProvisionedCodeFiles(fileMap, entries) { * project's fresh export. When true the classes have landed and only step 3 — * the pubspec push — remains, so the interrupted deploy is finishable without * re-running the slower class write. - * @param {{landed: Array, notLanded: Array}} partition - Outcome of - * `partitionProvisionedCodeFiles` + * + * A missing partition is `false`, not a crash. The reconcile re-read can fail + * or time out, and it then returns the caller's original error, which carries + * no partition — so an absent partition is an ordinary outcome here, not an + * impossible one. Treating it as un-finishable is also the safe answer: without + * a proven partition we cannot claim every class landed. + * @param {{landed: Array, notLanded: Array}|undefined|null} partition - Outcome + * of `partitionProvisionedCodeFiles`, when one could be taken * @returns {boolean} */ export function isDeployFinishable(partition) { + if (!partition) return false; return partition.notLanded.length === 0 && partition.landed.length > 0; } diff --git a/src/flutterFlowCodeFileProvisioning.test.js b/src/flutterFlowCodeFileProvisioning.test.js index 29fe809..dc9e5ab 100644 --- a/src/flutterFlowCodeFileProvisioning.test.js +++ b/src/flutterFlowCodeFileProvisioning.test.js @@ -163,6 +163,15 @@ test("isDeployFinishable is true only when every attempted class landed", () => assert.equal(isDeployFinishable(nothingLanded), false); }); +test("isDeployFinishable is false for a missing partition, not a crash", () => { + // The reconcile re-read can fail or time out, and then returns the caller's + // original error — which carries no partition. That is an ordinary outcome, + // and an unproven partition must not offer a finish action. Treating it as + // impossible threw a TypeError out of the whole deploy. + assert.equal(isDeployFinishable(undefined), false); + assert.equal(isDeployFinishable(null), false); +}); + // DONE CRITERION (client): a deploy whose step-2 response is lost must still // be finishable — the classes are present and the finish action pushes the // pubspec without re-provisioning them. These tests drive the exact pure From 30de965ae52259b57835e8b763625bba4f18e18b Mon Sep 17 00:00:00 2001 From: Stuart Date: Sun, 4 Oct 2026 17:43:32 +1100 Subject: [PATCH 16/20] feat(runner): record deploy runs and serve their status so a lost outcome 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. --- app.js | 60 +++ cloud-run/ffai-runner/bin/server.dart | 190 +++++++- cloud-run/ffai-runner/lib/run_record.dart | 419 ++++++++++++++++++ cloud-run/ffai-runner/lib/sha256.dart | 111 +++++ .../ffai-runner/test/run_record_test.dart | 170 +++++++ cloud-run/ffai-runner/test/sha256_test.dart | 34 ++ src/runStatus.js | 227 ++++++++++ src/runStatus.test.js | 186 ++++++++ 8 files changed, 1377 insertions(+), 20 deletions(-) create mode 100644 cloud-run/ffai-runner/lib/run_record.dart create mode 100644 cloud-run/ffai-runner/lib/sha256.dart create mode 100644 cloud-run/ffai-runner/test/run_record_test.dart create mode 100644 cloud-run/ffai-runner/test/sha256_test.dart create mode 100644 src/runStatus.js create mode 100644 src/runStatus.test.js diff --git a/app.js b/app.js index be2abeb..d0ecabd 100644 --- a/app.js +++ b/app.js @@ -38,6 +38,10 @@ import { partitionProvisionedCodeFiles, } from "./src/flutterFlowCodeFileProvisioning.js"; import { buildReviewPresentation } from "./src/reviewPresentation.js"; +import { + generateDeployRunId, + queryRunStatusWithinBudget, +} from "./src/runStatus.js"; import { expectedWidgetClassFromFileName, findUnbalancedBracketError, @@ -3254,6 +3258,15 @@ async function provisionMissingCodeFiles( return { remoteFiles, syncFileMap: fileMap, unverified: [], approximate: [] }; } + // The client generates the run id so that, if this response is ever lost, + // a 404 on GET /runStatus/ is proof the request never reached the + // runner (nothing written, retry safe) rather than an ambiguous "unknown". + const runId = generateDeployRunId(); + + // True once the run-status lookup has decided this attempt's real outcome, + // so the late-arriving provision stream cannot double-credit its writes. + let runStatusResolved = false; + // The runner compiles these against the project's own package versions // before it writes anything, so an API the generated code invented - a named // argument the package never declared - stops the deploy instead of landing @@ -3310,6 +3323,7 @@ async function provisionMissingCodeFiles( projectId: apiClient.projectId, baseUrl: apiClient.baseUrl, commitMessage, + runId, customClasses: missingCodeFiles, verification, stream: true, @@ -3413,6 +3427,9 @@ async function provisionMissingCodeFiles( // retires them outright. provisionWork?.then( (late) => { + // When the run-status lookup already decided this attempt's real + // outcome, the late stream must not double-credit its writes. + if (runStatusResolved) return; if (late?.finalResultReceived && late.success) { adjustProjectSourceDirty(apiClient, missingCodeFiles, { uncertain: -1, @@ -3429,6 +3446,49 @@ async function provisionMissingCodeFiles( }, () => {}, ); + // The record is the authority here: ask the runner what actually + // happened instead of re-reading the project and guessing. Only when the + // answer is definitive does it replace the estimate; a 404, a 5xx or an + // unrecognised body (an older runner) falls through to the reconcile. + let statusOutcome = { definitive: null }; + try { + statusOutcome = await queryRunStatusWithinBudget({ + provisionEndpoint: FLUTTERFLOW_CLASS_PROVISION_ENDPOINT, + runId, + apiKey: apiClient.apiKey, + }); + } catch { + // The status query must fail closed: any unexpected throw from it (a + // browser abort surfacing unusually, a half-updated bundle) falls back + // to the existing reconcile instead of fabricating a new failure. + statusOutcome = { definitive: null }; + } + if (statusOutcome.definitive === "success") { + // The runner confirms every class it was asked to write landed, so the + // provision succeeds and step 3 (the pubspec push) runs as usual — this + // is a known success, not an estimate. + runStatusResolved = true; + adjustProjectSourceDirty(apiClient, missingCodeFiles, { + uncertain: -1, + verified: 1, + }); + return { + remoteFiles, + syncFileMap: excludeProvisionedCodeFiles(fileMap, missingCodeFiles), + unverified, + approximate, + provisionSucceeded: true, + }; + } + 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.", + ); + } + const reconciled = await reconcileUnconfirmedProvision( apiClient, missingCodeFiles, diff --git a/cloud-run/ffai-runner/bin/server.dart b/cloud-run/ffai-runner/bin/server.dart index be4b67a..b774885 100644 --- a/cloud-run/ffai-runner/bin/server.dart +++ b/cloud-run/ffai-runner/bin/server.dart @@ -3,6 +3,8 @@ import 'dart:convert'; import 'dart:io'; import 'package:ccc_ffai_runner/request_validation.dart'; +import 'package:ccc_ffai_runner/run_record.dart'; +import 'package:ccc_ffai_runner/sha256.dart'; // The lowest flutterflow_cli the vendored FlutterFlow AI snapshot accepts. The // image pins FLUTTERFLOW_CLI_VERSION to this (see Dockerfile); it must be kept @@ -35,6 +37,19 @@ Future _handle(HttpRequest request) async { return; } + // GET /runStatus/ answers what happened to a deploy whose response + // was lost. The id in the path is the one the client itself generated, so + // a 404 is a proof the request never reached the runner (nothing was + // written, a retry is safe), never an ambiguous "unknown". The caller must + // present the same API key the run started with; see fetchRunStatus. + 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)!); + return; + } + if (request.method != 'POST' || request.uri.path != '/deployCustomClasses') { await channel.result(HttpStatus.notFound, { @@ -54,6 +69,7 @@ Future _handle(HttpRequest request) async { String projectId; String baseUrl; String commitMessage; + String runId; bool dryRun; List classes; VerificationRequest? verification; @@ -73,6 +89,7 @@ Future _handle(HttpRequest request) async { maxLength: 300, required: false, ); + runId = stringField(payload, 'runId', maxLength: 120, required: false); dryRun = payload['dryRun'] == true; classes = _normalizeClasses(payload['customClasses']); verification = normalizeVerification(payload['verification']); @@ -86,19 +103,64 @@ Future _handle(HttpRequest request) async { return; } + // The client generates the run id so a 404 on /runStatus proves the + // request never arrived. It becomes a document id in Firestore, so it is + // constrained to characters that are safe there (letters, digits, - and _). + if (runId.isNotEmpty && !RegExp(r'^[A-Za-z0-9_-]+$').hasMatch(runId)) { + await channel.result(HttpStatus.badRequest, { + 'success': false, + 'error': 'Invalid runId.', + 'preWrite': true, + }); + return; + } + + // A run record lets a client whose connection dropped or whose wait expired + // ask what actually happened instead of re-reading the project and + // guessing. Older clients send no runId; for them nothing is recorded and + // the deploy behaves exactly as before. + late RunRecorder recorder; + final recordRun = runId.isNotEmpty; + if (recordRun) { + recorder = RunRecorder( + FirestoreRunStore( + project: + Platform.environment['FIRESTORE_PROJECT'] ?? + defaultFirestoreProject, + ), + runId: runId, + projectId: projectId, + keyHash: sha256Hex(utf8.encode(apiKey)), + classNames: classes.map((entry) => entry.className).toList(), + ); + } + // Only stream for callers that asked for it, so older clients keep getting // the single JSON response they parse. if (payload['stream'] == true) { channel.beginStream(); } - channel.phase('connected', 'Connected to the FlutterFlow deploy runner.'); + _phase(channel, recordRun ? recorder : null, 'connected', + 'Connected to the FlutterFlow deploy runner.'); final workspace = Directory( Platform.environment['FFAI_WORKSPACE'] ?? '/workspace/custom_code_connect', ); - final initResult = await _ensureWorkspace(workspace, apiKey, channel); + final initResult = await _ensureWorkspace( + workspace, + apiKey, + channel, + recordRun ? recorder : null, + ); if (initResult != null) { + if (recordRun) { + await recorder.recordFailed( + initResult.timedOut + ? 'Preparing the FlutterFlow AI workspace timed out.' + : 'FlutterFlow AI workspace initialization failed.', + ); + } await channel.result(HttpStatus.badGateway, { 'success': false, 'error': @@ -118,8 +180,18 @@ Future _handle(HttpRequest request) async { // accepts a call to a named argument the package never declared - exactly // the class of error that otherwise lands in the project and breaks every // custom widget or action importing it. - final analysis = await _verifyCustomCode(workspace, verification, channel); + final analysis = await _verifyCustomCode( + workspace, + verification, + channel, + recordRun ? recorder : null, + ); if (analysis != null && analysis.hasErrors) { + if (recordRun) { + await recorder.recordFailed( + 'The generated custom code does not compile.', + ); + } await channel.result(HttpStatus.unprocessableEntity, { 'success': false, 'error': @@ -157,7 +229,9 @@ Future _handle(HttpRequest request) async { args.addAll(['--commit-message', commitMessage]); } - channel.phase( + _phase( + channel, + recordRun ? recorder : null, 'deploy_start', classes.length == 1 ? 'Deploying ${classes.single.className} to FlutterFlow...' @@ -169,10 +243,14 @@ Future _handle(HttpRequest request) async { workingDirectory: workspace.path, apiKey: apiKey, channel: channel, + recorder: recordRun ? recorder : null, timeout: _deployTimeout(classes.length), ); if (result.timedOut) { + if (recordRun) { + await recorder.recordFailed('FlutterFlow AI DSL deploy timed out.'); + } await channel.result(HttpStatus.gatewayTimeout, { 'success': false, 'error': 'FlutterFlow AI DSL deploy timed out.', @@ -184,6 +262,9 @@ Future _handle(HttpRequest request) async { } if (result.exitCode != 0) { + if (recordRun) { + await recorder.recordFailed('FlutterFlow AI DSL deploy failed.'); + } await channel.result(HttpStatus.badGateway, { 'success': false, 'error': 'FlutterFlow AI DSL deploy failed.', @@ -194,18 +275,25 @@ Future _handle(HttpRequest request) async { return; } + final deployed = classes + .map( + (entry) => { + 'artifactId': entry.artifactId, + 'className': entry.className, + }, + ) + .toList(); + + // The terminal write is awaited (bounded and error-swallowed in the + // recorder) so a client whose response was lost can learn the run finished. + if (recordRun) { + await recorder.recordDone(deployed, dryRun); + } + await channel.result(HttpStatus.ok, { 'success': true, 'message': 'Custom classes upserted through FlutterFlow AI DSL.', - 'deployed': - classes - .map( - (entry) => { - 'artifactId': entry.artifactId, - 'className': entry.className, - }, - ) - .toList(), + 'deployed': deployed, 'dryRun': dryRun, 'verified': analysis?.verifiedFiles ?? const [], 'verificationSkipped': analysis?.skippedReason, @@ -213,6 +301,9 @@ Future _handle(HttpRequest request) async { } catch (error, stackTrace) { stderr.writeln(error); stderr.writeln(stackTrace); + // 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). await channel.result(HttpStatus.internalServerError, { 'success': false, 'error': '$error', @@ -224,20 +315,35 @@ Future _handle(HttpRequest request) async { } } +/// Reports a phase to the streaming channel and, when a run is being recorded, +/// persists it to the run record so a disconnected client can later ask what +/// happened. +void _phase( + _ResponseChannel channel, + RunRecorder? recorder, + String id, + String message, +) { + channel.phase(id, message); + recorder?.recordPhase(id, message); +} + Future<_RunOutcome?> _ensureWorkspace( Directory workspace, String apiKey, _ResponseChannel channel, + RunRecorder? recorder, ) async { final packageConfig = File( '${workspace.path}/.dart_tool/package_config.json', ); if (packageConfig.existsSync()) { - channel.phase('workspace_ready', 'Build environment is ready.'); + _phase(channel, recorder, 'workspace_ready', 'Build environment is ready.'); return null; } - channel.phase('workspace_init', 'Preparing the FlutterFlow AI workspace...'); + _phase(channel, recorder, 'workspace_init', + 'Preparing the FlutterFlow AI workspace...'); final parent = workspace.parent; await parent.create(recursive: true); final workspaceName = workspace.path.split(Platform.pathSeparator).last; @@ -252,13 +358,14 @@ Future<_RunOutcome?> _ensureWorkspace( workingDirectory: parent.path, apiKey: apiKey, channel: channel, + recorder: recorder, ); if (result.timedOut || result.exitCode != 0) { return result; } - channel.phase('workspace_ready', 'Build environment is ready.'); + _phase(channel, recorder, 'workspace_ready', 'Build environment is ready.'); return null; } @@ -283,6 +390,7 @@ Future<_RunOutcome> _runFlutterFlow( required String workingDirectory, required String apiKey, required _ResponseChannel channel, + RunRecorder? recorder, Duration timeout = const Duration(minutes: 5), }) async { final process = await Process.start( @@ -315,7 +423,7 @@ Future<_RunOutcome> _runFlutterFlow( // Markers are progress reporting, not output worth showing in an error. final marker = _readPhaseMarker(line); if (marker != null) { - channel.phase(marker.phase, marker.message); + _phase(channel, recorder, marker.phase, marker.message); return; } @@ -459,8 +567,10 @@ void _writeCorsHeaders(HttpResponse response) { final allowedOrigin = Platform.environment['ALLOWED_ORIGIN'] ?? '*'; response.headers ..set('Access-Control-Allow-Origin', allowedOrigin) - ..set('Access-Control-Allow-Methods', 'POST, OPTIONS') - ..set('Access-Control-Allow-Headers', 'Content-Type') + ..set('Access-Control-Allow-Methods', 'POST, GET, OPTIONS') + // x-ff-api-key lets the browser present the deploy key on GET /runStatus so + // a run id is never a bearer capability on its own. + ..set('Access-Control-Allow-Headers', 'Content-Type, x-ff-api-key') ..set('Vary', 'Origin'); } @@ -505,6 +615,45 @@ Future _writeJsonBody(HttpResponse response, Object body) async { } catch (_) {} } +/// Serves `GET /runStatus/`: looks up the run record and enforces that +/// the caller presents the same API key the run started with. +Future _handleRunStatus(HttpRequest request, String runId) async { + final store = FirestoreRunStore( + project: + Platform.environment['FIRESTORE_PROJECT'] ?? defaultFirestoreProject, + ); + final lookup = await fetchRunStatus( + runId: runId, + presentedKey: _apiKeyFromRequest(request), + store: store, + ); + await _writeJsonResponse(request.response, lookup.statusCode, lookup.body); +} + +/// The API key the caller must present to read a run. Read from a header +/// (preferred, to keep it out of URLs and access logs) with a query-parameter +/// fallback for callers that cannot set a custom header. +String? _apiKeyFromRequest(HttpRequest request) { + final header = request.headers.value('x-ff-api-key')?.trim(); + if (header != null && header.isNotEmpty) return header; + final query = request.uri.queryParameters['key']?.trim(); + if (query != null && query.isNotEmpty) return query; + return null; +} + +Future _writeJsonResponse( + HttpResponse response, + int statusCode, + Object body, +) async { + response.statusCode = statusCode; + response.headers.contentType = ContentType.json; + response.write(jsonEncode(body)); + try { + await response.close(); + } catch (_) {} +} + Future> _readJson(HttpRequest request) async { final raw = await utf8.decoder.bind(request).join(); final decoded = jsonDecode(raw); @@ -566,6 +715,7 @@ Future<_AnalysisOutcome?> _verifyCustomCode( Directory workspace, VerificationRequest? verification, _ResponseChannel channel, + RunRecorder? recorder, ) async { if (verification == null) return null; if (verification.unavailableReason != null) { @@ -573,7 +723,7 @@ Future<_AnalysisOutcome?> _verifyCustomCode( } if (verification.sources.isEmpty) return null; - channel.phase('verifying', 'Compiling your custom code...'); + _phase(channel, recorder, 'verifying', 'Compiling your custom code...'); final packageDir = Directory('${workspace.parent.path}/custom_code_analysis'); final libDir = Directory('${packageDir.path}/lib'); diff --git a/cloud-run/ffai-runner/lib/run_record.dart b/cloud-run/ffai-runner/lib/run_record.dart new file mode 100644 index 0000000..2e34704 --- /dev/null +++ b/cloud-run/ffai-runner/lib/run_record.dart @@ -0,0 +1,419 @@ +/// The deploy-run record: a per-request record the runner writes as a deploy +/// progresses, and the status endpoint that reads it back. +/// +/// Why it exists: a deploy can outlive the browser's wait bound, and until now +/// the only way the browser could learn what happened was to re-read the whole +/// project and guess. A record that tracks the run's phases, terminal outcome, +/// per-class results and error string lets a client whose response was lost +/// ask the runner directly. +/// +/// The record lives in Firestore in a fixed project (default `low-code-connect`) +/// because that project exists, its Firestore is enabled, and the runner's +/// service account already holds `roles/editor` there - so no IAM change and no +/// runtime pub dependency is needed. It is reached through the Firestore REST +/// API with an OAuth token borrowed from the GCE metadata server, all over +/// `dart:io`. +/// +/// Two invariants are enforced from here because they are the point of the +/// feature: +/// - The API key is never stored - only its SHA-256 digest is. The status +/// endpoint requires the caller to present the same key and compares +/// digests, so a run id alone is never a bearer capability over +/// `--allow-unauthenticated`. +/// - Writing the record can never fail a deploy. Every Firestore call is +/// bounded and its failure is logged, not propagated. The status endpoint, +/// by contrast, must answer its caller, so a read failure surfaces as a +/// temporary-unavailable error the browser treats as "unknown" and falls +/// back to reconciling by re-reading the project. +library; + +import 'dart:async'; +import 'dart:convert'; +import 'dart:io'; + +import 'package:ccc_ffai_runner/sha256.dart'; + +/// The GCP project that owns the Firestore database holding run records. The +/// runner's service account holds roles/editor here, so no separate grant is +/// needed; overridden by FIRESTORE_PROJECT if the deployment ever relocates. +const defaultFirestoreProject = 'low-code-connect'; + +/// 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'; + +/// Cap on a single record write. Writing must never hold up or fail a deploy, +/// so even the terminal write - the one a waiting client depends on - is +/// bounded to this and its failure swallowed. +const runRecordWriteTimeout = Duration(seconds: 2); + +/// An HTTP response from the backing store, kept opaque so the real transport +/// (dart:io) and a test double share one shape. +class FirestoreHttpResponse { + const FirestoreHttpResponse(this.status, this.body); + + final int status; + final String body; +} + +/// The transport the store talks to Firestore through. Injectable so tests can +/// exercise the store without a network or a metadata server. +typedef FirestoreTransport = + Future Function( + String method, + Uri uri, { + Map? headers, + String? body, + }); + +/// A Firestore lookup/update that did not succeed. Message is for the log +/// stream only; it is never shown to the caller of a deploy. +class RunStoreException implements Exception { + RunStoreException(this.message); + + final String message; + + @override + String toString() => 'RunStoreException: $message'; +} + +/// A dependency-free Firestore client scoped to one document path (the run +/// records), using the REST API and an OAuth token from the GCE metadata +/// server over dart:io. Both the token source and the HTTP transport are +/// injectable so the store is testable in isolation. +class FirestoreRunStore { + FirestoreRunStore({ + String project = defaultFirestoreProject, + String collection = runStatusCollection, + FirestoreTransport? transport, + Future Function()? fetchAccessToken, + }) : _project = project, + _collection = collection, + _transport = transport, + _fetchAccessToken = fetchAccessToken; + + final String _project; + final String _collection; + final FirestoreTransport? _transport; + final Future Function()? _fetchAccessToken; + + String? _cachedToken; + DateTime? _tokenExpiry; + + // Every field the runner writes, listed so the create-or-update PATCH + // targets exactly this set and never clobbers fields another writer owns. + static const _fieldKeys = [ + 'runId', + 'projectId', + 'keyHash', + 'classNames', + 'status', + 'phase', + 'message', + 'error', + 'deployed', + 'dryRun', + 'updatedAt', + ]; + + Uri _documentUri(String docId) => Uri.parse( + 'https://firestore.googleapis.com/v1/projects/$_project' + '/databases/(default)/documents/$_collection/$docId', + ); + + Uri _writeUri(String docId) => Uri.parse( + '${_documentUri(docId)}?' + '${_fieldKeys.map((k) => 'updateMask.fieldPaths=$k').join('&')}', + ); + + /// Writes a document, creating it if absent and otherwise updating only the + /// fields the runner owns. Throws `RunStoreException` on any non-2xx. + Future put(String docId, Map fields) async { + final token = await _accessToken(); + final response = await _send( + 'PATCH', + _writeUri(docId), + headers: {'Authorization': 'Bearer $token'}, + body: jsonEncode({'fields': encodeFields(fields)}), + ); + if (response.status < 200 || response.status >= 300) { + throw RunStoreException('write returned HTTP ${response.status}'); + } + } + + /// Reads a document, or returns null when it does not exist. + Future?> get(String docId) async { + final token = await _accessToken(); + final response = await _send( + 'GET', + _documentUri(docId), + headers: {'Authorization': 'Bearer $token'}, + ); + if (response.status == 404) return null; + if (response.status < 200 || response.status >= 300) { + throw RunStoreException('read returned HTTP ${response.status}'); + } + final decoded = jsonDecode(response.body); + if (decoded is! Map) { + throw RunStoreException('unexpected Firestore response shape'); + } + return decodeFields(decoded['fields'] as Map? ?? const {}); + } + + Future _accessToken() async { + final now = DateTime.now(); + if (_cachedToken != null && + _tokenExpiry != null && + now.isBefore(_tokenExpiry!)) { + return _cachedToken!; + } + // The real token source is the GCE metadata server; tests inject their own + // so no metadata round trip happens in the test suite. + final token = await (_fetchAccessToken ?? _metadataToken)(); + // Refresh comfortably ahead of the hour-long token lifetime. + _cachedToken = token; + _tokenExpiry = now.add(const Duration(minutes: 50)); + return token; + } + + static Future _metadataToken() async { + final client = HttpClient(); + try { + final request = await client.getUrl( + Uri.parse( + 'http://metadata.google.internal/computeMetadata/v1/instance/' + 'service-accounts/default/token', + ), + ); + request.headers.set('Metadata-Flavor', 'Google'); + final response = await request.close(); + final body = await utf8.decoder.bind(response).join(); + final decoded = jsonDecode(body); + final token = decoded is Map ? decoded['access_token'] : null; + if (token is! String || token.isEmpty) { + throw RunStoreException('metadata server returned no access token'); + } + return token; + } finally { + client.close(); + } + } + + Future _send( + String method, + Uri uri, { + Map? headers, + String? body, + }) { + if (_transport != null) { + return _transport(method, uri, headers: headers, body: body); + } + return _dartIoSend(method, uri, headers: headers, body: body); + } + + static Future _dartIoSend( + String method, + Uri uri, { + Map? headers, + String? body, + }) async { + final client = HttpClient(); + try { + final request = await client.openUrl(method, uri); + headers?.forEach((key, value) => request.headers.set(key, value)); + if (body != null) request.write(body); + final response = await request.close(); + final text = await utf8.decoder.bind(response).join(); + return FirestoreHttpResponse(response.statusCode, text); + } finally { + client.close(); + } + } +} + +/// Encodes a flat/typed object map into Firestore REST `fields` values. +Map encodeFields(Map fields) { + return { + for (final entry in fields.entries) entry.key: _toFirestoreValue(entry.value), + }; +} + +/// Decodes a Firestore REST `fields` map back into plain Dart values. +Map decodeFields(Map fields) { + final out = {}; + fields.forEach((key, value) => out['$key'] = _fromFirestoreValue(value)); + return out; +} + +Object? _toFirestoreValue(Object? value) { + if (value == null) return {'nullValue': null}; + if (value is bool) return {'booleanValue': value}; + if (value is String) return {'stringValue': value}; + if (value is int) return {'integerValue': '$value'}; + if (value is List) { + return { + 'arrayValue': { + 'values': [for (final item in value) _toFirestoreValue(item)], + }, + }; + } + if (value is Map) { + return {'mapValue': {'fields': encodeFields(Map.from(value))}}; + } + return {'nullValue': null}; +} + +Object? _fromFirestoreValue(Object? value) { + if (value is! Map) return null; + if (value.containsKey('stringValue')) return value['stringValue']; + if (value.containsKey('booleanValue')) return value['booleanValue']; + if (value.containsKey('integerValue')) { + return int.tryParse('${value['integerValue']}'); + } + if (value.containsKey('doubleValue')) return value['doubleValue']; + final array = value['arrayValue']; + if (array is Map) { + final values = array['values'] as List? ?? const []; + return [for (final item in values) _fromFirestoreValue(item)]; + } + final map = value['mapValue']; + if (map is Map) { + return decodeFields(map['fields'] as Map? ?? const {}); + } + return null; +} + +/// Records a deploy run's progress to Firestore. Idempotent by run id: every +/// write targets the same document, so a phase update overwrites the previous +/// one rather than appending. +class RunRecorder { + RunRecorder( + this._store, { + required this.runId, + required this.projectId, + required this.keyHash, + required List classNames, + }) : _classNames = List.unmodifiable(classNames); + + final FirestoreRunStore _store; + final String runId; + final String projectId; + final String keyHash; + final List _classNames; + String _phase = 'connected'; + + /// Records a non-terminal progress update. Fire-and-forget: never awaited, + /// never able to stall or fail the deploy. + void recordPhase(String phase, String message) { + _phase = phase; + _write({'status': 'running', 'phase': phase, 'message': message}); + } + + /// Records a successful terminal state. Awaited (bounded and swallowed + /// internally) so a client that lost the response can reliably learn the run + /// finished before its request returns. + Future recordDone(List> deployed, bool dryRun) { + _phase = 'done'; + return _write( + {'status': 'done', 'deployed': deployed, 'dryRun': dryRun}, + ); + } + + /// Records a terminal failure. The phase at failure is kept so the client can + /// tell a refusal that preceded any write from a failure that may have + /// followed classes already uploaded. + Future recordFailed(String error) { + return _write({'status': 'failed', 'error': error}); + } + + Future _write(Map patch) async { + final fields = { + 'runId': runId, + 'projectId': projectId, + 'keyHash': keyHash, + 'classNames': _classNames, + 'phase': _phase, + 'updatedAt': DateTime.now().toUtc().toIso8601String(), + ...patch, + }; + try { + await _store.put(runId, fields).timeout(runRecordWriteTimeout); + } catch (error) { + // The record is advisory: Firestore being slow, unreachable or + // misconfigured must degrade to the pre-record behaviour, not turn a + // good deploy into an error. + stderr.writeln('[run record] failed to persist $runId: $error'); + } + } +} + +/// The terminal endpoint's answer to a status read. +class RunStatusLookup { + const RunStatusLookup(this.statusCode, this.body); + + final int statusCode; + final Map body; +} + +/// Looks up a run by its client-generated id for `GET /runStatus/`. +/// +/// A run id alone is deliberately not a capability: the caller must present +/// the same API key the run started with, compared by SHA-256 digest, so +/// anyone reading another project's class names still has to know its key. +/// +/// 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). +/// - 200 with the record when found and authenticated. +Future fetchRunStatus({ + required String runId, + required String? presentedKey, + required FirestoreRunStore store, +}) async { + if (presentedKey == null || presentedKey.isEmpty) { + return const RunStatusLookup(401, { + 'success': false, + 'error': 'An API key is required to read a run.', + }); + } + + final Map? document; + try { + document = await store.get(runId); + } catch (error) { + stderr.writeln('[run status] lookup failed for $runId: $error'); + return const RunStatusLookup(503, { + 'success': false, + 'error': 'Run status is temporarily unavailable.', + }); + } + if (document == null) { + return const RunStatusLookup(404, { + 'success': false, + 'error': 'Unknown run.', + }); + } + + if ('${document['keyHash']}' != sha256Hex(utf8.encode(presentedKey))) { + return const RunStatusLookup(403, { + 'success': false, + 'error': 'The API key does not match this run.', + }); + } + + return RunStatusLookup(200, { + 'runId': document['runId'], + 'projectId': document['projectId'], + 'status': document['status'], + 'phase': document['phase'], + 'message': document['message'], + 'error': document['error'], + 'deployed': document['deployed'], + 'dryRun': document['dryRun'] == true, + 'updatedAt': document['updatedAt'], + 'classNames': document['classNames'], + }); +} diff --git a/cloud-run/ffai-runner/lib/sha256.dart b/cloud-run/ffai-runner/lib/sha256.dart new file mode 100644 index 0000000..d500498 --- /dev/null +++ b/cloud-run/ffai-runner/lib/sha256.dart @@ -0,0 +1,111 @@ +/// SHA-256 as a dependence-free pure-Dart implementation. +/// +/// The runner keeps zero runtime dependencies so its image build stays fast, +/// and that constraint is exactly why this exists: the deploy-run record must +/// authenticate status reads without ever storing the callers' API key, so it +/// stores a digest of the key instead. Rather than pull in a pub package (the +/// one thing this codebase deliberately avoids at runtime), the digest is +/// computed here and pinned by tests against the standard FIPS-180-4 vectors. +/// +/// The digest is used only as a capability the caller proves it knows the key +/// for, compared for equality in the status handler - it is not a MAC and is +/// never used to transit or key anything, so there is no secret material to +/// protect from timing side channels. +library; + +/// Returns the SHA-256 digest of [bytes] as a lowercase hex string. +String sha256Hex(List bytes) { + final state = [ + 0x6a09e667, 0xbb67ae85, 0x3c6ef372, 0xa54ff53a, + 0x510e527f, 0x9b05688c, 0x1f83d9ab, 0x5be0cd19, + ]; + + final messageLengthBits = bytes.length * 8; + // Padding: 0x80, zeros to a 56-byte residue, then the 64-bit length. + final paddedLength = ((bytes.length + 1 + 8 + 63) ~/ 64) * 64; + final padded = List.filled(paddedLength, 0); + for (var i = 0; i < bytes.length; i++) { + padded[i] = bytes[i]; + } + padded[bytes.length] = 0x80; + final lengthHigh = (messageLengthBits >>> 32) & 0xFFFFFFFF; + final lengthLow = messageLengthBits & 0xFFFFFFFF; + padded[paddedLength - 8] = (lengthHigh >>> 24) & 0xFF; + padded[paddedLength - 7] = (lengthHigh >>> 16) & 0xFF; + padded[paddedLength - 6] = (lengthHigh >>> 8) & 0xFF; + padded[paddedLength - 5] = lengthHigh & 0xFF; + padded[paddedLength - 4] = (lengthLow >>> 24) & 0xFF; + padded[paddedLength - 3] = (lengthLow >>> 16) & 0xFF; + padded[paddedLength - 2] = (lengthLow >>> 8) & 0xFF; + padded[paddedLength - 1] = lengthLow & 0xFF; + + final w = List.filled(64, 0); + for (var blockStart = 0; blockStart < paddedLength; blockStart += 64) { + for (var i = 0; i < 16; i++) { + final base = blockStart + i * 4; + w[i] = (((padded[base] << 24) | + (padded[base + 1] << 16) | + (padded[base + 2] << 8) | + padded[base + 3]) & + 0xFFFFFFFF); + } + _compress(state, w); + } + + return state + .map((word) => word.toRadixString(16).padLeft(8, '0')) + .join(); +} + +void _compress(List h, List w) { + for (var i = 16; i < 64; i++) { + final s0 = _rotr(w[i - 15], 7) ^ _rotr(w[i - 15], 18) ^ (w[i - 15] >>> 3); + final s1 = _rotr(w[i - 2], 17) ^ _rotr(w[i - 2], 19) ^ (w[i - 2] >>> 10); + w[i] = (w[i - 16] + s0 + w[i - 7] + s1) & 0xFFFFFFFF; + } + + var a = h[0], b = h[1], c = h[2], d = h[3]; + var e = h[4], f = h[5], g = h[6], gh = h[7]; + + for (var i = 0; i < 64; i++) { + final bigS1 = _rotr(e, 6) ^ _rotr(e, 11) ^ _rotr(e, 25); + final ch = (e & f) ^ ((~e) & g); + final temp1 = (gh + bigS1 + ch + _roundConstants[i] + w[i]) & 0xFFFFFFFF; + final bigS0 = _rotr(a, 2) ^ _rotr(a, 13) ^ _rotr(a, 22); + final maj = (a & b) ^ (a & c) ^ (b & c); + final temp2 = (bigS0 + maj) & 0xFFFFFFFF; + + gh = g; + g = f; + f = e; + e = (d + temp1) & 0xFFFFFFFF; + d = c; + c = b; + b = a; + a = (temp1 + temp2) & 0xFFFFFFFF; + } + + final tv = [a, b, c, d, e, f, g, gh]; + for (var i = 0; i < 8; i++) { + h[i] = (h[i] + tv[i]) & 0xFFFFFFFF; + } +} + +int _rotr(int value, int bits) => + ((value >>> bits) | (value << (32 - bits))) & 0xFFFFFFFF; + +// The first 32 bits of the fractional parts of the cube roots of the first 64 +// primes. Inlined as data rather than generated so the loop can index it. +const List _roundConstants = [ + 0x428a2f98, 0x71374491, 0xb5c0fbcf, 0xe9b5dba5, 0x3956c25b, 0x59f111f1, + 0x923f82a4, 0xab1c5ed5, 0xd807aa98, 0x12835b01, 0x243185be, 0x550c7dc3, + 0x72be5d74, 0x80deb1fe, 0x9bdc06a7, 0xc19bf174, 0xe49b69c1, 0xefbe4786, + 0x0fc19dc6, 0x240ca1cc, 0x2de92c6f, 0x4a7484aa, 0x5cb0a9dc, 0x76f988da, + 0x983e5152, 0xa831c66d, 0xb00327c8, 0xbf597fc7, 0xc6e00bf3, 0xd5a79147, + 0x06ca6351, 0x14292967, 0x27b70a85, 0x2e1b2138, 0x4d2c6dfc, 0x53380d13, + 0x650a7354, 0x766a0abb, 0x81c2c92e, 0x92722c85, 0xa2bfe8a1, 0xa81a664b, + 0xc24b8b70, 0xc76c51a3, 0xd192e819, 0xd6990624, 0xf40e3585, 0x106aa070, + 0x19a4c116, 0x1e376c08, 0x2748774c, 0x34b0bcb5, 0x391c0cb3, 0x4ed8aa4a, + 0x5b9cca4f, 0x682e6ff3, 0x748f82ee, 0x78a5636f, 0x84c87814, 0x8cc70208, + 0x90befffa, 0xa4506ceb, 0xbef9a3f7, 0xc67178f2, +]; diff --git a/cloud-run/ffai-runner/test/run_record_test.dart b/cloud-run/ffai-runner/test/run_record_test.dart new file mode 100644 index 0000000..684c6eb --- /dev/null +++ b/cloud-run/ffai-runner/test/run_record_test.dart @@ -0,0 +1,170 @@ +import 'dart:convert'; + +import 'package:ccc_ffai_runner/run_record.dart'; +import 'package:ccc_ffai_runner/sha256.dart'; +import 'package:test/test.dart'; + +String _keyHash(String key) => sha256Hex(utf8.encode(key)); + +/// Builds a store whose transport is a canned responder and whose token is +/// injected, so no metadata server or network is touched. +FirestoreRunStore _store( + Future Function( + String method, + Uri uri, { + Map? headers, + String? body, + }) transport, +) { + return FirestoreRunStore( + transport: transport, + fetchAccessToken: () async => 'test-token', + ); +} + +Future _jsonResponse(Object body) async => + FirestoreHttpResponse(200, jsonEncode(body)); + +void main() { + final key = 'ff-key-123'; + final goodDoc = { + 'runId': 'dr_abc', + 'projectId': 'ff-proj', + 'keyHash': _keyHash(key), + 'classNames': ['GaugeWidget'], + 'status': 'done', + 'phase': 'done', + 'message': 'done', + 'error': null, + 'deployed': [ + {'className': 'GaugeWidget', 'artifactId': 'g'}, + ], + 'dryRun': false, + }; + + group('fetchRunStatus auth and identity', () { + test('rejects a missing key before ever querying the store', () async { + final store = _store((method, uri, {headers, body}) async { + fail('store must not be queried without a key'); + }); + final lookup = await fetchRunStatus( + runId: 'dr_abc', + presentedKey: null, + store: store, + ); + expect(lookup.statusCode, 401); + }); + + test('404 for an unknown run id a client generated itself', () async { + final store = _store( + (method, uri, {headers, body}) async => + const FirestoreHttpResponse(404, ''), + ); + final lookup = await fetchRunStatus( + runId: 'dr_never-sent', + presentedKey: key, + store: store, + ); + expect(lookup.statusCode, 404); + }); + + test('refuses a wrong key (hash mismatch)', () async { + final store = _store( + (method, uri, {headers, body}) async => _jsonResponse({ + 'fields': { + for (final f in {'keyHash': _keyHash(key)}.entries) + f.key: {'stringValue': f.value}, + }, + }), + ); + final lookup = await fetchRunStatus( + runId: 'dr_abc', + presentedKey: 'some-other-key', + store: store, + ); + expect(lookup.statusCode, 403); + }); + + test('returns the record for the matching key', () async { + final store = _store( + (method, uri, {headers, body}) async => + _jsonResponse({'fields': encodeFields(goodDoc)}), + ); + final lookup = await fetchRunStatus( + runId: 'dr_abc', + presentedKey: key, + store: store, + ); + expect(lookup.statusCode, 200); + expect(lookup.body['status'], 'done'); + expect(lookup.body['projectId'], 'ff-proj'); + expect(lookup.body['deployed'], isA()); + // The stored digest is never returned to the caller. + expect(lookup.body.containsKey('keyHash'), isFalse); + }); + + test('503 when Firestore cannot be read', () async { + final store = _store( + (method, uri, {headers, body}) async => + const FirestoreHttpResponse(500, 'server error'), + ); + final lookup = await fetchRunStatus( + runId: 'dr_abc', + presentedKey: key, + store: store, + ); + expect(lookup.statusCode, 503); + }); + }); + + group('RunRecorder', () { + test('never throws even when Firestore rejects every write', () async { + final store = _store( + (method, uri, {headers, body}) async => + const FirestoreHttpResponse(500, 'denied'), + ); + final recorder = RunRecorder( + store, + runId: 'dr_abc', + projectId: 'ff-proj', + keyHash: _keyHash('key'), + classNames: const ['GaugeWidget'], + ); + // Intermediate progress is fire-and-forget. + recorder.recordPhase('verifying', 'Compiling...'); + // The terminal write is awaited but must not throw or hang past its cap. + await expectLater( + recorder.recordDone(const [ + {'className': 'GaugeWidget', 'artifactId': 'g'}, + ], false), + completes, + ); + await expectLater(recorder.recordFailed('something broke'), completes); + }); + }); + + group('Firestore field codec', () { + test('round-trips the record shape', () { + final original = { + 'runId': 'dr_abc', + 'status': 'done', + 'dryRun': false, + 'deployed': [ + {'className': 'GaugeWidget', 'artifactId': 'g'}, + ], + 'classNames': ['GaugeWidget'], + 'error': null, + }; + final decoded = decodeFields(encodeFields(original)); + expect(decoded, original); + }); + + test('store.get returns null for a 404', () async { + final store = _store( + (method, uri, {headers, body}) async => + const FirestoreHttpResponse(404, 'not found'), + ); + expect(await store.get('missing'), isNull); + }); + }); +} diff --git a/cloud-run/ffai-runner/test/sha256_test.dart b/cloud-run/ffai-runner/test/sha256_test.dart new file mode 100644 index 0000000..7c3ac0d --- /dev/null +++ b/cloud-run/ffai-runner/test/sha256_test.dart @@ -0,0 +1,34 @@ +import 'dart:convert'; + +import 'package:ccc_ffai_runner/sha256.dart'; +import 'package:test/test.dart'; + +void main() { + // The first two are the canonical FIPS-180-4 examples; the rest exercise the + // padding-length path (a message that ends exactly at a block boundary, and + // messages that span two blocks). Expected values verified with `shasum`. + final vectors = <(String, String)>[ + ('', 'e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855'), + ('abc', 'ba7816bf8f01cfea414140de5dae2223b00361a396177a9cb410ff61f20015ad'), + ( + 'abcdbcdecdefdefgefghfghighijhijkijkljklmklmnlmnomnopnopq', + '248d6a61d20638b8e5c026930c3e6039a33ce45964ff2167f6ecedd419db06c1', + ), + ('${'a' * 64}', 'ffe054fe7ae0cb6dc65c3af9b61d5209f439851db43d0ba5997337df154668eb'), + ('${'a' * 1000}', '41edece42d63e8d9bf515a9ba6932e1c20cbc9f5a5d134645adb5db1b9737ea3'), + ]; + + for (final (input, expected) in vectors) { + test('sha256Hex(${input.length} bytes)', () { + expect(sha256Hex(utf8.encode(input)), expected); + }); + } + + test('is deterministic and distinct for a close pair', () { + final a = sha256Hex(utf8.encode('secret-key-abc')); + final b = sha256Hex(utf8.encode('secret-key-abc')); + final c = sha256Hex(utf8.encode('secret-key-abd')); + expect(a, b); + expect(a, isNot(c)); + }); +} diff --git a/src/runStatus.js b/src/runStatus.js new file mode 100644 index 0000000..eae1bbf --- /dev/null +++ b/src/runStatus.js @@ -0,0 +1,227 @@ +/** + * Client side of the deploy-run record. + * + * When a custom-class provisioning request outlives the browser's wait bound or + * drops its connection, the only way to learn what happened used to be to + * re-read the whole project and guess. The runner now keeps a per-run record + * and answers `GET /runStatus/`: the client generates the run id and + * sends it with the deploy request, and on a lost outcome asks the runner + * directly. A 404 for an id the client itself generated is proof the request + * never reached the runner, so nothing was written and a retry is safe; a + * definitive `done` or provably pre-write `failed` answer replaces the guess. + * + * Everything an older runner predates stays a fallback: a 404, a 5xx, or an + * unrecognised body resolves to "unknown", and the caller re-reads the project + * exactly as before. + */ + +/** The request header that carries the API key to GET /runStatus. */ +export const RUN_STATUS_KEY_HEADER = "x-ff-api-key"; + +/** Prefix marking an id as client-generated (a runner could never mint these). */ +const RUN_ID_PREFIX = "dr_"; + +/** Allowed: letters, digits, `-` and `_`, up to 120 chars (matches the runner). */ +const RUN_ID_PATTERN = /^[A-Za-z0-9_-]{1,120}$/; + +/** + * Generates a client-side run id. A random UUID (prefixed so it can never be + * confused with a runner-minted id) with a small fallback for environments + * without `crypto.randomUUID`. + * @returns {string} + */ +export function generateDeployRunId() { + const cryptoObj = globalThis.crypto; + if (cryptoObj && typeof cryptoObj.randomUUID === "function") { + return `${RUN_ID_PREFIX}${cryptoObj.randomUUID()}`; + } + return `${RUN_ID_PREFIX}${Date.now().toString(36)}${Math.random() + .toString(36) + .slice(2, 10)}`; +} + +/** + * Whether a run id is well-formed enough to place in a URL path and pass to + * the runner. Everything else is treated as absent. + * @param {string} id + * @returns {boolean} + */ +export function isValidRunId(id) { + return typeof id === "string" && RUN_ID_PATTERN.test(id); +} + +/** + * The status endpoint lives on the same runner as the provision endpoint, so + * it is derived from it rather than configured separately. The provision + * endpoint must end in /deployCustomClasses; handle the unusual case of a + * differently-shaped override by falling back to it unchanged. + * @param {string} provisionEndpoint + * @param {string} runId + * @returns {string} + */ +export function runStatusUrl(provisionEndpoint, runId) { + const base = String(provisionEndpoint || "").replace( + /\/deployCustomClasses$/, + "", + ); + return `${base}/runStatus/${encodeURIComponent(runId)}`; +} + +/** + * Normalises a run-status response body into the shape the outcome mapper + * reads, or null when it is not a recognisable record (an older runner, or a + * malformed body - both must act as "unknown"). + * @param {Object} body - Parsed JSON from the runner + * @returns {Object|null} + */ +export function readRunStatus(body) { + if (!body || typeof body !== "object") return null; + const { status } = body; + if (status !== "running" && status !== "done" && status !== "failed") { + return null; + } + return { + status, + phase: typeof body.phase === "string" ? body.phase : "", + message: typeof body.message === "string" ? body.message : "", + error: typeof body.error === "string" ? body.error : null, + deployed: Array.isArray(body.deployed) ? body.deployed : [], + dryRun: body.dryRun === true, + }; +} + +/** + * The runner phases that provably precede any write to FlutterFlow, so a + * failure recorded at one of them is a definitive refusal. A failure at or + * after `deploy_start` may have landed classes and stays "unknown" - the + * caller must re-read the project to know what happened. + */ +export const PRE_WRITE_RUN_PHASES = new Set([ + "connected", + "workspace_init", + "workspace_ready", + "verifying", +]); + +/** + * Maps a run-status record to a decision the deploy flow can act on. + * + * Returns `{ definitive: "success", deployed }` only when the runner recorded + * a clean finish; `{ definitive: "failed", error }` only for a failure that + * provably preceded any write; and `{ definitive: null }` for a run still in + * progress or a failure whose write state is unknown - in both cases the caller + * falls back to reconciling by re-reading the project. + * @param {Object|null} record - From `readRunStatus` + * @returns {{definitive: (string|null), deployed?: Array, error?: string}} + */ +export function runStatusOutcome(record) { + if (!record || !record.status) return { definitive: null }; + if (record.status === "done") { + return { + definitive: "success", + deployed: Array.isArray(record.deployed) ? record.deployed : [], + }; + } + if (record.status === "failed" && PRE_WRITE_RUN_PHASES.has(record.phase)) { + return { + definitive: "failed", + error: + record.error || "The deploy failed before writing anything to FlutterFlow.", + }; + } + return { definitive: null }; +} + +/** + * Fetches the run-status record once. Any non-2xx (404 for a never-arrived or + * older-runner id, 5xx from an unreachable backend) or a fetch that throws or + * times out resolves to null - "unknown" - never an exception. + * @param {Object} opts + * @param {string} opts.provisionEndpoint + * @param {string} opts.runId + * @param {string} opts.apiKey + * @param {Function} [opts.fetchImpl] - Injectable for tests + * @param {number} [opts.timeoutMs] + * @returns {Promise} + */ +export async function fetchRunStatus({ + provisionEndpoint, + runId, + apiKey, + fetchImpl, + timeoutMs = 8000, +}) { + if (!isValidRunId(runId)) return null; + const doFetch = fetchImpl || globalThis.fetch; + if (!doFetch) return null; + + const controller = new AbortController(); + const timer = setTimeout(() => controller.abort(), timeoutMs); + try { + const response = await doFetch( + runStatusUrl(provisionEndpoint, runId), + { + method: "GET", + headers: { [RUN_STATUS_KEY_HEADER]: String(apiKey || "") }, + signal: controller.signal, + }, + ); + if (!response.ok) return null; + const body = await response.json(); + return readRunStatus(body); + } catch { + return null; + } finally { + clearTimeout(timer); + } +} + +/** + * Polls the run-status endpoint under a hard budget until it yields a + * definitive outcome, or the budget runs out. Used when a provision whose + * response was lost may still be finishing on the runner - this is the bounded + * ask that replaces the guess when the record is definitive. + * @param {Object} opts - As `fetchRunStatus`, plus a budget + * @param {number} [opts.budgetMs] + * @param {number} [opts.intervalMs] + * @returns {Promise<{definitive: (string|null), deployed?: Array, error?: string}>} + */ +export async function queryRunStatusWithinBudget({ + provisionEndpoint, + runId, + apiKey, + fetchImpl, + budgetMs = 12000, + intervalMs = 2000, +}) { + const deadline = Date.now() + budgetMs; + let nullPolls = 0; + while (Date.now() < deadline) { + const record = await fetchRunStatus({ + provisionEndpoint, + runId, + apiKey, + fetchImpl, + }); + if (record) { + const outcome = runStatusOutcome(record); + if (outcome.definitive) return outcome; + // A recognised but still-running record is worth waiting on: the runner + // reached back, so its answer is on its way. + nullPolls = 0; + } else { + // A 404 (never arrived, or an older runner with no endpoint) or an + // unreachable endpoint will not resolve within the budget. A single + // transient miss is retried once, then the question is left to the caller + // instead of burning the budget polling an endpoint that cannot answer. + nullPolls += 1; + if (nullPolls >= 2) return { definitive: null }; + } + const remaining = deadline - Date.now(); + if (remaining <= 0) break; + await new Promise((resolve) => + setTimeout(resolve, Math.min(intervalMs, remaining)), + ); + } + return { definitive: null }; +} diff --git a/src/runStatus.test.js b/src/runStatus.test.js new file mode 100644 index 0000000..87fe660 --- /dev/null +++ b/src/runStatus.test.js @@ -0,0 +1,186 @@ +import { describe, it } from "node:test"; +import assert from "node:assert/strict"; +import { + generateDeployRunId, + isValidRunId, + runStatusUrl, + readRunStatus, + runStatusOutcome, + fetchRunStatus, + queryRunStatusWithinBudget, + PRE_WRITE_RUN_PHASES, + RUN_STATUS_KEY_HEADER, +} from "./runStatus.js"; + +describe("runStatus — id plumbing", () => { + it("generates a valid, prefixed run id", () => { + const id = generateDeployRunId(); + assert.ok(id.startsWith("dr_")); + assert.ok(isValidRunId(id)); + }); + + it("rejects invalid and over-long ids", () => { + assert.equal(isValidRunId(""), false); + assert.equal(isValidRunId("has a space"), false); + assert.equal(isValidRunId("a".repeat(121)), false); + assert.equal(isValidRunId(null), false); + }); + + it("derives the status url from the provision endpoint", () => { + assert.equal( + runStatusUrl("https://runner.example/deployCustomClasses", "dr_123"), + "https://runner.example/runStatus/dr_123", + ); + assert.equal( + runStatusUrl("/api/ffai-runner/deployCustomClasses", "dr_123"), + "/api/ffai-runner/runStatus/dr_123", + ); + }); +}); + +describe("runStatus — response-to-outcome mapping", () => { + it("accepts only recognisable records", () => { + assert.equal(readRunStatus({ status: "bogus" }), null); + assert.equal(readRunStatus("not an object"), null); + assert.equal(readRunStatus(null), null); + }); + + it("maps a done record to a definitive success with deployed classes", () => { + const outcome = runStatusOutcome( + readRunStatus({ + status: "done", + deployed: [{ className: "GaugeWidget", artifactId: "g" }], + }), + ); + assert.deepEqual(outcome, { + definitive: "success", + deployed: [{ className: "GaugeWidget", artifactId: "g" }], + }); + }); + + it("maps a pre-write failure to a definitive failure", () => { + const outcome = runStatusOutcome( + readRunStatus({ + status: "failed", + phase: "verifying", + error: "does not compile", + }), + ); + assert.deepEqual(outcome, { + definitive: "failed", + error: "does not compile", + }); + }); + + it("leaves a running record unknown", () => { + const outcome = runStatusOutcome( + readRunStatus({ status: "running", phase: "uploading" }), + ); + assert.equal(outcome.definitive, null); + }); + + it("leaves a post-write failure unknown so the caller reconciles", () => { + assert.ok(PRE_WRITE_RUN_PHASES.has("verifying")); + assert.ok(!PRE_WRITE_RUN_PHASES.has("deploy_start")); + const outcome = runStatusOutcome( + readRunStatus({ status: "failed", phase: "uploading" }), + ); + assert.equal(outcome.definitive, null); + }); +}); + +describe("runStatus — client fallback when the endpoint is absent or 404s", () => { + // A never-arrived request is a 404 the client can distinguish from "unknown": + // it generated the id itself, so not finding it proves nothing was written. + it("resolves a 404 to unknown, never a failure", async () => { + const fetchImpl = async () => ({ ok: false, status: 404 }); + const outcome = await queryRunStatusWithinBudget({ + provisionEndpoint: "https://runner.example/deployCustomClasses", + runId: "dr_never-sent", + apiKey: "key", + fetchImpl, + budgetMs: 50, + intervalMs: 10, + }); + assert.equal(outcome.definitive, null); + }); + + it("sends the api key in the run-status header", async () => { + let sentHeader = null; + let sentUrl = null; + const fetchImpl = async (url, opts) => { + sentUrl = url; + sentHeader = opts.headers[RUN_STATUS_KEY_HEADER]; + return { ok: true, json: async () => ({ status: "done", deployed: [] }) }; + }; + const outcome = await queryRunStatusWithinBudget({ + provisionEndpoint: "https://runner.example/deployCustomClasses", + runId: "dr_123", + apiKey: "the-key", + fetchImpl, + budgetMs: 50, + }); + assert.equal(sentUrl, "https://runner.example/runStatus/dr_123"); + assert.equal(sentHeader, "the-key"); + assert.equal(outcome.definitive, "success"); + }); + + it("returns a definitive success even if the first poll sees it running", async () => { + let calls = 0; + const fetchImpl = async () => { + calls += 1; + if (calls === 1) { + return { ok: true, json: async () => ({ status: "running", phase: "uploading" }) }; + } + return { ok: true, json: async () => ({ status: "done", deployed: [] }) }; + }; + const outcome = await queryRunStatusWithinBudget({ + provisionEndpoint: "https://runner.example/deployCustomClasses", + runId: "dr_123", + apiKey: "key", + fetchImpl, + budgetMs: 500, + intervalMs: 10, + }); + assert.equal(outcome.definitive, "success"); + }); + + it("ignores a malformed body (older runner) and falls back to unknown", async () => { + const fetchImpl = async () => ({ ok: true, json: async () => ({ event: "heartbeat" }) }); + const outcome = await queryRunStatusWithinBudget({ + provisionEndpoint: "https://runner.example/deployCustomClasses", + runId: "dr_123", + apiKey: "key", + fetchImpl, + budgetMs: 50, + intervalMs: 10, + }); + assert.equal(outcome.definitive, null); + }); + + it("does not throw the app when a network error lands mid-poll", async () => { + const fetchImpl = async () => { + throw new TypeError("fetch failed"); + }; + const outcome = await queryRunStatusWithinBudget({ + provisionEndpoint: "https://runner.example/deployCustomClasses", + runId: "dr_123", + apiKey: "key", + fetchImpl, + budgetMs: 50, + intervalMs: 10, + }); + assert.equal(outcome.definitive, null); + }); + + it("returns 404-unknown via fetchRunStatus for a never-arrived id", async () => { + const fetchImpl = async () => ({ ok: false, status: 404 }); + const record = await fetchRunStatus({ + provisionEndpoint: "https://runner.example/deployCustomClasses", + runId: "dr_never", + apiKey: "key", + fetchImpl, + }); + assert.equal(record, null); + }); +}); From bf71e9774aed45f0b4b51383ac7193acce49f935 Mon Sep 17 00:00:00 2001 From: Stuart Date: Sun, 4 Oct 2026 18:07:11 +1100 Subject: [PATCH 17/20] fix(runner): close the run record when a deploy throws unexpectedly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The catch-all could not reach the recorder: it was declared inside the try block, and a variable declared in a try block is not in scope in its catch — so an unexpected mid-run failure left the record reporting "running" forever, and a later status query answered with a state that was no longer true. That is exactly the unknown outcome the record exists to remove. The recorder is now declared before the try, assigned inside it, and closed out from the catch. Null when the throw preceded the record opening, so the catch cannot itself fail on an uninitialised variable. --- cloud-run/ffai-runner/bin/server.dart | 62 +++++++++++++++------------ 1 file changed, 34 insertions(+), 28 deletions(-) diff --git a/cloud-run/ffai-runner/bin/server.dart b/cloud-run/ffai-runner/bin/server.dart index b774885..b2a8683 100644 --- a/cloud-run/ffai-runner/bin/server.dart +++ b/cloud-run/ffai-runner/bin/server.dart @@ -23,6 +23,11 @@ Future main() async { Future _handle(HttpRequest request) async { final channel = _ResponseChannel(request.response); + // Declared outside the try so the catch below can close the record out. A + // variable declared inside a try block is not in scope in its catch, and an + // unexpected mid-run failure would otherwise leave the record reporting + // "running" forever — exactly the unknown state a record exists to remove. + RunRecorder? recorder; try { _writeCorsHeaders(request.response); @@ -119,28 +124,26 @@ Future _handle(HttpRequest request) async { // ask what actually happened instead of re-reading the project and // guessing. Older clients send no runId; for them nothing is recorded and // the deploy behaves exactly as before. - late RunRecorder recorder; - final recordRun = runId.isNotEmpty; - if (recordRun) { - recorder = RunRecorder( - FirestoreRunStore( - project: - Platform.environment['FIRESTORE_PROJECT'] ?? - defaultFirestoreProject, - ), - runId: runId, - projectId: projectId, - keyHash: sha256Hex(utf8.encode(apiKey)), - classNames: classes.map((entry) => entry.className).toList(), - ); - } + recorder = runId.isEmpty + ? null + : RunRecorder( + FirestoreRunStore( + project: + Platform.environment['FIRESTORE_PROJECT'] ?? + defaultFirestoreProject, + ), + runId: runId, + projectId: projectId, + keyHash: sha256Hex(utf8.encode(apiKey)), + classNames: classes.map((entry) => entry.className).toList(), + ); // Only stream for callers that asked for it, so older clients keep getting // the single JSON response they parse. if (payload['stream'] == true) { channel.beginStream(); } - _phase(channel, recordRun ? recorder : null, 'connected', + _phase(channel, recorder, 'connected', 'Connected to the FlutterFlow deploy runner.'); final workspace = Directory( @@ -151,10 +154,10 @@ Future _handle(HttpRequest request) async { workspace, apiKey, channel, - recordRun ? recorder : null, + recorder, ); if (initResult != null) { - if (recordRun) { + if (recorder != null) { await recorder.recordFailed( initResult.timedOut ? 'Preparing the FlutterFlow AI workspace timed out.' @@ -184,10 +187,10 @@ Future _handle(HttpRequest request) async { workspace, verification, channel, - recordRun ? recorder : null, + recorder, ); if (analysis != null && analysis.hasErrors) { - if (recordRun) { + if (recorder != null) { await recorder.recordFailed( 'The generated custom code does not compile.', ); @@ -231,7 +234,7 @@ Future _handle(HttpRequest request) async { _phase( channel, - recordRun ? recorder : null, + recorder, 'deploy_start', classes.length == 1 ? 'Deploying ${classes.single.className} to FlutterFlow...' @@ -243,12 +246,12 @@ Future _handle(HttpRequest request) async { workingDirectory: workspace.path, apiKey: apiKey, channel: channel, - recorder: recordRun ? recorder : null, + recorder: recorder, timeout: _deployTimeout(classes.length), ); if (result.timedOut) { - if (recordRun) { + if (recorder != null) { await recorder.recordFailed('FlutterFlow AI DSL deploy timed out.'); } await channel.result(HttpStatus.gatewayTimeout, { @@ -262,7 +265,7 @@ Future _handle(HttpRequest request) async { } if (result.exitCode != 0) { - if (recordRun) { + if (recorder != null) { await recorder.recordFailed('FlutterFlow AI DSL deploy failed.'); } await channel.result(HttpStatus.badGateway, { @@ -286,7 +289,7 @@ Future _handle(HttpRequest request) async { // The terminal write is awaited (bounded and error-swallowed in the // recorder) so a client whose response was lost can learn the run finished. - if (recordRun) { + if (recorder != null) { await recorder.recordDone(deployed, dryRun); } @@ -301,9 +304,12 @@ Future _handle(HttpRequest request) async { } catch (error, stackTrace) { stderr.writeln(error); stderr.writeln(stackTrace); - // 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). + // Close the record out too, or an unexpected failure leaves it reporting + // "running" forever and a later status query answers with a state that is + // no longer true. Best effort: the write is bounded and swallows its own + // failures, so it cannot take down the response below. Null when the throw + // preceded the record being opened, which is why it is nullable. + await recorder?.recordFailed('$error'); await channel.result(HttpStatus.internalServerError, { 'success': false, 'error': '$error', From 373d29a497368cf3ef5811e70b8c02d180d60522 Mon Sep 17 00:00:00 2001 From: Stuart Date: Sun, 4 Oct 2026 18:35:53 +1100 Subject: [PATCH 18/20] fix(runner): bound the run-status read, which held the instance at concurrency 1 Only the write path was timeout-bounded. A lookup covers the metadata-server token fetch and the Firestore read, and if either hangs the handler never returns - and Cloud Run runs this runner at concurrency 1, so that one slot is the whole instance. Every deploy routed to it would wait behind a read nobody is waiting for. The read now has its own 5s bound, twice the write's, and an expired read becomes the 503 the browser already treats as "unknown" and answers by reconciling. The bound is injectable so a test can prove the expiry without a real 5s wait; that test fails if the bound is removed. --- cloud-run/ffai-runner/lib/run_record.dart | 12 +++++++++++- .../ffai-runner/test/run_record_test.dart | 18 ++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/cloud-run/ffai-runner/lib/run_record.dart b/cloud-run/ffai-runner/lib/run_record.dart index 2e34704..eccb407 100644 --- a/cloud-run/ffai-runner/lib/run_record.dart +++ b/cloud-run/ffai-runner/lib/run_record.dart @@ -47,6 +47,15 @@ const runStatusCollection = 'deploy_runs'; /// bounded to this and its failure swallowed. const runRecordWriteTimeout = Duration(seconds: 2); +/// Cap on a status lookup, covering both the metadata-server token fetch and +/// the Firestore read. The write path is bounded so a slow backend can never +/// hold up a deploy; the read path needs its own bound for the same reason +/// pointed at the service: Cloud Run runs this runner at concurrency 1, so one +/// lookup that never returns occupies the instance's only slot and every +/// deploy routed to it waits. A read that exceeds this becomes a 503, which +/// the browser already treats as "unknown" and answers by reconciling. +const runRecordReadTimeout = Duration(seconds: 5); + /// An HTTP response from the backing store, kept opaque so the real transport /// (dart:io) and a test double share one shape. class FirestoreHttpResponse { @@ -372,6 +381,7 @@ Future fetchRunStatus({ required String runId, required String? presentedKey, required FirestoreRunStore store, + Duration readTimeout = runRecordReadTimeout, }) async { if (presentedKey == null || presentedKey.isEmpty) { return const RunStatusLookup(401, { @@ -382,7 +392,7 @@ Future fetchRunStatus({ final Map? document; try { - document = await store.get(runId); + document = await store.get(runId).timeout(readTimeout); } catch (error) { stderr.writeln('[run status] lookup failed for $runId: $error'); return const RunStatusLookup(503, { diff --git a/cloud-run/ffai-runner/test/run_record_test.dart b/cloud-run/ffai-runner/test/run_record_test.dart index 684c6eb..3345a73 100644 --- a/cloud-run/ffai-runner/test/run_record_test.dart +++ b/cloud-run/ffai-runner/test/run_record_test.dart @@ -1,3 +1,4 @@ +import 'dart:async'; import 'dart:convert'; import 'package:ccc_ffai_runner/run_record.dart'; @@ -115,6 +116,23 @@ void main() { ); expect(lookup.statusCode, 503); }); + + // The service runs at concurrency 1, so a lookup that never returns would + // occupy the instance's only slot and stall every deploy routed to it. The + // bound must turn that into an answer the browser can act on. + test('503 when the read never returns, instead of hanging forever', () async { + final store = _store( + (method, uri, {headers, body}) => + Completer().future, + ); + final lookup = await fetchRunStatus( + runId: 'dr_abc', + presentedKey: key, + store: store, + readTimeout: const Duration(milliseconds: 50), + ); + expect(lookup.statusCode, 503); + }); }); group('RunRecorder', () { From 759a511248977b637551e486572fc92ca4d8d9d4 Mon Sep 17 00:00:00 2001 From: Stuart Date: Sun, 4 Oct 2026 18:43:43 +1100 Subject: [PATCH 19/20] test(e2e): prove the run-status wiring changes a timed-out deploy's outcome PR #118 made the client generate a run id, send it with the deploy request, and on an unconfirmed provision ask GET /runStatus/ whether the runner recorded a definitive outcome. The mechanism is identity: the id queried must be the id the deploy carried, or the answer belongs to someone else's run. This spec captures both ids and asserts they match in every case, so a spec that stubbed the two independently would pass even if the wiring were broken. Three outcomes are driven through the real __CCC_PROVISION_CUSTOM_CLASSES__ transport with the deploy stalled past a shortened UI bound: (a) status DONE -> provision resolves as a known success, not unconfirmed (b) status FAILED at a pre-write phase (verifying) -> a real failure, nothing written (c) status 404 -> falls back to the existing reconcile and still, reports unconfirmed (backward compatibility) Neutralising the queryRunStatusWithinBudget call in a scratch app.js copy (never committed) flips all three to red on the same identity assertion, which is the bite that proves the spec would catch the wiring being removed. --- e2e/run-status-wiring.spec.js | 244 ++++++++++++++++++++++++++++++++++ 1 file changed, 244 insertions(+) create mode 100644 e2e/run-status-wiring.spec.js diff --git a/e2e/run-status-wiring.spec.js b/e2e/run-status-wiring.spec.js new file mode 100644 index 0000000..b5d7185 --- /dev/null +++ b/e2e/run-status-wiring.spec.js @@ -0,0 +1,244 @@ +import { test, expect } from "@playwright/test"; +import JSZip from "jszip"; +import { applyDefaultRoutes, ENDPOINTS } from "./fixtures/apiFixtures.js"; + +/** + * E2E proof of the run-status wiring (PR #118) — that the client's generated + * run id actually changes the outcome of a timed-out deploy. + * + * A deploy can outlive the browser UI bound. Before the run-status work, a + * time-out was always UNCONFIRMED and the user had to go check FlutterFlow by + * hand. Now the client generates a run id, sends it with the deploy request, + * and on an unconfirmed provision asks `GET /runStatus/` what happened. + * The whole mechanism is identity: the id queried must be the id the deploy + * carried, or the answer belongs to someone else's run. These tests therefore + * capture the id in the deploy request body AND the id in the status query and + * assert they are the same — a spec that stubbed the two independently would + * pass even if the wiring were broken. + * + * The runner's status record is a `{status, phase, message, error, deployed}` + * object served at `GET /runStatus/` on the same runner as + * `/deployCustomClasses`, exactly as `src/runStatus.js` reads it. + * + * Three outcomes are proven: + * (a) DONE -> the provision SUCCEEDS (a known success, not unconfirmed) + * and step 3 (the pubspec push) is still reachable. + * (b) FAILED at a pre-write phase ("verifying") -> a REAL failure with + * nothing written, not an unconfirmed outcome. + * (c) 404 -> an older runner / a run that never arrived: falls back to + * the existing reconcile and still reports unconfirmed — + * backward compatibility, not a crash/fabricated failure. + * + * The provisioning transport is driven through the real + * `__CCC_PROVISION_CUSTOM_CLASSES__` hook (same entry point as stu380-deploy, + * deploy-timeout-reconcile and finish-deploy), with the deploy stalled past a + * shortened `uiTimeoutMs` instead of the real 840s bound — exactly how the + * sibling timeout specs already do it. + */ + +const identity = { + projectId: "ff-proj-0007", + endpoint: "https://api.flutterflow.io", + artifactType: "CustomClass", + artifactName: "GaugeModel", + fileName: "gauge_widget.dart", +}; + +// One custom class the runner is asked to write. For the pre-write failure (b) +// nothing lands; for the reconcile fallback (c) it lands so the reconcile can +// report what actually happened. +const attemptedClass = { + "gauge_model.dart": { + artifactName: "GaugeModel", + content: "class GaugeModel { final int value; GaugeModel(this.value); }", + type: "C", + path: "lib/custom_code/gauge_model.dart", + }, +}; + +async function loadDeployHooks(page) { + await page.addInitScript(() => { + localStorage.setItem("hasSeenWalkthrough", "true"); + }); + await applyDefaultRoutes(page); + await page.goto("/"); + await page.waitForFunction( + () => typeof window.__CCC_PROVISION_CUSTOM_CLASSES__ === "function", + ); +} + +// Drive the real provision transport and return the settled outcome exactly as +// the caller sees it: the resolved value for a success, or name/outcome/message +// for a rejection. +async function provision(page, args) { + return page.evaluate(async (args) => { + try { + const result = await window.__CCC_PROVISION_CUSTOM_CLASSES__(args); + return { settled: "resolved", result }; + } catch (error) { + return { + settled: "rejected", + name: error.name, + outcome: error.outcome, + message: error.message, + }; + } + }, args); +} + +// A project export that already contains the custom class — what exists after +// a pre-write failure is NOT this (nothing is written), but what the reconcile +// fallback re-reads when the class did land. +async function projectZipWithClassLanded() { + const zip = new JSZip(); + zip.file("pubspec.yaml", "name: my_app\n"); + zip.file( + "lib/custom_code/gauge_model.dart", + "class GaugeModel { final int value; GaugeModel(this.value); }", + ); + return zip.generateAsync({ type: "base64" }); +} + +test.describe("run-status wiring on a timed-out deploy", () => { + test("(a) a runner-recorded DONE makes the timed-out provision a known success, queried with the id the deploy sent", async ({ page }) => { + await loadDeployHooks(page); + + // Capture the run id the deploy request actually carried, then never answer + // so the provision times out at the shortened bound. + let deployRunId = null; + await page.route(ENDPOINTS.deployCustomClasses, async (route) => { + deployRunId = (JSON.parse(route.request().postData() || "{}").runId) || null; + await new Promise(() => {}); // stall: the runner never answers on time + }); + + // Route the status endpoint: answer DONE and remember the id it was asked + // about, so we can prove identity rather than stub the two independently. + let queriedRunId = null; + let statusCalls = 0; + await page.route("**/runStatus/**", async (route) => { + statusCalls += 1; + queriedRunId = route.request().url().split("/runStatus/")[1] || null; + await route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify({ + status: "done", + phase: "done", + message: "done", + deployed: ["gauge_model.dart"], + dryRun: false, + }), + }); + }); + + const outcome = await provision(page, { + fileMap: attemptedClass, + uiTimeoutMs: 60, + }); + + // The status request actually went out — the wiring ran, it was not skipped. + expect(statusCalls).toBeGreaterThan(0); + // The id the client queried is the same id it sent with the deploy request: + // this is the whole mechanism, not two independent stubs. + expect(queriedRunId).toBe(deployRunId); + expect(deployRunId).toMatch(/^dr_/); + // DONE upgrades the unconfirmed provision to a KNOWN SUCCESS: it resolves, + // reports provisionSucceeded, and the flow may proceed to step 3 — it does + // NOT report an unknown outcome. + expect(outcome.settled).toBe("resolved"); + expect(outcome.result.provisionSucceeded).toBe(true); + }); + + test("(b) a runner-recorded FAILURE at a pre-write phase surfaces as a real failure, not unconfirmed", async ({ page }) => { + await loadDeployHooks(page); + + let deployRunId = null; + await page.route(ENDPOINTS.deployCustomClasses, async (route) => { + deployRunId = (JSON.parse(route.request().postData() || "{}").runId) || null; + await new Promise(() => {}); + }); + + let queriedRunId = null; + await page.route("**/runStatus/**", async (route) => { + queriedRunId = route.request().url().split("/runStatus/")[1] || null; + await route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify({ + status: "failed", + phase: "verifying", // pre-write: the compile gate refused it + message: "verifying", + error: "Compile gate refused the widget signature.", + deployed: [], + dryRun: false, + }), + }); + }); + + const outcome = await provision(page, { + fileMap: attemptedClass, + uiTimeoutMs: 60, + }); + + // Same-id identity holds for the failure path too. + expect(queriedRunId).toBe(deployRunId); + // A pre-write phase failure means nothing was written: it is a REAL failure + // (a plain Error derived from the runner's answer), NOT an unknown outcome. + expect(outcome.settled).toBe("rejected"); + expect(outcome.name).toBe("Error"); + expect(outcome.name).not.toBe("UnconfirmedDeployError"); + expect(outcome.outcome).toBeUndefined(); + expect(outcome.message).toContain("Compile gate refused"); + }); + + test("(c) a 404 status answer falls back to the reconcile and still reports unconfirmed — backward compatible", async ({ page }) => { + await loadDeployHooks(page); + + // The class lands in the export, so the reconcile re-read can report it. + await page.route("**/exportCode", async (route) => { + await route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify({ + value: { project_zip: await projectZipWithClassLanded() }, + }), + }); + }); + + let deployRunId = null; + await page.route(ENDPOINTS.deployCustomClasses, async (route) => { + deployRunId = (JSON.parse(route.request().postData() || "{}").runId) || null; + await new Promise(() => {}); + }); + + // The status endpoint answers 404 — an older runner with no run-status + // endpoint, or a run that never arrived. This is the backward-compat case. + let queriedRunId = null; + await page.route("**/runStatus/**", async (route) => { + queriedRunId = route.request().url().split("/runStatus/")[1] || null; + await route.fulfill({ + status: 404, + contentType: "application/json", + body: JSON.stringify({ success: false, error: "Unknown run." }), + }); + }); + + const outcome = await provision(page, { + fileMap: attemptedClass, + uiTimeoutMs: 60, + }); + + // Even the fallback queried the same id the deploy carried — the wiring + // attempted the truth before it fell back. + expect(queriedRunId).toBe(deployRunId); + // No definitive answer is available: it falls back to the reconcile and + // reports UNCONFIRMED — it does not crash, and it does not fabricate a + // failure. The reconcile re-read ran: the message reports the class that + // actually landed (backward-compatible truthfulness). + expect(outcome.settled).toBe("rejected"); + expect(outcome.name).toBe("UnconfirmedDeployError"); + expect(outcome.outcome).toBe("unconfirmed"); + expect(outcome.message).toContain("GaugeModel"); + expect(outcome.message).not.toContain("Compile gate refused"); + }); +}); From d5991e5c7aa391b11efcddccbda84ba5890b2373 Mon Sep 17 00:00:00 2001 From: Stuart Date: Mon, 5 Oct 2026 05:24:30 +1100 Subject: [PATCH 20/20] fix(run-status): never mint a run id from Math.random CodeQL flagged the id generator's fallback as an insecure random in a security context. The id names a Firestore document, so a guessable one could be overwritten by someone writing to a run they do not own - and the id exists to tell "never arrived" apart from "unknown", which a predictable value undermines. The fallback is now crypto.getRandomValues, and where no cryptographic source exists at all it returns null instead of a weak id. A null id is the safe answer: the runner keeps no record and the deploy behaves exactly as it did before this feature, rather than recording under a guessable key. --- src/runStatus.js | 23 +++++++++++++++++------ src/runStatus.test.js | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 6 deletions(-) diff --git a/src/runStatus.js b/src/runStatus.js index eae1bbf..9e56eda 100644 --- a/src/runStatus.js +++ b/src/runStatus.js @@ -26,18 +26,29 @@ const RUN_ID_PATTERN = /^[A-Za-z0-9_-]{1,120}$/; /** * Generates a client-side run id. A random UUID (prefixed so it can never be - * confused with a runner-minted id) with a small fallback for environments - * without `crypto.randomUUID`. - * @returns {string} + * confused with a runner-minted id), with `crypto.getRandomValues` for + * environments that have Web Crypto but not `randomUUID`. + * + * Randomness is load-bearing: the id names a Firestore document, so a + * guessable one could be overwritten by someone writing to a run they do not + * own. Where no cryptographic source exists it therefore returns `null` rather + * than a guessing-friendly id — without an id the runner keeps no record and + * the deploy behaves exactly as it did before this feature existed, which is a + * safe degradation rather than a weak id. + * @returns {string|null} */ export function generateDeployRunId() { const cryptoObj = globalThis.crypto; if (cryptoObj && typeof cryptoObj.randomUUID === "function") { return `${RUN_ID_PREFIX}${cryptoObj.randomUUID()}`; } - return `${RUN_ID_PREFIX}${Date.now().toString(36)}${Math.random() - .toString(36) - .slice(2, 10)}`; + if (cryptoObj && typeof cryptoObj.getRandomValues === "function") { + const bytes = cryptoObj.getRandomValues(new Uint8Array(16)); + return `${RUN_ID_PREFIX}${Array.from(bytes, (byte) => + byte.toString(16).padStart(2, "0"), + ).join("")}`; + } + return null; } /** diff --git a/src/runStatus.test.js b/src/runStatus.test.js index 87fe660..f9ea109 100644 --- a/src/runStatus.test.js +++ b/src/runStatus.test.js @@ -19,6 +19,38 @@ describe("runStatus — id plumbing", () => { assert.ok(isValidRunId(id)); }); + // The id names a Firestore document, so a guessable one could be overwritten + // by someone writing to a run they do not own. Without Web Crypto the right + // answer is no id at all: the runner then keeps no record and the deploy + // behaves exactly as it did before this feature. + it("returns null rather than a guessable id when there is no Web Crypto", () => { + const original = Object.getOwnPropertyDescriptor(globalThis, "crypto"); + Object.defineProperty(globalThis, "crypto", { + value: undefined, + configurable: true, + }); + try { + assert.equal(generateDeployRunId(), null); + } finally { + Object.defineProperty(globalThis, "crypto", original); + } + }); + + it("falls back to getRandomValues rather than Math.random when randomUUID is absent", () => { + const original = Object.getOwnPropertyDescriptor(globalThis, "crypto"); + Object.defineProperty(globalThis, "crypto", { + value: { getRandomValues: (array) => array.fill(7) }, + configurable: true, + }); + try { + const id = generateDeployRunId(); + assert.equal(id, `dr_${"07".repeat(16)}`); + assert.ok(isValidRunId(id)); + } finally { + Object.defineProperty(globalThis, "crypto", original); + } + }); + it("rejects invalid and over-long ids", () => { assert.equal(isValidRunId(""), false); assert.equal(isValidRunId("has a space"), false);