fly.io connector - #464
Conversation
|
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:
WalkthroughAdds an end-to-end Fly.io connector: client auth UI and proxy, server Fly.io routes and FlyioClient, agent cloud_exec support and a Prometheus metrics tool, provider registration, and Docker installation of flyctl. ChangesFly.io Connector Integration
Sequence DiagramsequenceDiagram
participant Client
participant ProxyRoute
participant ServerFlyioRoutes
participant FlyioClient
Client->>ProxyRoute: HTTP /api/proxy/flyio/{...} (status|connect|disconnect)
ProxyRoute->>ServerFlyioRoutes: forwardRequest -> /flyio_api/flyio/{...}
ServerFlyioRoutes->>FlyioClient: list_apps / query_prometheus / validate token
FlyioClient-->>ServerFlyioRoutes: apps / metrics / errors
ServerFlyioRoutes-->>ProxyRoute: JSON response
ProxyRoute-->>Client: JSON response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 `@server/chat/backend/agent/tools/cloud_exec_tool.py`:
- Around line 1465-1470: The return statement inside the flyio branch is using
an unnecessary f-string prefix for a static string; update the return in the
normalized_provider == 'flyio' branch (the block that calls
setup_flyio_environment_isolated) to use a normal string literal instead of an
f-string for the error message in json.dumps({"error": "...", "final_command":
command, "requires_connection": True}), keeping the same keys and values and
leaving setup_flyio_environment_isolated, resource_id assignment, and the
surrounding logic unchanged.
- Around line 1127-1128: Replace the two f-string logger calls in
setup_flyio_environment_isolated with lazy %-style logging: change
logger.info(f"Fly.io isolated environment configured (org: {org_slug})") to
logger.info("Fly.io isolated environment configured (org: %s)", org_slug) and
change logger.info(f"TIME: setup_flyio_environment_isolated completed in
{time.perf_counter() - fn_start:.2f}s") to logger.info("TIME:
setup_flyio_environment_isolated completed in %.2fs", time.perf_counter() -
fn_start) so interpolation is deferred until the log level is enabled
(references: setup_flyio_environment_isolated, logger.info, org_slug, fn_start).
- Line 1133: The except block currently calls logger.error(f"Failed to setup
Fly.io environment: {e}") which omits the traceback; replace that call with
logger.exception("Failed to setup Fly.io environment") (or logger.error("...",
exc_info=True)) so the traceback is captured; update the logger invocation in
cloud_exec_tool.py where the logger.error for the Fly.io setup occurs to use
logger.exception instead.
In `@server/Dockerfile`:
- Around line 210-218: Remove the unused architecture detection and FLY_ARCH
variable in the RUN instruction: delete the ARCH=$(uname -m) assignment and the
entire if/elif/else block that sets FLY_ARCH (and the echo/exit path), and
replace the multi-step RUN with a single invocation of the Fly install script
(keep the curl -fsSL "https://fly.io/install.sh" | FLYCTL_INSTALL="/usr/local"
sh line). This removes the unused FLY_ARCH symbol and relies on the install
script's built-in detection while leaving the install invocation (curl ... sh)
intact.
In `@server/main_compute.py`:
- Around line 601-603: Add an explicit CORS rule for the Fly.io blueprint so its
endpoints registered via app.register_blueprint(flyio_bp,
url_prefix="/flyio_api") receive the same explicit CORS policy as the other
cloud provider routes; update the Flask CORS configuration (where other entries
for OVH/Scaleway/Tailscale/Cloudflare are defined) to include a resource for
"/flyio_api/*" with the same origins/methods/headers settings used for the other
providers.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4b3b2f6e-16d4-420b-8ede-cf0efe711c83
⛔ Files ignored due to path filters (1)
client/public/flyio.svgis excluded by!**/*.svg
📒 Files selected for processing (17)
client/src/app/api/proxy/flyio/[...path]/route.tsclient/src/app/flyio/auth/page.tsxclient/src/components/connectors/ConnectorRegistry.tsserver/Dockerfileserver/chat/backend/agent/prompt/provider_rules.pyserver/chat/backend/agent/skills/integrations/flyio/SKILL.mdserver/chat/backend/agent/tools/cloud_exec_tool.pyserver/chat/backend/agent/tools/cloud_tools.pyserver/chat/backend/agent/tools/flyio_tool.pyserver/connectors/flyio_connector/__init__.pyserver/connectors/flyio_connector/api_client.pyserver/connectors/flyio_connector/auth.pyserver/main_compute.pyserver/routes/flyio/__init__.pyserver/routes/flyio/flyio_routes.pyserver/utils/providers.pyserver/utils/secrets/secret_ref_utils.py
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@server/chat/backend/agent/tools/cloud_exec_tool.py`:
- Around line 1132-1134: Remove the unused exception variable in the except
clause where the Fly.io environment setup fails: change the handler that
currently reads "except Exception as e:" to "except Exception:" so that
logger.exception("Failed to setup Fly.io environment") is used without the
unused variable; locate the except block in cloud_exec_tool.py around the Fly.io
setup where logger.exception is called.
In `@server/chat/backend/agent/tools/flyio_tool.py`:
- Line 36: The function query_flyio_metrics currently declares an unused
session_id and uses deprecated implicit Optional style (str = None); either
remove session_id if it's not needed or change its type to explicit optional
(session_id: str | None) and add a short comment in query_flyio_metrics
explaining how session_id will be used for context tracking, then update the
signatures for user_id to user_id: str | None as well and ensure any new
session_id usage is referenced inside the function (e.g., added to
logs/metrics/context) or remove the parameter entirely to avoid dead parameters.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: b12499a7-0bd3-4d70-a761-2683b4ecceb3
📒 Files selected for processing (4)
server/Dockerfileserver/chat/backend/agent/tools/cloud_exec_tool.pyserver/chat/backend/agent/tools/flyio_tool.pyserver/main_compute.py
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 `@server/chat/backend/agent/tools/cloud_exec_tool.py`:
- Around line 1690-1692: The gating logic is using _CLI_PREFIX as the source of
truth but 'flyio' was added later only in the provider branch
(supported_cli_tools/default_cli), so gate_command() is still seeing the
unprefixed action; update the gating to match the actual CLI you will execute by
either adding 'flyio' (and its CLI aliases 'fly'/'flyctl') to the _CLI_PREFIX
mapping used by gate_command() or by propagating the chosen CLI (default_cli or
resolved from supported_cli_tools) into the call to gate_command(); update
references for provider, supported_cli_tools, default_cli, gate_command, and
_CLI_PREFIX so the HITL/destructive-command checks apply to Fly.io commands
exactly as they do for other providers.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: cda69866-4243-49de-b494-22be5eae380e
📒 Files selected for processing (7)
client/src/components/tool-calls/CommandLogo.tsxclient/src/components/tool-calls/ToolExecutionWidget.tsxclient/src/components/tool-calls/tool-command-parser.tsserver/chat/backend/agent/skills/integrations/flyio/SKILL.mdserver/chat/backend/agent/tools/cloud_exec_tool.pyserver/connectors/flyio_connector/api_client.pyserver/utils/auth/command_policy.py
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
server/chat/backend/agent/tools/flyio_tool.py (1)
21-25:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGate registration on the credentials this tool actually needs.
cloud_tools.pyonly registersquery_flyio_metricswhenis_flyio_connected()returns true, but this function immediately fails unless bothapi_tokenandorg_slugare present. As written, a user can get a registered Fly.io metrics tool that is guaranteed to return"Incomplete Fly.io credentials".♻️ Proposed fix
def is_flyio_connected(user_id: str) -> bool: """Check if Fly.io is connected for a user.""" try: - token_data = get_token_data(user_id, "flyio") - return bool(token_data and "api_token" in token_data) + token_data = get_token_data(user_id, "flyio") or {} + return bool(token_data.get("api_token") and token_data.get("org_slug")) except Exception: return FalseBased on learnings: gate backend tool capabilities with connectivity checks so tools are registered only when credentials exist.
Also applies to: 42-46
🤖 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 `@server/chat/backend/agent/tools/flyio_tool.py` around lines 21 - 25, The is_flyio_connected check currently only verifies presence of "api_token" but the Fly.io tool (query_flyio_metrics) also requires "org_slug", so update is_flyio_connected to fetch token_data via get_token_data(user_id, "flyio") and return True only when token_data contains both "api_token" and "org_slug"; also scan and adjust any other places (e.g., similar gate checks around query_flyio_metrics) that gate registration based on is_flyio_connected so they consistently require both fields before registering the Fly.io tool.
♻️ Duplicate comments (1)
server/chat/backend/agent/tools/cloud_exec_tool.py (1)
1397-1401:⚠️ Potential issue | 🟠 Major | ⚡ Quick winCanonicalize
flyctlbefore the unified gate.
startswith("fly")also matchesflyctl, sogate_command()still receivesflyctl apps destroy ...here while the execution path later accepts bothflyandflyctl. Because this gate depends on CLI-prefixed patterns,flyctlcommands can still miss theflyapproval/HITL rules unless you normalize the alias first.♻️ Proposed fix
_CLI_PREFIX = {"aws": "aws", "gcp": "gcloud", "azure": "az", "scaleway": "scw", "ovh": "ovhcloud", "tailscale": "tailscale", "flyio": "fly"} prefix = _CLI_PREFIX.get(provider.lower(), "") - gated_cmd = f"{prefix} {command}" if prefix and not command.strip().startswith(prefix) else command + normalized_cmd = command.strip() + if provider.lower() == "flyio": + if normalized_cmd.startswith("flyctl "): + normalized_cmd = f"fly {normalized_cmd[len('flyctl '):]}" + elif not normalized_cmd.startswith("fly "): + normalized_cmd = f"fly {normalized_cmd}" + gated_cmd = normalized_cmd + else: + gated_cmd = ( + f"{prefix} {normalized_cmd}" + if prefix and not normalized_cmd.startswith(f"{prefix} ") + else normalized_cmd + )🤖 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 `@server/chat/backend/agent/tools/cloud_exec_tool.py` around lines 1397 - 1401, The gate builds gated_cmd using _CLI_PREFIX and then checks startswith(prefix); modify the logic in the gate (around _CLI_PREFIX, prefix, and where gated_cmd is computed in gate_command()/cloud_exec_tool.py) to canonicalize any leading "flyctl" to "fly" before applying the CLI-prefix gating: detect if command.strip() starts with "flyctl" and replace that token with "fly" so gated_cmd uses the canonical "fly" CLI, ensuring the later approval/HITL checks that expect "fly" match correctly while preserving other provider prefixes and the existing startswith(prefix) behavior.
🤖 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.
Outside diff comments:
In `@server/chat/backend/agent/tools/flyio_tool.py`:
- Around line 21-25: The is_flyio_connected check currently only verifies
presence of "api_token" but the Fly.io tool (query_flyio_metrics) also requires
"org_slug", so update is_flyio_connected to fetch token_data via
get_token_data(user_id, "flyio") and return True only when token_data contains
both "api_token" and "org_slug"; also scan and adjust any other places (e.g.,
similar gate checks around query_flyio_metrics) that gate registration based on
is_flyio_connected so they consistently require both fields before registering
the Fly.io tool.
---
Duplicate comments:
In `@server/chat/backend/agent/tools/cloud_exec_tool.py`:
- Around line 1397-1401: The gate builds gated_cmd using _CLI_PREFIX and then
checks startswith(prefix); modify the logic in the gate (around _CLI_PREFIX,
prefix, and where gated_cmd is computed in gate_command()/cloud_exec_tool.py) to
canonicalize any leading "flyctl" to "fly" before applying the CLI-prefix
gating: detect if command.strip() starts with "flyctl" and replace that token
with "fly" so gated_cmd uses the canonical "fly" CLI, ensuring the later
approval/HITL checks that expect "fly" match correctly while preserving other
provider prefixes and the existing startswith(prefix) behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7d60e6a2-259f-499a-a252-0360a2a9408d
📒 Files selected for processing (2)
server/chat/backend/agent/tools/cloud_exec_tool.pyserver/chat/backend/agent/tools/flyio_tool.py
damianloch
left a comment
There was a problem hiding this comment.
Only did partial review until cloud_exec_tool.py. Tested connector on ui
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
client/src/app/flyio/auth/page.tsx (1)
144-159:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFly.io disconnect should clear provider preference
client/src/app/flyio/auth/page.tsxhandleDisconnect(success branch around lines 144-156) only removesisFlyioConnectedand dispatchesproviderStateChanged; it never deselects Fly.io in provider preferences (noproviderPreferencesService.smartAutoSelect('flyio', false)and noproviderPreferenceChangedwithdetail: { providers: [] }), while other connectors do emitproviderPreferenceChangedwith an emptyprovidersarray on disconnect.🤖 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 `@client/src/app/flyio/auth/page.tsx` around lines 144 - 159, In handleDisconnect's success branch, clear Fly.io from provider preferences by calling providerPreferencesService.smartAutoSelect('flyio', false) and then dispatch the same providerPreferenceChanged event other connectors use (new CustomEvent('providerPreferenceChanged', { detail: { providers: [] } })); keep the existing localStorage removal, providerStateChanged dispatch, toasts, and state resets (setStatus, setApiToken, setOrgSlug) but add those two preference-clearing steps so Fly.io is deselected on disconnect.
🤖 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 `@client/src/app/api/provider-preferences/route.ts`:
- Line 53: Extract the duplicated provider literals into a single exported
constant (e.g., PROVIDERS) and use that constant wherever validProviders or
allProviders are currently defined (notably in the GET and POST handlers and any
other occurrences). Move the OVH conditional logic to run once when building
PROVIDERS (apply the OVH filter/append there) so the handlers simply reference
PROVIDERS (or a derived allowed list like ALLOWED_PROVIDERS =
PROVIDERS.filter(...) if needed), replacing the three separate literal arrays to
keep the list in one place.
In `@client/src/app/flyio/auth/page.tsx`:
- Around line 51-85: The current useEffect + loadStatus in page.tsx directly
calls fetch(`/api/proxy/flyio/status`) and the validate variant instead of using
the app's query system; replace this bespoke logic with a useQuery (or
queryClient.fetchQuery/invalidateQueries) for the Fly.io status and enable
revalidateOnEvents for the existing state events so it re-runs when connection
changes. Specifically, remove the loadStatus/useEffect block and wire the
component to the same query key used by useConnectedAccounts (e.g., the Fly.io
status query) and configure revalidateOnEvents to include 'providerStateChanged'
and 'providerConnectionAction' (or call queryClient.invalidateQueries with that
key when those events fire) so connect/disconnect triggers the same event-driven
revalidation as other providers while retaining error handling and localStorage
updates via your existing applyStatusResponse.
In `@server/chat/backend/agent/tools/cloud_exec_tool.py`:
- Around line 1407-1416: The current error hint always recommends "Observability
Only" or "Standard Operations" for any "No matching allow rule" denial; update
the branch that builds error_msg (using gate.code and gate.block_reason) to
choose the remediation based on whether the blocked command is read-only or
mutating: detect read-only via an existing gate flag (e.g., gate.is_read_only or
similar) or, if none exists, derive it from the command/context (e.g., the
prefix/command string or provider) and then append the Observability/Standard
templates for read-only operations but recommend "Full Cloud Access" (or a
manual allow rule) for mutating/write commands; keep the existing error_msg and
json structure but swap the suggested template text based on that check.
In `@server/Dockerfile`:
- Around line 210-211: The RUN pipeline that pipes the Fly installer into sh
(the ARG FLYCTL_VERSION + RUN curl ... | FLYCTL_INSTALL=... FLYCTL_VERSION=...
sh) can mask curl failures; change it to first download the installer to a
temporary file with curl (ensuring curl exits non-zero on failure), verify the
download succeeded, then execute the installer file with the environment
variables (FLYCTL_INSTALL and FLYCTL_VERSION) via sh /tmp/install_fly.sh, and
remove the temp file; this guarantees the build fails if fetching the script
fails and keeps the ARG FLYCTL_VERSION usage intact.
In `@server/utils/auth/command_policy.py`:
- Around line 756-757: The standard_ops whitelist currently includes "deploy" in
the regex at priority 139 which contradicts the "no infrastructure mutations"
intent; update the command_policy entry (the dict with "priority": 139 and
description "Fly.io standard operations") to remove "deploy" from the pattern
(or alternatively change the description/template to explicitly allow deploys if
you intend to keep it), ensuring the regex no longer matches deploy invocations
so mid-tier standard_ops does not grant rollout/write access.
---
Outside diff comments:
In `@client/src/app/flyio/auth/page.tsx`:
- Around line 144-159: In handleDisconnect's success branch, clear Fly.io from
provider preferences by calling
providerPreferencesService.smartAutoSelect('flyio', false) and then dispatch the
same providerPreferenceChanged event other connectors use (new
CustomEvent('providerPreferenceChanged', { detail: { providers: [] } })); keep
the existing localStorage removal, providerStateChanged dispatch, toasts, and
state resets (setStatus, setApiToken, setOrgSlug) but add those two
preference-clearing steps so Fly.io is deselected on disconnect.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 13d105e5-715e-4e2e-9a7b-2bc0214db4ed
📒 Files selected for processing (8)
client/src/app/api/provider-preferences/route.tsclient/src/app/flyio/auth/page.tsxserver/Dockerfileserver/chat/backend/agent/tools/cloud_exec_tool.pyserver/connectors/flyio_connector/auth.pyserver/routes/flyio/__init__.pyserver/routes/flyio/flyio_routes.pyserver/utils/auth/command_policy.py
|








Fly.io connector which connects to fly.io through cli and api for metrics
Summary by CodeRabbit
New Features
Chores