feat(run): preflight Cloudflare AI Gateway seats for required env vars (#394) - #415
Conversation
A worker seat on a `cloudflare-ai-gateway/<provider>/<model>` route could reach dispatch and then die inside the child with `CLOUDFLARE_ACCOUNT_ID and CLOUDFLARE_GATEWAY_ID missing`, recorded as a generic adapter error. The route names the Cloudflare path and the required variable names are static, so this is diagnosable before the child launches. - Add `is_cloudflare_ai_gateway_route` / `missing_cloudflare_ai_gateway_env_vars` / `cloudflare_ai_gateway_preflight_detail` helpers (exact first-segment match; empty-string values count as missing; names only, never values). - `roster doctor` fails a Cloudflare seat that is missing either variable, naming the missing var(s) with the smallest remediation, never printing values. - Preflight every child-launch path before dispatch: direct worker, the grok invalid-final fallback agent, and the orchestrator seat. Missing env yields a provider-config failure (`failure_phase=preflight`, `failure_kind=provider-config`) instead of launching the child. - Propagate the provider-config classification into worker results, the top-level run.json failure fields, and the human summary. Endpoint-mode seats are intentionally exempt from the doctor check. Closes #394 Co-authored-by: Cursor <cursoragent@cursor.com>
|
@coderabbitai review |
|
bugbot run |
|
Important Review skippedAuto reviews are limited based on label configuration. 🏷️ Required labels (at least one) (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository: escoffier-labs/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughCloudflare AI Gateway routes now validate required environment variables during roster checks, worker dispatch, fallback dispatch, and orchestration. Missing configuration produces provider-config preflight failures, and receipts preserve the corresponding failure phase and metadata. ChangesCloudflare AI Gateway preflight
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant BrigadeRun
participant run_transport.dispatch
participant CloudflareGateway
participant WorkerCLI
BrigadeRun->>run_transport.dispatch: dispatch Cloudflare-routed worker
run_transport.dispatch->>CloudflareGateway: check required environment variables
CloudflareGateway-->>run_transport.dispatch: return configuration status
alt configuration missing
run_transport.dispatch-->>BrigadeRun: provider-config preflight failure
else configuration present
run_transport.dispatch->>WorkerCLI: launch worker process
WorkerCLI-->>BrigadeRun: return worker result
end
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
✅ Action performedReview finished.
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 04cc3f8. Configure here.
| missing = missing_cloudflare_ai_gateway_env_vars() | ||
| if not missing: | ||
| return None | ||
| return f"Cloudflare AI Gateway seat missing required env vars: {', '.join(missing)}; set them before running" |
There was a problem hiding this comment.
Preflight ignores seat env
Medium Severity
Cloudflare gateway preflight calls missing_cloudflare_ai_gateway_env_vars() against os.environ only. Worker and orchestrator dispatch already merge seat env (and *_REF targets) into the child via run_agent, so seats that supply CLOUDFLARE_ACCOUNT_ID / CLOUDFLARE_GATEWAY_ID only in roster env can fail preflight with provider-config even though the adapter would receive those variables.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 04cc3f8. Configure here.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/brigade/run_transport.py`:
- Around line 574-577: The fallback preflight failure path in the run flow must
preserve the prior Grok attempt history and elapsed duration. Update the
`_cloudflare_preflight_failure` result handling around `fallback_agent` so it
threads the accumulated `attempts` into the returned failure, or routes through
`finish()` while retaining that metadata; ensure the persisted WorkerResult
includes both earlier attempts. Extend
`test_cloudflare_gateway_preflight_fallback_agent_fails_without_launch` to
assert `result.attempts`.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: escoffier-labs/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 798ea65d-e927-432c-b903-a179e75aeecf
📒 Files selected for processing (8)
src/brigade/aboyeur.pysrc/brigade/agents.pysrc/brigade/roster_cmd.pysrc/brigade/run_transport.pytests/test_aboyeur.pytests/test_agents.pytests/test_roster_cmd.pytests/test_run_transport_env.py
| fallback_agent = roster.agents[fallback_name] | ||
| fallback_preflight = _cloudflare_preflight_failure(fallback_agent, assignment) | ||
| if fallback_preflight is not None: | ||
| return fallback_preflight |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Fallback-preflight failure drops the prior grok attempt history.
By the time this fallback preflight check fires, attempts already holds the initial and continuation grok attempts (built at lines 524-534 and 545-558). Returning fallback_preflight directly bypasses finish(), so those attempts (and elapsed duration_seconds) never reach the persisted WorkerResult — the receipt for this case will show only the Cloudflare provider-config failure with an empty attempts tuple, hiding the two real grok invocations that preceded the fallback.
🔧 Proposed fix: thread `attempts` into the preflight failure
-def _cloudflare_preflight_failure(agent: Agent, assignment: Assignment) -> WorkerResult | None:
+def _cloudflare_preflight_failure(
+ agent: Agent,
+ assignment: Assignment,
+ *,
+ attempts: tuple[WorkerAttempt, ...] = (),
+) -> WorkerResult | None:
"""Return a preflight failure if the agent's Cloudflare route lacks env.
Empty string values are treated as missing.
"""
detail = agents.cloudflare_ai_gateway_preflight_detail(agent.model)
if detail is None:
return None
return WorkerResult(
worker=assignment.worker,
task=assignment.task,
text="",
ok=False,
detail=detail,
failure_phase="preflight",
failure_kind="provider-config",
+ attempts=attempts,
) fallback_agent = roster.agents[fallback_name]
- fallback_preflight = _cloudflare_preflight_failure(fallback_agent, assignment)
+ fallback_preflight = _cloudflare_preflight_failure(fallback_agent, assignment, attempts=tuple(attempts))
if fallback_preflight is not None:
return fallback_preflightConsider also asserting result.attempts in test_cloudflare_gateway_preflight_fallback_agent_fails_without_launch once fixed.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fallback_agent = roster.agents[fallback_name] | |
| fallback_preflight = _cloudflare_preflight_failure(fallback_agent, assignment) | |
| if fallback_preflight is not None: | |
| return fallback_preflight | |
| fallback_agent = roster.agents[fallback_name] | |
| fallback_preflight = _cloudflare_preflight_failure(fallback_agent, assignment, attempts=tuple(attempts)) | |
| if fallback_preflight is not None: | |
| return fallback_preflight |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/brigade/run_transport.py` around lines 574 - 577, The fallback preflight
failure path in the run flow must preserve the prior Grok attempt history and
elapsed duration. Update the `_cloudflare_preflight_failure` result handling
around `fallback_agent` so it threads the accumulated `attempts` into the
returned failure, or routes through `finish()` while retaining that metadata;
ensure the persisted WorkerResult includes both earlier attempts. Extend
`test_cloudflare_gateway_preflight_fallback_agent_fails_without_launch` to
assert `result.attempts`.
Route the Cloudflare fallback-agent preflight failure through finish() so the accumulated grok attempt history and elapsed duration are preserved in the persisted WorkerResult, instead of returning a bare preflight result that dropped them. Co-authored-by: Claude <noreply@anthropic.com>
1 participant
What
Fixes #394. A worker seat on a
cloudflare-ai-gateway/<provider>/<model>route could reach dispatch and then die inside the child withError: CLOUDFLARE_ACCOUNT_ID and CLOUDFLARE_GATEWAY_ID missing, recorded as a generic adapter failure. The route identifies the Cloudflare AI Gateway path and the required variable names are static, so this is diagnosable at plan/dispatch time.Changes
agents.py):is_cloudflare_ai_gateway_route(exact first-segment match, socloudflare-ai-gateway-other/...is rejected),missing_cloudflare_ai_gateway_env_vars(empty-string counts as missing), andcloudflare_ai_gateway_preflight_detail. Names only — the secret values are never read, logged, or interpolated.roster doctor(roster_cmd.py): a Cloudflare seat missing eitherCLOUDFLARE_ACCOUNT_ID/CLOUDFLARE_GATEWAY_IDfails, naming the missing var(s) and smallest remediation without printing values. Endpoint-mode seats are intentionally exempt.run_transport.py,aboyeur.py): direct worker, the grok invalid-final fallback agent, and the orchestrator seat all preflight before launching. Missing env →failure_phase=preflight,failure_kind=provider-config, no child launched.Tests
test_agents.py(route classification incl. near-miss, missing-var lists, empty-string),test_roster_cmd.py(doctor OK/FAIL + values-not-echoed),test_run_transport_env.py(worker + fallback preflight, no child launched, values-not-echoed),test_aboyeur.py(orchestrator preflight fails before planning,run_agentasserted never called, run.json top-level classification).Verification
./scripts/verifygreen (coverage 82.42% ≥ 78 floor). Captured receipt20260721-162830-work-verify-e170be.Note
Low Risk
Additive configuration checks on a specific model route; no changes to auth, data handling, or default run paths when env is set.
Overview
Adds early validation for seats whose
modelis acloudflare-ai-gateway/...route so missingCLOUDFLARE_ACCOUNT_ID/CLOUDFLARE_GATEWAY_IDfails before any child CLI is launched, withpreflight/provider-configclassification instead of a generic adapter error.New helpers in
agents.pydetect the route (exact first path segment), list missing env vars (empty string counts as missing; only names are surfaced), and build the preflight message.roster doctorfails or passes a per-agent Cloudflare gateway check; endpoint-mode agents stay exempt.Preflight runs on the orchestrator (
aboyeur), every worker dispatch (run_transport), and the grok invalid-final fallback agent. Plan failures that originated from orchestrator preflight now writefailure_phase: preflightonrun.json, and top-levelfailure_kindis only set when a kind is known.Reviewed by Cursor Bugbot for commit 04cc3f8. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
New Features
Bug Fixes