Skip to content

feat: add ASCII Box provider#157

Merged
steipete merged 6 commits into
openclaw:mainfrom
zozo123:feat/ascii-box-provider
May 30, 2026
Merged

feat: add ASCII Box provider#157
steipete merged 6 commits into
openclaw:mainfrom
zozo123:feat/ascii-box-provider

Conversation

@zozo123
Copy link
Copy Markdown
Contributor

@zozo123 zozo123 commented May 25, 2026

Summary

  • add provider: ascii-box as a direct SSH-lease provider for ASCII Box Ubuntu sandboxes
  • use the documented box --json CLI as the control plane instead of private/guessed REST routes
  • create/list/status/delete through the CLI, prepare the managed SSH key through box ssh <id> -- true, then use standard Crabbox SSH sync/run behavior
  • add config/env/flags, provider docs, provider registration, and regression coverage

Design Notes

The first version of this PR guessed private REST execution/upload routes. This version intentionally removes that surface. ASCII Box docs call the CLI the stable automation surface, so Crabbox shells out to box --json for lifecycle operations and keeps command execution in Crabbox over SSH.

When an API key is configured, Crabbox does not pass it as a command-line argument. It writes a private Box CLI config under CRABBOX_ASCII_BOX_HOME or Crabbox's state directory with mode 0600, then runs the CLI with that isolated home. The default SSH key is the CLI-managed key under that same private home.

Config / Secrets

  • API key env: CRABBOX_ASCII_BOX_API_KEY or ASCII_BOX_API_KEY
  • CLI path env: CRABBOX_ASCII_BOX_CLI or BOX_CLI
  • optional isolated CLI home: CRABBOX_ASCII_BOX_HOME
  • config block: asciiBox.baseUrl, asciiBox.cliPath, asciiBox.workdir
  • aliases: ascii, asciibox, ascii-box

Verification

Local proof on the rebased branch:

go test ./internal/cli ./internal/providers/asciibox ./internal/providers/all
go test ./...
go build -trimpath -o bin/crabbox ./cmd/crabbox
npm run docs:check
git diff --check
.agents/skills/autoreview/scripts/autoreview --mode branch --base origin/main

Redacted live proof with a real ASCII Box API key:

crabbox doctor --provider ascii-box
ok provider provider=ascii-box timeout=10s auth=ready cli=ready control_plane=ready limits=ready mutation=false runtime=unchecked

crabbox run --provider ascii-box --no-sync -- true
provisioning provider=ascii-box lease=<redacted> slug=<redacted> ttl=1h30m0s
ascii-box CLI new failed: {"code":"billing_required","error":"Start the $20/month Box plan to create sandboxes...","event":"error","status":402}

What is proven:

  • local provider registration/config/docs/tests pass
  • CLI auth/config isolation works with a real key
  • doctor reaches the real Box CLI/account limits endpoint successfully
  • create is blocked by account billing before a Box can be provisioned
  • token-bearing CLI error output is redacted by Crabbox

Remaining live blocker:

  • Full create/SSH/sync/run/status/delete E2E still needs an ASCII Box account with billing/trial enabled. Current live response is billing_required for Box creation.

@clawsweeper
Copy link
Copy Markdown

clawsweeper Bot commented May 25, 2026

Codex review: needs real behavior proof before merge. Reviewed May 30, 2026, 4:20 PM ET / 20:20 UTC.

Summary
The branch adds a built-in ascii-box SSH-lease provider backed by the local box --json CLI, with config/env/flags, docs, registration, SSH ControlMaster opt-out, and tests.

Reproducibility: not applicable. this is a new built-in provider PR, not a bug report. The relevant verification gap is runtime proof, and the PR body currently shows doctor plus billing_required output rather than a successful lifecycle.

Review metrics: 2 noteworthy metrics.

  • Changed surface: 17 files changed, 1904 additions, 17 deletions. This is a full built-in provider addition spanning CLI config, SSH behavior, docs, registry, and provider tests.
  • Live lifecycle proof: 0 successful live create/run/delete lifecycles shown. The PR body shows real doctor/auth output but the actual box creation path is blocked by billing before the runtime lifecycle is exercised.

Merge readiness
Overall: 🦐 gold shrimp
Proof: 🦪 silver shellfish
Patch quality: 🐚 platinum hermit
Result: blocked until stronger real behavior proof 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 successful live lifecycle terminal/log proof for provider=ascii-box.
  • Remove the CHANGELOG.md entry and keep release-note context in the PR body.

Proof guidance:

  • [P1] Needs stronger real behavior proof before merge: The PR includes local tests plus redacted live doctor and billing-blocked terminal output, but still needs redacted proof of a successful create/SSH/sync or no-sync run/status/delete lifecycle; redact API keys, IPs, phone numbers, non-public endpoints, and other private details, then update the PR body for automatic re-review or ask a maintainer to comment @clawsweeper re-review.

Risk before merge

  • [P1] The branch would ship a new external provider before a successful live ASCII Box create/SSH/sync/run/status/delete lifecycle has been shown; the current live output stops at billing_required.
  • [P1] The provider intentionally executes a local box binary and writes an isolated CLI config containing the API token, so merge confidence depends on redacted fresh-auth and lifecycle proof against the real CLI/account path.
  • [P1] The PR still edits release-owned CHANGELOG.md, which should be removed before merge.

Maintainer options:

  1. Require live lifecycle proof (recommended)
    Ask the contributor or provider owner to add redacted terminal/log proof of a successful create, SSH setup, sync or no-sync run, status, and delete/stop lifecycle before merge.
  2. Accept provider contract risk
    Maintainers may intentionally merge with only doctor plus billing-blocked proof if they own the ASCII Box account and CLI contract risk for the first release.
  3. Pause until account proof is available
    If nobody can provide a billing/trial-enabled live smoke soon, keep the branch paused rather than shipping an unproven built-in provider.

Next step before merge

  • [P1] Needs contributor or provider-owner live proof plus maintainer acceptance of the new built-in provider surface; automation cannot supply a paid/trial ASCII Box lifecycle on the contributor's behalf.

Security
Cleared: No concrete security or supply-chain regression was found; the provider keeps the API key out of argv and writes the isolated CLI config with mode 0600, while live proof should still redact secrets and private endpoints.

Review findings

  • [P3] Remove the release-owned changelog entry — CHANGELOG.md:9
Review details

Best possible solution:

Land this after maintainer acceptance of the built-in provider surface, redacted successful live lifecycle proof from a paid or trial-enabled account, and removal of the release-owned changelog edit.

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

Not applicable; this is a new built-in provider PR, not a bug report. The relevant verification gap is runtime proof, and the PR body currently shows doctor plus billing_required output rather than a successful lifecycle.

Is this the best way to solve the issue?

Unclear until live proof lands; the CLI-backed SSH-lease design is a narrower and more maintainable direction than the earlier guessed REST execution surface, but the branch is not ship-ready without successful lifecycle proof and the changelog cleanup.

Full review comments:

  • [P3] Remove the release-owned changelog entry — CHANGELOG.md:9
    CHANGELOG.md is release-owned for this review lane, so normal feature PRs should leave release-note context in the PR body or commits. Please drop this line before merge.
    Confidence: 0.94

Overall correctness: patch is correct
Overall confidence: 0.82

AGENTS.md: found and applied where relevant.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 190257b0f609.

Label changes

Label changes:

  • add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
  • remove rating: 🦪 silver shellfish: Current PR rating is rating: 🦐 gold shrimp, so this older rating label is no longer current.

Label justifications:

  • P2: This is a normal-priority provider feature with bounded blast radius but real external runtime validation still outstanding.
  • merge-risk: 🚨 other: Green local tests cannot settle the new provider's real ASCII Box account, CLI, billing, SSH, and cleanup contract.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The PR includes local tests plus redacted live doctor and billing-blocked terminal output, but still needs redacted proof of a successful create/SSH/sync or no-sync run/status/delete lifecycle; redact API keys, IPs, phone numbers, non-public endpoints, and other private details, then update the PR body for automatic re-review or ask a maintainer to comment @clawsweeper re-review.
Evidence reviewed

What I checked:

  • Repository policy applied: AGENTS.md was read fully; the provider-neutral boundary, test expectations, PR summary/proof guidance, and secret-handling guidance apply to this provider PR. (AGENTS.md:11, 190257b0f609)
  • Not implemented on current main: Current main has no ascii-box, AsciiBox, ASCII Box, or asciibox source/docs matches, so the new provider is not already present. (190257b0f609)
  • Not in latest release: Latest release tag v0.22.1 points at 710cf4b and has no ASCII Box provider matches. (710cf4b5b079)
  • Provider registration: The PR registers ascii-box as a direct Linux SSH-lease provider with SSH and Crabbox sync features, then imports it in the built-in provider registry. (internal/providers/asciibox/provider.go:21, ed625a325b5c)
  • CLI and secret handling implementation: The provider requires an API key from config/env, invokes the local box CLI with --json, and writes the isolated CLI config with mode 0600 rather than passing the token as an argv value. (internal/providers/asciibox/client.go:56, ed625a325b5c)
  • Test coverage added: The PR adds unit coverage for official CLI command shape, env isolation, acquisition, claim persistence, release, status mapping, terminal status, config, and SSH ControlMaster behavior. (internal/providers/asciibox/backend_test.go:45, ed625a325b5c)

Likely related people:

  • Peter Steinberger: Authored the current ASCII Box branch commits and appears in provider-backend history for provider routing/capabilities work. (role: current PR branch implementer and recent provider-area contributor; confidence: high; commits: f8708234ee57, 9f81c0918ec6, de8940793a1f; files: internal/providers/asciibox/backend.go, internal/providers/asciibox/client.go, internal/cli/provider_backend.go)
  • Vincent Koc: Visible current-main blame/log ties provider backend, config, SSH, and v0.22.1 release state to the merge commit currently anchoring these files. (role: recent current-main area contributor; confidence: medium; commits: 710cf4b5b079, 0b131572702b; files: internal/cli/provider_backend.go, internal/cli/config.go, internal/cli/ssh.go)
  • Yossi Eliaz: Appears in prior provider-related history, including RunPod and pond/provider work, and is the contributor associated with this feature proposal context. (role: adjacent direct-provider contributor; confidence: medium; commits: cef985b92482, ed692508089d; files: internal/providers/runpod, docs/features/providers.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. labels May 25, 2026
@clawsweeper
Copy link
Copy Markdown

clawsweeper Bot commented May 25, 2026

ClawSweeper PR egg

🎁 Pass real behavior proof to wake the egg and unlock a hatchable treat.

Where did the egg go?
  • The egg game starts only after the PR passes the real-behavior proof check.
  • Before that, no creature or rarity is rolled. The treat waits for real proof.
  • This is still just collectible flavor: proof affects review readiness, not creature quality.

@clawsweeper clawsweeper Bot added P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. labels May 25, 2026
@zozo123
Copy link
Copy Markdown
Contributor Author

zozo123 commented May 25, 2026

Working Proof

Pushed proof commit 6972a5c (test: add ASCII Box provider proof).

What it proves:

  • TestRunE2EAgainstHTTPAPI exercises the ASCII Box delegated run path against an HTTP API harness: create box, archive sync upload, env file write, exec-stream output/exit parsing, env cleanup, and one-shot delete.
  • TestAsciiBoxConfigDefaultsFileAndEnv covers ASCII Box YAML config and env override loading, including ASCII_BOX_* fallback and CRABBOX_ASCII_BOX_* precedence.

Local verification run before push:

  • go test ./internal/cli ./internal/providers/asciibox ./internal/providers/all passed
  • go test ./... passed
  • core coverage gate measured 90.4%, above the 90.0% CI threshold

GitHub CI after push:

  • Docs: pass
  • Go: pass
  • Plugin: pass
  • Release Check: pass
  • Worker: pass
  • Socket checks: pass

Note: this is deterministic provider proof against the HTTP contract. A live ASCII Box smoke still requires an ASCII_BOX_API_KEY/CRABBOX_ASCII_BOX_API_KEY in the runner environment.

@zozo123
Copy link
Copy Markdown
Contributor Author

zozo123 commented May 26, 2026

PR #157 proof check

I rechecked this PR at head f197ce7363f568acabf74bc70bc79290299d90f6 in an isolated worktree.

No-secret local proof passed:

$ go test -v ./internal/providers/asciibox -run 'TestRunE2EAgainstHTTPAPI'
=== RUN   TestRunE2EAgainstHTTPAPI
--- PASS: TestRunE2EAgainstHTTPAPI (0.26s)
PASS
ok  github.com/openclaw/crabbox/internal/providers/asciibox  0.848s

$ go test -v ./internal/cli -run 'TestAsciiBoxConfigDefaultsFileAndEnv'
=== RUN   TestAsciiBoxConfigDefaultsFileAndEnv
--- PASS: TestAsciiBoxConfigDefaultsFileAndEnv (0.00s)
PASS
ok  github.com/openclaw/crabbox/internal/cli  0.664s

$ go test ./internal/cli ./internal/providers/asciibox ./internal/providers/all
ok  github.com/openclaw/crabbox/internal/cli  43.768s
ok  github.com/openclaw/crabbox/internal/providers/asciibox  1.387s
ok  github.com/openclaw/crabbox/internal/providers/all  0.730s

No-secret live probes:

$ unset ASCII_BOX_API_KEY CRABBOX_ASCII_BOX_API_KEY
$ go run ./cmd/crabbox run --provider ascii-box -- true
provider=ascii-box requires ASCII_BOX_API_KEY
exit status 2

GET https://ascii.dev/api/box/install -> 200 application/x-sh
GET https://ascii.dev/api/box -> 404 application/json;charset=utf-8
POST https://ascii.dev/api/box -> 404 application/json;charset=utf-8
GET https://box.ascii.dev/api/box -> 404 text/html; charset=utf-8
POST https://box.ascii.dev/api/box -> 404 text/html; charset=utf-8

What this proves: the harness/config/registration path still passes, the provider refuses to run without a key, and no secrets/spend were used.

What it does not prove: real ASCII Box create/upload/write/exec-stream/delete behavior. The public no-auth probes do not validate the PR's /api/box lifecycle contract; they currently return 404, so I do not think this satisfies the existing real-behavior proof requirement. A maintainer/provider owner needs to supply redacted live smoke output or confirm/fix the REST contract before this is ship-ready.

@zozo123
Copy link
Copy Markdown
Contributor Author

zozo123 commented May 26, 2026

PR #157 proof recheck

Rechecked head f197ce7363f568acabf74bc70bc79290299d90f6 in the isolated PR #157 worktree. No ASCII Box API key is present in the environment, so I did not run any live paid/external lifecycle action.

No-secret local proof still passes:

$ unset ASCII_BOX_API_KEY CRABBOX_ASCII_BOX_API_KEY
$ go test -v ./internal/providers/asciibox -run 'TestRunE2EAgainstHTTPAPI|TestClientUsesAsciiBoxRESTShape'
=== RUN   TestClientUsesAsciiBoxRESTShape
--- PASS: TestClientUsesAsciiBoxRESTShape (0.00s)
=== RUN   TestRunE2EAgainstHTTPAPI
--- PASS: TestRunE2EAgainstHTTPAPI (0.25s)
PASS
ok  github.com/openclaw/crabbox/internal/providers/asciibox  0.900s

$ go test -v ./internal/cli -run 'TestAsciiBoxConfigDefaultsFileAndEnv'
=== RUN   TestAsciiBoxConfigDefaultsFileAndEnv
--- PASS: TestAsciiBoxConfigDefaultsFileAndEnv (0.00s)
PASS
ok  github.com/openclaw/crabbox/internal/cli  0.741s

$ go test ./internal/cli ./internal/providers/asciibox ./internal/providers/all
ok  github.com/openclaw/crabbox/internal/cli  (cached)
ok  github.com/openclaw/crabbox/internal/providers/asciibox  (cached)
ok  github.com/openclaw/crabbox/internal/providers/all  (cached)

$ go run ./cmd/crabbox run --provider ascii-box -- true
provider=ascii-box requires ASCII_BOX_API_KEY
exit status 2

No-secret public probes:

GET https://ascii.dev/api/box/install -> 200 application/x-sh
GET https://ascii.dev/api/box -> 404 application/json;charset=utf-8
POST https://ascii.dev/api/box -> 404 application/json;charset=utf-8
GET https://box.ascii.dev/api/box -> 404 text/html; charset=utf-8
POST https://box.ascii.dev/api/box -> 404 text/html; charset=utf-8

This validates the local provider harness, REST-shape parser path, config/env loading, registration, and missing-key refusal. It still does not prove real ASCII Box create/upload/write/exec-stream/delete behavior. The remaining author/maintainer action is to provide redacted live smoke output with ASCII_BOX_API_KEY/CRABBOX_ASCII_BOX_API_KEY, or confirm/fix the REST contract before treating this as ship-ready.

@zozo123
Copy link
Copy Markdown
Contributor Author

zozo123 commented May 27, 2026

PR #157 ready update

Pushed dd71c8a to zozo123:feat/ascii-box-provider.

What changed:

  • Reworked provider=ascii-box from delegated REST execution to an SSH lease provider.
  • Replaced the guessed /api/box REST shape with the lifecycle routes used by the official Box CLI: POST/GET /api/box/boxes, GET/DELETE /api/box/boxes/<id>.
  • Removed the unverified REST exec/upload/write/exec-stream implementation and the unsupported image/size/keep-alive ASCII Box config surface.
  • Store the returned Box id in the local lease claim, then use standard Crabbox SSH sync/run/status/stop behavior.

Validation:

  • Local: go test ./...
  • Local: npm test
  • Local: npm run docs:check
  • Local: git diff --check
  • GitHub CI at dd71c8a: Go, Worker, Plugin, Docs, and Release Check all passed.

The previous blocker was valid: the old provider shipped guessed private execution endpoints. This update removes that surface and keeps the provider on lifecycle API + SSH only.

Add ASCII Box as a direct SSH-lease provider using the documented box CLI JSON surface.

Co-authored-by: Yossi Eliaz <zozo123@users.noreply.github.com>
@steipete steipete force-pushed the feat/ascii-box-provider branch from dd71c8a to f870823 Compare May 30, 2026 18:27
@steipete
Copy link
Copy Markdown
Contributor

@clawsweeper re-review

Updated this branch on current main with a CLI-based ASCII Box provider. The PR body has local proof plus redacted live doctor proof. Full create/SSH/run/delete E2E is still blocked by the current ASCII Box account returning billing_required on box new.

@clawsweeper
Copy link
Copy Markdown

clawsweeper Bot commented May 30, 2026

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. labels May 30, 2026
@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels May 30, 2026
@steipete steipete merged commit dd2a0bd into openclaw:main May 30, 2026
5 checks passed
@zozo123 zozo123 deleted the feat/ascii-box-provider branch May 30, 2026 21:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. P2 Normal priority bug or improvement with limited blast radius. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. 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.

2 participants