Skip to content

feat: add Vultr direct lease provider#300

Open
coygeek wants to merge 7 commits into
openclaw:mainfrom
coygeek:feat/vultr-provider
Open

feat: add Vultr direct lease provider#300
coygeek wants to merge 7 commits into
openclaw:mainfrom
coygeek:feat/vultr-provider

Conversation

@coygeek

@coygeek coygeek commented Jun 13, 2026

Copy link
Copy Markdown
Contributor

Closes #276

Summary

  • add the vultr direct Linux SSH-lease provider with typed config, built-in registration, account-bound ownership tags, per-lease SSH keys, recovery-safe cleanup, and non-mutating doctor support
  • add a Vultr REST client with bearer auth, cursor pagination, retry handling, response redaction, and fake-HTTP coverage for request/response safety
  • document the Vultr provider and add a guarded opt-in live smoke script with offline tests for gating, cleanup, recovery, and token redaction

Verification

  • go test ./internal/providers/vultr ./internal/providers/all ./internal/cli ./cmd/crabbox
  • go test -race ./internal/providers/vultr ./internal/providers/all ./internal/cli
  • go vet ./...
  • node --test scripts/live-vultr-smoke.test.js
  • node scripts/check-docs-links.mjs
  • node scripts/check-command-docs.mjs
  • scripts/check-docs.sh
  • git diff --check

Live Smoke

  • live Vultr smoke was not run because the opt-in live environment and credential boundary was not present
  • local no-live classification returned classification=environment_blocked reason=CRABBOX_LIVE_not_enabled

coygeek added 7 commits June 12, 2026 19:42
Add typed Vultr config, config-show output, and built-in provider registration for the direct Linux SSH-lease provider foundation.

Keep the lifecycle paths explicitly not implemented for the follow-up Vultr API lifecycle plan while exposing non-secret config and discovery contracts.
Add the Vultr REST client, direct SSH lease backend, ownership tags, recovery-safe cleanup, and non-mutating doctor support. Cover request shape, pagination, rate-limit retry, redaction, acquire, release, cleanup, recovery, and ownership safety with fake HTTP and unit tests.
# Conflicts:
#	internal/cli/config.go
#	internal/cli/config_cmd.go
#	internal/cli/config_test.go
#	internal/cli/providers_builtin_test.go
#	internal/cli/providers_test.go
@clawsweeper

clawsweeper Bot commented Jun 13, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed June 13, 2026, 1:09 AM ET / 05:09 UTC.

Summary
The branch adds a built-in vultr direct Linux SSH-lease provider with config loading, REST client lifecycle logic, tests, docs, and a guarded live smoke script.

Reproducibility: yes. for the PR findings from source inspection: bootSource ignores the generic explicit OS selection, and config show hard-codes the Vultr SSH user to root. I did not run tests because this review must keep the checkout read-only.

Review metrics: 2 noteworthy metrics.

  • Diff size: 24 files, +4079/-11. This is a large provider addition touching lifecycle code, config, docs, and live validation scripts.
  • New cloud surface: 1 provider, 1 live smoke script. The PR introduces a new external cloud API and credentialed provisioning path that needs runtime proof beyond unit tests.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🦐 gold shrimp
Result: blocked until real behavior proof from a real setup is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • [P1] Add redacted terminal output, logs, or a recording that shows a real Vultr warmup/run/status/stop/cleanup flow.
  • [P1] Fix generic --os/os: handling so unsupported explicit values fail closed or supported values map deliberately.
  • Align config show and its tests with the runtime limited SSH user behavior.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR body lists unit/fake/offline checks and says live Vultr smoke was not run; it needs redacted terminal/log proof from a real Vultr warmup/run/stop/cleanup path before merge. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Risk before merge

  • [P1] Live Vultr provisioning, SSH bootstrap, stop, and cleanup are not proven in a real account yet.
  • [P1] The PR can silently ignore an explicit generic --os or os: setting and provision Ubuntu 24.04 instead.
  • [P1] crabbox config show currently reports root for vultr.userScheme: limited, while runtime defaults use limited.

Maintainer options:

  1. Fix OS and config contracts first (recommended)
    Map supported generic OS values or fail closed for unsupported explicit --os, add focused tests, and align config show with the runtime SSH user before merge.
  2. Require live Vultr proof
    Ask for redacted terminal output or logs showing real warmup, run, status, stop, and cleanup behavior before treating this provider as merge-ready.
  3. Pause if live access stays unavailable
    If no Vultr account can run the smoke soon, keep this PR parked until maintainers decide whether fake-HTTP coverage is enough for a first-party cloud provider.

Next step before merge

  • [P1] Manual follow-up is needed because the contributor must provide real Vultr proof and maintainers may need to accept the first-party provider/product direction before merge.

Security
Cleared: The diff adds credentialed cloud API code and live scripts, but this pass found no concrete API-key or supply-chain regression beyond the functional proof gap.

Review findings

  • [P2] Honor explicit OS selection before resolving Ubuntu — internal/providers/vultr/client.go:436-440
  • [P2] Show the limited SSH user in config output — internal/cli/config_cmd.go:49-50
Review details

Best possible solution:

Land a Vultr provider after it preserves the existing OS/config contracts, reports effective SSH defaults accurately, and includes redacted real Vultr proof or an explicit maintainer acceptance of the live-proof gap.

Do we have a high-confidence way to reproduce the issue?

Yes for the PR findings from source inspection: bootSource ignores the generic explicit OS selection, and config show hard-codes the Vultr SSH user to root. I did not run tests because this review must keep the checkout read-only.

Is this the best way to solve the issue?

No. Adding a provider adapter is the right overall shape, but the implementation should preserve generic OS/config behavior and provide real cloud proof before merge.

Full review comments:

  • [P2] Honor explicit OS selection before resolving Ubuntu — internal/providers/vultr/client.go:436-440
    bootSource falls through to resolveUbuntuOS whenever the Vultr-specific boot source fields are empty. That means an explicit generic setting like --os ubuntu:26.04 still provisions the auto-resolved Ubuntu 24.04 image instead of being mapped or rejected, so tests can run on the wrong OS.
    Confidence: 0.89
  • [P2] Show the limited SSH user in config output — internal/cli/config_cmd.go:49-50
    Runtime defaults switch the SSH user to limited when vultr.userScheme is limited and no generic SSH user was explicit, and the new docs say the effective value appears in config show. This branch still hard-codes root for Vultr config output, so users get the wrong connection hint.
    Confidence: 0.86

Overall correctness: patch is incorrect
Overall confidence: 0.84

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 6704b62ca2aa.

Label changes

Label changes:

  • add P2: This is a normal-priority provider feature with concrete correctness and proof gaps but limited blast radius.
  • add merge-risk: 🚨 compatibility: The Vultr path currently ignores Crabbox's existing explicit OS setting, which can silently provision a different image than configured.
  • add rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦐 gold shrimp.
  • add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body lists unit/fake/offline checks and says live Vultr smoke was not run; it needs redacted terminal/log proof from a real Vultr warmup/run/stop/cleanup path before merge. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Label justifications:

  • P2: This is a normal-priority provider feature with concrete correctness and proof gaps but limited blast radius.
  • merge-risk: 🚨 compatibility: The Vultr path currently ignores Crabbox's existing explicit OS setting, which can silently provision a different image than configured.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦐 gold shrimp.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body lists unit/fake/offline checks and says live Vultr smoke was not run; it needs redacted terminal/log proof from a real Vultr warmup/run/stop/cleanup path before merge. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed

Acceptance criteria:

  • [P1] go test ./internal/providers/vultr ./internal/providers/all ./internal/cli ./cmd/crabbox.
  • [P1] go test -race ./internal/providers/vultr ./internal/providers/all ./internal/cli.
  • [P1] node --test scripts/live-vultr-smoke.test.js.
  • [P1] CRABBOX_LIVE=1 CRABBOX_LIVE_PROVIDERS=vultr scripts/live-vultr-smoke.sh.

What I checked:

  • current main has no Vultr provider: A current-main search for vultr, Vultr, or VULTR returned no matches, so this PR is not obsolete on main. (6704b62ca2aa)
  • explicit OS is not consumed by Vultr boot selection: The PR's bootSource only counts cfg.Vultr.OS, cfg.Vultr.Image, and cfg.Vultr.Snapshot; when those are empty it resolves Ubuntu 24.04 without checking whether the generic os/--os value was explicitly set. (internal/providers/vultr/client.go:411, f7210d2b9c79)
  • existing direct provider validates explicit OS: The Linode direct provider rejects explicit generic OS selections when no provider image mapping exists, which is the safer current-main pattern for direct cloud providers. (internal/providers/linode/config.go:66, d6be66ec65cf)
  • config show conflicts with runtime/docs for limited user scheme: The PR docs say vultr.userScheme: limited makes Crabbox use SSH user limited when no generic SSH user is explicit, but effectiveConfigForShow hard-codes root for Vultr. (internal/cli/config_cmd.go:47, f7210d2b9c79)
  • real behavior proof is absent: The PR body lists fake/unit/offline verification and says the live Vultr smoke was not run because the live environment and credential boundary were unavailable. (f7210d2b9c79)
  • repository policy and vision boundary checked: AGENTS.md and docs/vision.md were read; they allow provider-specific lifecycle, labels, retries, and resource handling inside provider adapters while keeping core limited to routing/config glue. (AGENTS.md:13, 6704b62ca2aa)

Likely related people:

  • coygeek: Authored the merged direct Linode provider and the current Vultr branch, covering the closest direct-provider analogue and the affected config/provider patterns. (role: recent area contributor; confidence: high; commits: d6be66ec65cf, f7210d2b9c79; files: internal/providers/linode/config.go, internal/providers/vultr/client.go, internal/cli/config.go)
  • Peter Steinberger: Recent current-main history and blame cover the baseline config defaults, DigitalOcean direct-provider validation pattern, release state, and provider architecture docs used as review anchors. (role: adjacent owner; confidence: medium; commits: f2f6afeb4eba, 6704b62ca2aa; files: internal/cli/config.go, internal/providers/digitalocean/backend.go, docs/vision.md)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. labels Jun 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. P2 Normal priority bug or improvement with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add Vultr as a direct Linux SSH lease provider

1 participant