Repository navigation
runner: heartbeat, health probe, and a per-class deploy cap - #114
Conversation
… 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.
|
This PR does not match any of the 1 configured review trigger rule. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| // 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'; |
There was a problem hiding this comment.
🔴 Deployed runner fails to start
When the image starts, server.dart cannot load the validation library. The image build copies bin/ but not lib/, leaving both deploys and health checks unavailable.
Learn more
The Docker image runs the Dart server from /app, but its build copies the package manifest and bin/ only. The new package import resolves to /app/lib/request_validation.dart, which does not exist in that image. The local analyzer and tests see the checked-out library, so they do not validate the image's runtime contents. The service fails at startup before it binds its port.
Example: Build with the current image instructions, then start the image. The server cannot load its validation package instead of answering GET /healthz.
Recommended fix: Copy lib/ into /app/lib/ in the Dockerfile before starting the server, and smoke-test the built image rather than only the repository checkout.
Was this helpful? React with 👍 or 👎 to provide feedback.
| workingDirectory: workspace.path, | ||
| apiKey: apiKey, | ||
| channel: channel, | ||
| timeout: _deployTimeout(classes.length), |
There was a problem hiding this comment.
🔴 Large deploys outlive request deadline
After slow verification, _deployTimeout allows a 20-class upload another 600 seconds without accounting for time already spent. The verification steps can consume 480 seconds, exceeding the configured 900-second request deadline before the upload finishes.
Learn more
The service deploy configuration sets a 900-second request deadline in the deploy command. Before _runFlutterFlow begins the upload, _verifyCustomCode permits four minutes each for flutter pub get and flutter analyze; _ensureWorkspace can also spend five minutes initializing. The new timeout starts only when the final CLI process starts. Cloud Run can therefore close the request before the runner's timeout fires, even while classes are being written.
Example: With an already initialized workspace, package resolution and analysis take 240 seconds each, then a 20-class upload takes 500 seconds. The 980-second request exceeds Cloud Run's 900-second limit, despite finishing inside the upload's 600-second cap.
Recommended fix: Establish one deadline for the entire request and cap each remaining subprocess by the smaller of its own limit and the remaining time, leaving enough time for the runner to send its final response. Include the 840-second browser wait in the total budget if a browser-visible result is required.
Was this helpful? React with 👍 or 👎 to provide feedback.
| /// vendored snapshot requires. | ||
| Future<Map<String, Object>> _healthz() async { | ||
| return { | ||
| 'sha': Platform.environment['RUNNER_GIT_SHA'] ?? '', |
There was a problem hiding this comment.
🟡 Health probe omits deployed revision
For deployments through the deploy command, _healthz always returns an empty sha. The command never sets RUNNER_GIT_SHA, so the probe cannot distinguish an old image from a newly deployed fix.
Learn more
The new health endpoint reports the RUNNER_GIT_SHA environment variable, defaulting to an empty string. The repository's deploy command only sets ALLOWED_ORIGIN. As a result, every deployment made through that command reports the same empty revision, even when its image changed.
Example: Deploy revision abc123 with the provided script, then deploy revision def456 the same way. Both /healthz responses contain "sha":"" rather than their respective revisions.
Recommended fix: Pass the source revision into the image build or deployment and expose that exact value as RUNNER_GIT_SHA; validate a standard deployment reports a nonempty matching SHA.
Was this helpful? React with 👍 or 👎 to provide feedback.
| /// 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) { |
There was a problem hiding this comment.
🔍 Manifest contract test targets old location
The manifest contract test searches only the server source for helpers moved here. Update its source and renamed constructor assertion to restore the Node test suite.
Was this helpful? React with 👍 or 👎 to provide feedback.
| final heartbeat = Timer.periodic( | ||
| const Duration(seconds: 15), | ||
| (_) => channel.heartbeat(), | ||
| ); |
There was a problem hiding this comment.
🔍 CLI heartbeat lacks error-path cleanup
If output decoding or draining fails, the periodic timer remains active after _runFlutterFlow exits. Cancel it in finally, as _runProcess does.
Was this helpful? React with 👍 or 👎 to provide feedback.
…uite 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.
What & why
Hardening the
ccc-ffai-runnerCloud Run service so a long deploy isobservable, a large bundle can't be killed mid-write, and a
merged-but-never-deployed fix is detectable in minutes.
Liveness (ITEM 4 runner half). A cold runner can be silent for a minute+
(SDK snapshot download,
flutter analyze), which the browser can't tell froma dead connection. Emits
{"event":"heartbeat"}every 15s while any CLI workruns (both
_runFlutterFlowand_runProcess). The channel drops heartbeatsfor non-streaming callers, and
src/provisionStream.js'shandleEventignores unknown events, so this is backward compatible (confirmed by reading
that file, not edited here).
Per-class deploy cap (ITEM 6).
_runFlutterFlowhard-codedDuration(minutes: 5)for every invocation. Deploy now uses_deployTimeout(classCount)= 5 min + 15s/class. Arithmetic: 1 class -> 315s;max 20 classes -> 600s (10 min), comfortably under Cloud Run's 900s request
timeout (set via
TIMEOUTin scripts/deploy_cloud_run_ffai.sh) and inside thebrowser's 840s wait. The 5-minute default — used by
flutterflow ai init(workspace init) — is kept separate/tighter.
GET /healthz (ITEM 7). Returns
{sha, flutterflowCli, minFlutterflowCli}.shacomes fromRUNNER_GIT_SHAenv with empty fallback;flutterflowCliisread live from
dart pub global list;minFlutterflowCliis a const mirroringthe Dockerfile pin. No new dependency.
Extract validation + regression test (ITEM 11). The manifest validation
helpers moved from private top-level functions in
bin/server.dartto publicfunctions in
lib/request_validation.dart, logic byte-for-byte identical.test/request_validation_test.dartpins thatflutter_web_pluginsandflutter_localizationsare accepted (the exact months-long productionregression), that dashed/colon/YAML-punctuation names are still rejected, and
that dependency-map and constraint validation still reject the inputs they
always did. Dev-only
testdependency added to pubspec.yaml.Verification
dart analyze(cloud-run/ffai-runner): no issuesdart test(cloud-run/ffai-runner): 13/13 passGET /healthz-> 200{"sha":"","flutterflowCli":"0.0.41","minFlutterflowCli":"0.0.41"}(empty sha = fallback, RUNNER_GIT_SHA not set);GET /deployCustomClasses-> 404; OPTIONS -> 204; bad POST -> 400node --test src/*.test.js: 398/399 (one pre-existing-style break, see below — this PR did NOT introduce it via src; it's a consequence of the mandated lib/ extraction)Out-of-scope changes the orchestrator must route elsewhere
COPY lib/ ./lib/.cloud-run/ffai-runner/Dockerfileonly copies
bin/, so the deployed image would fail at runtime on the newpackage:ccc_ffai_runner/request_validation.dartimport. Not in my scope toedit.
src/cloudRunRunner.test.js"Cloud Run builds the analysis manifestitself" now fails. It reads only
bin/server.dartand regex-asserts code(
String toPubspec(),_packageNamePattern = RegExp,_constraintPattern = RegExp,value['sdkOverrides'],value.containsKey('pubspec'),_VerificationRequest.unavailable(...))that legitimately moved to
lib/request_validation.dart, and_VerificationRequest.unavailable(is nowVerificationRequest.unavailable(.The owning agent should make
runnerSourcealso readlib/request_validation.dartand update that identifier. I could not edit src/ per scope.
RUNNER_GIT_SHAas an env var (the deploy script isout of my scope) for /healthz's sha to be meaningful.