Skip to content

feat(compute): support the self-hosted Docker provider - #227

Open
Fermionic-Lyu wants to merge 1 commit into
mainfrom
feat/compute-docker-provider
Open

feat(compute): support the self-hosted Docker provider#227
Fermionic-Lyu wants to merge 1 commit into
mainfrom
feat/compute-docker-provider

Conversation

@Fermionic-Lyu

@Fermionic-Lyu Fermionic-Lyu commented Aug 8, 2026

Copy link
Copy Markdown
Member

InsForge/InsForge#1892 added a self-hosted Docker compute driver. compute deploy still assumed Fly, so a self-hoster on the new driver got a region silently recorded and never honoured — and compute deploy <dir> could not work at all, since source mode minted a Fly deploy token and shelled out to flyctl.

The CLI now asks GET /api/metadata what the configured provider can do and shapes the request accordingly.

What changed

  • region is omitted when the provider has none, so the stored row doesn't claim a choice that never took effect. Passing --region explicitly prints that it's being ignored rather than failing.
  • scaleToZero likewise, including --always-on.
  • Source mode branches on sourceBuild:
    • context-upload (Docker) packs the directory and POSTs it to /:id/build, which builds, tags and deploys in one call — no deploy token, no flyctl on PATH, no follow-up PATCH.
    • flyctl (Fly) keeps the existing remote-build path unchanged.
    • none fails with a message naming --image as the way through. Checked before the Dockerfile lookup, since a missing Dockerfile is beside the point when the provider can't build regardless.

The context packer

Deliberately not modelled on the deployments bundler, which strips node_modules, dist, build. Those exclusions are right for a static site and wrong for a Docker build: a Dockerfile may legitimately COPY any of them, and silently dropping one produces a build failure that nothing on screen explains.

It follows Docker's own contract — everything, minus .dockerignore — with .git the single unconditional exclusion, since it's usually the largest thing in the tree and carries every secret ever committed. When a context is too big, the backend already answers 413 and names .dockerignore as the fix.

Built with archiver and ignore, both already dependencies here, so no new dependency.

Compatibility

The capability lookup never throws. A CLI that can't deploy because a probe failed is worse than one that tries the way it always has, so any failure — and any backend older than the compute slice — falls back to the Fly-shaped behaviour, field by field.

Verification

782 tests pass; eslint clean.

Existing deploy tests asserted on ossFetch call indices, which the capability probe shifted by one. They now route the mock by URL, which also survives the next call anyone adds. Eight new tests: four on capability-driven request shaping (region omitted / still sent, scaleToZero omitted, sourceBuild: none refused) and four on the packer (default inclusion, .dockerignore with negations, .git exclusion, tar block validity). Both new behaviours verified by reverting the code and watching the tests fail.

Note: this repo has no prettier config, so the diff is hand-matched to the surrounding single-quote style — no reformatting noise.

🤖 Generated with Claude Code


Summary by cubic

Add self-hosted Docker provider support to compute deploy. The CLI now reads /api/metadata capabilities and adapts deploy requests and source builds accordingly.

  • New Features

    • Probe capabilities via /api/metadata; fall back to Fly-shaped defaults if missing.
    • Skip region when the provider has none; show a note if --region is passed; still send it for multi-region providers.
    • Skip scaleToZero when unsupported; ignore --always-on accordingly.
    • Source builds respect sourceBuild: context-upload packs the directory and POSTs to /:id/build (no deploy token, no flyctl), flyctl keeps the current path, none errors with guidance to use --image.
    • New build-context packer follows .dockerignore and always excludes .git; uses archiver and ignore (no new deps).
  • Refactors

    • Tests route ossFetch mocks by URL instead of call order; added coverage for capability-driven shaping and context packing.

Written for commit 62ef0de. Summary will update on new commits.

Review in cubic

Note

Add self-hosted Docker provider support to the compute deploy command

  • Adds fetchComputeCapabilities to query /api/metadata for provider capabilities, with fallbacks to Fly.io defaults on missing or failed responses.
  • The deploy command now conditionally omits region and scaleToZero from request bodies when the provider does not support them, logging an info message in each case.
  • Adds a new context-upload source mode via packBuildContext, which tars the build directory (honoring .dockerignore, always excluding .git) and uploads it to the backend build endpoint.
  • Throws a CLIError when source mode is requested but the provider has sourceBuild: 'none', directing users to build an image manually.
  • flyctl is now only required for providers that use the flyctl-based build path.
  • Risk: providers without region or scale-to-zero support silently drop those fields rather than erroring, which may be unexpected if the flag was intentional.

Macroscope summarized 62ef0de.

`compute deploy` assumed Fly: it always sent a region, always sent scale-to-zero,
and in source mode always minted a Fly deploy token and shelled out to flyctl. A
self-hoster running the new Docker driver got a region silently recorded and never
honoured, and `compute deploy <dir>` could not work at all.

The CLI now asks GET /api/metadata what the configured provider can do and shapes
the request accordingly:

- region is omitted when the provider has none, so the stored row does not claim a
  choice that never took effect; passing --region explicitly says so rather than
  failing
- scaleToZero is omitted when unsupported, likewise for --always-on
- source mode branches on `sourceBuild`. `context-upload` packs the directory and
  POSTs it to /:id/build, which builds, tags and deploys in one call — no deploy
  token, no flyctl on PATH, no follow-up PATCH. `flyctl` keeps the existing path.
  `none` fails with a message naming --image as the way through, checked before the
  Dockerfile lookup since a missing Dockerfile is beside the point there.

The context packer is deliberately not modelled on the deployments bundler, which
strips node_modules, dist and build. Those exclusions are right for a static site and
wrong for a Docker build, where a Dockerfile may COPY any of them and dropping one
produces a failure nothing on screen explains. It follows Docker's own contract —
everything, minus .dockerignore — with .git the single unconditional exclusion, since
it is usually the largest thing in the tree and carries every secret ever committed.
Built with the archiver and ignore packages already in use here, so no new
dependency.

Capability lookup never throws: a CLI that cannot deploy because a probe failed is
worse than one that tries the way it always has, so any failure and any older backend
fall back to the Fly-shaped behaviour, field by field.

Existing deploy tests asserted on ossFetch call *indices*, which the capability probe
shifted. They now route the mock by URL, which survives the next added call too.
Eight new tests: four on capability-driven request shaping, four on the packer
(default inclusion, .dockerignore with negations, .git exclusion, tar validity). Both
new behaviours verified by reverting the code and watching them fail.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Aug 8, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@Fermionic-Lyu, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 30 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: ff46d14d-0901-44c3-a3d9-f3be21d9119a

📥 Commits

Reviewing files that changed from the base of the PR and between bf27db7 and 62ef0de.

📒 Files selected for processing (5)
  • src/commands/compute/deploy.test.ts
  • src/commands/compute/deploy.ts
  • src/lib/build-context.test.ts
  • src/lib/build-context.ts
  • src/lib/compute-capabilities.ts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@greptile-apps

greptile-apps Bot commented Aug 8, 2026

Copy link
Copy Markdown

Greptile Summary

The PR adds provider-capability-aware compute deployment and a tar-based source-build path for self-hosted Docker providers.

  • Reads compute capabilities from /api/metadata and omits unsupported region and scale-to-zero fields.
  • Uploads a .dockerignore-filtered tar context for Docker-backed source builds.
  • Preserves the existing Fly/flyctl source deployment path as the compatibility fallback.

Confidence Score: 3/5

The PR should not merge until Docker source redeploys preserve configuration updates and valid symbolic links in build contexts.

Existing Docker services silently retain stale deploy settings because the upload branch sends only a tar, while build contexts omit symlinks that Dockerfiles may consume.

Files Needing Attention: src/commands/compute/deploy.ts, src/lib/build-context.ts

Important Files Changed

Filename Overview
src/commands/compute/deploy.ts Adds capability-driven request shaping and Docker context uploads, but existing Docker services lose deploy-option updates during source redeployment.
src/lib/build-context.ts Adds in-memory tar packing with .dockerignore support, but drops symbolic links from otherwise valid Docker contexts.
src/lib/compute-capabilities.ts Adds field-level capability defaults and a non-throwing legacy Fly fallback.
src/commands/compute/deploy.test.ts Adds capability request-shaping coverage but does not exercise context-upload redeployment of existing services.
src/lib/build-context.test.ts Covers regular-file inclusion, ignore negation, .git exclusion, and tar framing, but not symlink fidelity.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[compute deploy] --> B[GET /api/metadata]
  B --> C{sourceBuild}
  C -->|none| D[Require --image]
  C -->|flyctl| E[Remote Fly build]
  E --> F[PATCH service configuration]
  C -->|context-upload| G[Pack build context]
  G --> H[POST tar to service build endpoint]
  H --> I[Backend builds and deploys]
Loading

Reviews (1): Last reviewed commit: "feat(compute): support the self-hosted D..." | Re-trigger Greptile

Comment on lines +246 to +248
if (found) {
serviceId = found.id;
if (!json) outputInfo(`Found existing service "${opts.name}", rebuilding...`);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Existing service settings stay stale

When source-redeploying an existing Docker-backed service with changed port, CPU, memory, environment, or protocol options, this branch retains only the service ID and the subsequent request sends only the tar archive. The new image is deployed while the service silently keeps its previous configuration, which can leave the container unreachable or incorrectly configured.

Knowledge Base Used: Compute & Deployments

Comment thread src/lib/build-context.ts
Comment on lines +73 to +75
if (!entry.isFile()) {
// Sockets, fifos and dangling symlinks have no meaning in a build context.
continue;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Build context drops symbolic links

When a Docker build consumes a symlink from its context, Dirent.isFile() is false and this branch silently omits the entry from the tar. A Dockerfile that copies the linked path then fails with a missing-file error or produces an image different from the corresponding local build.

Knowledge Base Used: Compute & Deployments

@jwfing jwfing left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: feat(compute): support the self-hosted Docker provider

Summary: A well-structured, defensively-designed change that teaches compute deploy to probe /api/metadata for provider capabilities and route source builds through a context upload for the self-hosted Docker driver, with a Fly-shaped fallback that never throws — no Critical issues, a few worthwhile follow-ups.

Requirements context

No matching spec/plan found under docs/specs/ (only 2026-03-27-diagnose-* and 2026-04-17-db-migrations-* live there; this repo has no docs/superpowers/). Assessed against the PR description and the referenced backend PR InsForge/InsForge#1892. I verified the two dependency claims (archiver@^7, ignore@^7 are already runtime deps used in src/commands/deployments/), and that ossFetch spreads options so the Buffer tar body reaches fetch unencoded with the application/x-tar header overriding the default application/json (src/lib/api/oss.ts:240-246) — the /build POST is wired correctly.

Findings

Critical

(none)

Suggestion

  • [software eng / analytics] Context-upload success path skips PostHog telemetry. src/commands/compute/deploy.ts:308 calls only reportCliUsage(...) on success, but both other success paths — image mode (:191) and flyctl source mode (:419) — also call await trackCommandUsage('compute', 'deploy', true). command-telemetry.ts states every command should emit exactly one PostHog event per invocation, and DEVELOPMENT.md §2 names PostHog the source of truth going forward. As written, a successful self-hosted Docker deploy emits no success event (only the catch at :442 fires trackCommandUsage on failure), so those deploys are undercounted. Add the trackCommandUsage('compute', 'deploy', true) call to the context-upload path.

  • [software eng / test coverage] The context-upload happy path and rollback are untested. The eight new tests cover capability-driven request shaping (region/scaleToZero omitted or kept, sourceBuild: 'none' refused) and the packer, but nothing drives a deploy <dir> through sourceBuild: 'context-upload' — the list→prepare→pack→POST /:id/build→output flow (deploy.ts:239-309) and the build-failure rollback (:275-285) ship with no coverage. That's the core new behavior; a regression there (including the missing telemetry call above) wouldn't be caught. Consider a test that routes the mocked /build and asserts the tar upload + rollback-on-failure.

  • [functionality] Symlinks are silently dropped from the build context. src/lib/build-context.ts:74-77 skips anything that isn't entry.isFile() (or a directory), so a symlink — even a valid one pointing inside the context — is excluded. This contradicts the module's own stated "everything, minus .dockerignore" contract, and reproduces the exact footgun the packer was written to avoid: a Dockerfile that COPYs a symlinked file/dir gets a build failure with nothing on screen explaining it. At minimum, follow the symlink for in-tree targets or note the exclusion.

  • [functionality / UX] --region notice uses process.argv instead of the parsed args. deploy.ts:136 gates the "Ignoring --region" message on process.argv.includes('--region'). Because --region has a default (:49), you can't read intent from opts — but the process.argv scan misses the --region=lhr equals form and is decoupled from what Commander actually parsed (which is also why the message can't be exercised via parseAsync). cmd.getOptionValueSource('region') === 'cli' is the robust, testable check. Note this only affects the advisory message; the actual region omission is correctly driven by capabilities.regions.

Information

  • [security] Default context includes .env/secrets unless .dockerignore'd. This is intentional and matches Docker's own contract (and the context goes to the user's own self-hosted backend), but it differs from the static deployments bundler users may expect — worth a line in --help/docs so no one is surprised that .env ships by default. .git is correctly excluded unconditionally.
  • [functionality] --scale-to-zero on a provider without scale-to-zero is dropped without a notice — only --always-on (scaleToZero === false) prints the "Ignoring" message (deploy.ts:141). Minor asymmetry.
  • The capability fallback design is sound: fetchComputeCapabilities never throws and degrades field-by-field to LEGACY_FLY, so an old backend or a failed probe keeps the historical Fly behavior. serviceId is encodeURIComponent'd on every path. No security-relevant regressions. Performance: the in-memory tar is bounded by the backend's 64MB ceiling and only one extra /api/metadata round-trip is added — acceptable for a CLI.

Verdict

approved — no Critical findings. The Suggestions (missing success telemetry + untested happy path especially) are worth addressing before merge but are non-blocking. (Informational verdict; explicit GitHub approval remains a separate human action.)

@jwfing jwfing left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - approved.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

8 issues found across 5 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/lib/compute-capabilities.ts">

<violation number="1" location="src/lib/compute-capabilities.ts:67">
P2: Providers reporting `deployTokenIssuance: false` still enter the flyctl path and request a deploy token, so deployment fails at an endpoint the capability says is unavailable. Gate that path with this capability and provide the appropriate early error/fallback, or remove the unused capability until supported.</violation>
</file>

<file name="src/commands/compute/deploy.ts">

<violation number="1" location="src/commands/compute/deploy.ts:136">
P3: Detecting an explicitly-supplied region via `process.argv.includes('--region')` misses the `--region=<value>` syntax that commander accepts. When a user writes `--region=lhr` against a single-host provider, no 'Ignoring --region' message is printed yet the region is still silently omitted from the request body (because `capabilities.regions` is false) — the exact situation the message exists to surface. Make the detection cover the `=`-form too (e.g. match `/^--region(=|$)/` against argv), so the informative message fires regardless of flag spelling.</violation>

<violation number="2" location="src/commands/compute/deploy.ts:247">
P1: Redeploying an existing Docker service ignores deploy settings such as `--port`, `--memory`, and `--env`, since this branch uploads only a tar after finding it. Update the existing service configuration before/with the build so source redeploys retain `compute deploy` semantics.</violation>

<violation number="3" location="src/commands/compute/deploy.ts:259">
P2: A filesystem/archiving failure after creating a new service skips rollback and leaves the prepared service behind. Include context packing in the rollback-protected operation so any source-build failure deletes a service created by this command.</violation>

<violation number="4" location="src/commands/compute/deploy.ts:308">
P2: The new context-upload source-build path ends with `reportCliUsage(...)` but never calls `trackCommandUsage('compute', 'deploy', true)`, unlike every other success path of this command: image mode (line 191), the flyctl remote-build path (line 419), and the shared error handler (line 442). `src/lib/command-telemetry.ts` documents that every command should emit `trackCommandUsage` exactly once per invocation, so a self-hosted Docker deploy silently produces no `cli_compute_deploy_invoked` telemetry while the other two modes do. Add the same `trackCommandUsage` call before `reportCliUsage` in this branch.</violation>
</file>

<file name="src/lib/build-context.ts">

<violation number="1" location="src/lib/build-context.ts:28">
P1: An unreadable `.dockerignore` silently disables exclusions and can upload secrets the file was meant to omit. Treat only `ENOENT` as absent; propagate other read errors.</violation>

<violation number="2" location="src/lib/build-context.ts:66">
P1: Builds fail when `.dockerignore` lists `Dockerfile`: validation succeeds, then this filter omits the file required by `/build`. Preserve `Dockerfile` (and `.dockerignore`, per Docker context semantics) regardless of ignore rules.</violation>

<violation number="3" location="src/lib/build-context.ts:73">
P1: Dockerfiles that COPY a symlinked file fail because every symlink is dropped from the build context. Include symbolic links; the configured tar archiver emits them natively.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic


let serviceId: string;
if (found) {
serviceId = found.id;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: Redeploying an existing Docker service ignores deploy settings such as --port, --memory, and --env, since this branch uploads only a tar after finding it. Update the existing service configuration before/with the build so source redeploys retain compute deploy semantics.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/commands/compute/deploy.ts, line 247:

<comment>Redeploying an existing Docker service ignores deploy settings such as `--port`, `--memory`, and `--env`, since this branch uploads only a tar after finding it. Update the existing service configuration before/with the build so source redeploys retain `compute deploy` semantics.</comment>

<file context>
@@ -199,10 +229,89 @@ export function registerComputeDeployCommand(computeCmd: Command): void {
+
+          let serviceId: string;
+          if (found) {
+            serviceId = found.id;
+            if (!json) outputInfo(`Found existing service "${opts.name}", rebuilding...`);
+          } else {
</file context>

Comment thread src/lib/build-context.ts
if (relative === '.git' || relative.startsWith('.git/')) {
continue;
}
if (matcher?.ignores(entry.isDirectory() ? `${relative}/` : relative)) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: Builds fail when .dockerignore lists Dockerfile: validation succeeds, then this filter omits the file required by /build. Preserve Dockerfile (and .dockerignore, per Docker context semantics) regardless of ignore rules.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/lib/build-context.ts, line 66:

<comment>Builds fail when `.dockerignore` lists `Dockerfile`: validation succeeds, then this filter omits the file required by `/build`. Preserve `Dockerfile` (and `.dockerignore`, per Docker context semantics) regardless of ignore rules.</comment>

<file context>
@@ -0,0 +1,87 @@
+      if (relative === '.git' || relative.startsWith('.git/')) {
+        continue;
+      }
+      if (matcher?.ignores(entry.isDirectory() ? `${relative}/` : relative)) {
+        continue;
+      }
</file context>

Comment thread src/lib/build-context.ts
await walk(absolute);
continue;
}
if (!entry.isFile()) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: Dockerfiles that COPY a symlinked file fail because every symlink is dropped from the build context. Include symbolic links; the configured tar archiver emits them natively.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/lib/build-context.ts, line 73:

<comment>Dockerfiles that COPY a symlinked file fail because every symlink is dropped from the build context. Include symbolic links; the configured tar archiver emits them natively.</comment>

<file context>
@@ -0,0 +1,87 @@
+        await walk(absolute);
+        continue;
+      }
+      if (!entry.isFile()) {
+        // Sockets, fifos and dangling symlinks have no meaning in a build context.
+        continue;
</file context>

Comment thread src/lib/build-context.ts
// Docker's .dockerignore is gitignore-ish; `ignore` covers the syntax that
// matters here. `!` negations and `**` both work.
return ignore().add(raw);
} catch {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: An unreadable .dockerignore silently disables exclusions and can upload secrets the file was meant to omit. Treat only ENOENT as absent; propagate other read errors.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/lib/build-context.ts, line 28:

<comment>An unreadable `.dockerignore` silently disables exclusions and can upload secrets the file was meant to omit. Treat only `ENOENT` as absent; propagate other read errors.</comment>

<file context>
@@ -0,0 +1,87 @@
+    // Docker's .dockerignore is gitignore-ish; `ignore` covers the syntax that
+    // matters here. `!` negations and `**` both work.
+    return ignore().add(raw);
+  } catch {
+    return null;
+  }
</file context>

regions: reported.regions ?? LEGACY_FLY.regions,
ingressModes: reported.ingressModes ?? LEGACY_FLY.ingressModes,
sourceBuild: reported.sourceBuild ?? LEGACY_FLY.sourceBuild,
deployTokenIssuance:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Providers reporting deployTokenIssuance: false still enter the flyctl path and request a deploy token, so deployment fails at an endpoint the capability says is unavailable. Gate that path with this capability and provide the appropriate early error/fallback, or remove the unused capability until supported.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/lib/compute-capabilities.ts, line 67:

<comment>Providers reporting `deployTokenIssuance: false` still enter the flyctl path and request a deploy token, so deployment fails at an endpoint the capability says is unavailable. Gate that path with this capability and provide the appropriate early error/fallback, or remove the unused capability until supported.</comment>

<file context>
@@ -0,0 +1,74 @@
+        regions: reported.regions ?? LEGACY_FLY.regions,
+        ingressModes: reported.ingressModes ?? LEGACY_FLY.ingressModes,
+        sourceBuild: reported.sourceBuild ?? LEGACY_FLY.sourceBuild,
+        deployTokenIssuance:
+          reported.deployTokenIssuance ?? LEGACY_FLY.deployTokenIssuance,
+      },
</file context>

}

if (!json) outputInfo('Packing build context...');
const { tar, fileCount } = await packBuildContext(absDir);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: A filesystem/archiving failure after creating a new service skips rollback and leaves the prepared service behind. Include context packing in the rollback-protected operation so any source-build failure deletes a service created by this command.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/commands/compute/deploy.ts, line 259:

<comment>A filesystem/archiving failure after creating a new service skips rollback and leaves the prepared service behind. Include context packing in the rollback-protected operation so any source-build failure deletes a service created by this command.</comment>

<file context>
@@ -199,10 +229,89 @@ export function registerComputeDeployCommand(computeCmd: Command): void {
+          }
+
+          if (!json) outputInfo('Packing build context...');
+          const { tar, fileCount } = await packBuildContext(absDir);
+          if (!json) {
+            const mb = (tar.length / 1024 / 1024).toFixed(1);
</file context>

console.log(` No public endpoint — reachable on the project's internal network.`);
}
}
await reportCliUsage('cli.compute.deploy', true);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: The new context-upload source-build path ends with reportCliUsage(...) but never calls trackCommandUsage('compute', 'deploy', true), unlike every other success path of this command: image mode (line 191), the flyctl remote-build path (line 419), and the shared error handler (line 442). src/lib/command-telemetry.ts documents that every command should emit trackCommandUsage exactly once per invocation, so a self-hosted Docker deploy silently produces no cli_compute_deploy_invoked telemetry while the other two modes do. Add the same trackCommandUsage call before reportCliUsage in this branch.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/commands/compute/deploy.ts, line 308:

<comment>The new context-upload source-build path ends with `reportCliUsage(...)` but never calls `trackCommandUsage('compute', 'deploy', true)`, unlike every other success path of this command: image mode (line 191), the flyctl remote-build path (line 419), and the shared error handler (line 442). `src/lib/command-telemetry.ts` documents that every command should emit `trackCommandUsage` exactly once per invocation, so a self-hosted Docker deploy silently produces no `cli_compute_deploy_invoked` telemetry while the other two modes do. Add the same `trackCommandUsage` call before `reportCliUsage` in this branch.</comment>

<file context>
@@ -199,10 +229,89 @@ export function registerComputeDeployCommand(computeCmd: Command): void {
+              console.log(`  No public endpoint — reachable on the project's internal network.`);
+            }
+          }
+          await reportCliUsage('cli.compute.deploy', true);
+          return;
+        }
</file context>

// self-hosted Docker daemon has one region and no scale-to-zero, and
// sending those anyway records a choice that never takes effect.
const { provider, capabilities } = await fetchComputeCapabilities();
if (!capabilities.regions && process.argv.includes('--region') && !json) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: Detecting an explicitly-supplied region via process.argv.includes('--region') misses the --region=<value> syntax that commander accepts. When a user writes --region=lhr against a single-host provider, no 'Ignoring --region' message is printed yet the region is still silently omitted from the request body (because capabilities.regions is false) — the exact situation the message exists to surface. Make the detection cover the =-form too (e.g. match /^--region(=|$)/ against argv), so the informative message fires regardless of flag spelling.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/commands/compute/deploy.ts, line 136:

<comment>Detecting an explicitly-supplied region via `process.argv.includes('--region')` misses the `--region=<value>` syntax that commander accepts. When a user writes `--region=lhr` against a single-host provider, no 'Ignoring --region' message is printed yet the region is still silently omitted from the request body (because `capabilities.regions` is false) — the exact situation the message exists to surface. Make the detection cover the `=`-form too (e.g. match `/^--region(=|$)/` against argv), so the informative message fires regardless of flag spelling.</comment>

<file context>
@@ -127,16 +129,36 @@ export function registerComputeDeployCommand(computeCmd: Command): void {
+        // self-hosted Docker daemon has one region and no scale-to-zero, and
+        // sending those anyway records a choice that never takes effect.
+        const { provider, capabilities } = await fetchComputeCapabilities();
+        if (!capabilities.regions && process.argv.includes('--region') && !json) {
+          outputInfo(
+            `Ignoring --region: the ${provider ?? 'configured'} provider runs on a single host.`
</file context>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants