Skip to content

[codex] Add Coder SSH-lease provider#271

Open
coygeek wants to merge 10 commits into
openclaw:mainfrom
coygeek:feat/coder-provider
Open

[codex] Add Coder SSH-lease provider#271
coygeek wants to merge 10 commits into
openclaw:mainfrom
coygeek:feat/coder-provider

Conversation

@coygeek

@coygeek coygeek commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Summary

Closes #265.

This PR adds the Coder provider as a direct Linux SSH-lease provider for Crabbox. It uses the Coder CLI for workspace lifecycle and SSH transport while keeping Crabbox responsible for sync, command execution, result collection, local claim tracking, and release behavior.

What changed

  • Add the coder provider implementation, registration, configuration, flags, docs, and provider list wiring.
  • Create Coder workspaces with lease-unique Coder-safe names while preserving friendly Crabbox slugs for local lookup.
  • Resolve leases by lease ID, local slug, Coder workspace name, and unique owner/workspace inventory matches.
  • Use Coder's OpenSSH proxy path through coder ssh --stdio and isolate Coder SSH known_hosts state under Crabbox's Coder config directory.
  • Persist Coder release intent in local claims so cleanup honors the original stop/delete behavior.
  • Restrict Coder cleanup mutations to locally claimed workspaces, avoiding accidental mutation of unrelated or reusable Coder environments.
  • Add rollback behavior for failed non-kept acquisitions and documentation for lifecycle, cleanup, and SSH behavior.

Validation

  • git diff --check origin/main...HEAD
  • go test ./internal/providers/coder -count=1
  • go test ./internal/providers/coder ./internal/providers/all ./internal/cli
  • go test ./...
  • autoreview --mode branch --base origin/main --engine codex --model gpt-5.5 --thinking medium --parallel-tests "go test ./..."

The Codex autoreview pass reported no accepted/actionable findings.

Notes

Live Coder workspace smoke testing still requires an authenticated Coder deployment via coder login <url>.

coygeek and others added 10 commits June 11, 2026 17:22
Add a direct Coder provider that leases workspaces through the local Coder CLI and exposes them as proxy-backed SSH leases for Crabbox commands. Keep Coder auth in the native CLI store while making doctor, run, ssh, stop, and cleanup work with conservative stop-first lifecycle defaults.

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
Require explicit Crabbox markers or legacy Crabbox labels before generic lease metadata is trusted, while keeping prefix-based ownership and legacy label resolution working. This prevents cleanup or resolve from acting on unrelated Coder workspaces after the provider branch is merged.

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
Check claim freshness before stopped-state cleanup so opted-in delete cleanup cannot remove still-claimed Coder workspaces. Add a regression test for stopped active claims to keep resolve-on-demand leases reusable.

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
Use the configured stop-vs-delete release policy for post-create rollback paths, but only after verifying the workspace exists when coder create itself fails. This keeps disposable Coder workspaces cleaned up while preserving the original create error for failures that never produced a workspace.

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
Hash overlong Coder workspace names before truncation and fall back to a lease-hash slug suffix when an existing workspace already occupies the derived name. This keeps long requested slugs stable enough for humans while avoiding deterministic create collisions.

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
Decide between coder list and coder list --all from the original request or stored claim reference before resolving and keep that same scope for post-start refreshes. This lets lease-id and slug based commands keep working when a claim points at an owner-qualified workspace.

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
Attach the proxy SSH target to ready status-only leases and preserve accumulated doctor checks when inventory listing fails. This keeps status JSON honest for ready workspaces and retains inventory diagnostics in failing doctor output.

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
Propagate Coder work-root defaults through normal config loading, stamp resolved Coder servers with that work root, and use unique slash-free SSH host aliases for owner-qualified workspaces while keeping the full ref in the proxy command.

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
Reuse persisted keep metadata from local claims when resolving, listing, and cleaning Coder workspaces so explicit keep requests survive later status and run flows without making ordinary claims immortal.

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
Persist Coder release intent in local claims, make workspace names lease-unique, and restrict cleanup to claimed workspaces so Crabbox does not mutate unrelated Coder environments.

Also preserve owner-qualified workspace resolution, add safer acquisition rollback, isolate Coder known_hosts state, and include the delete-on-release flag in generated stop commands.

Refs: openclaw#265
@coygeek coygeek marked this pull request as ready for review June 12, 2026 00:26
@clawsweeper

clawsweeper Bot commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed June 11, 2026, 9:01 PM ET / 01:01 UTC.

Summary
Adds a direct Linux Coder SSH-lease provider with CLI-backed lifecycle, configuration, documentation, provider registration, and tests.

Reproducibility: yes. for the review defect: source inspection establishes that every post-create rollback invokes coder delete regardless of coder.deleteOnRelease. The full provider workflow itself has not been reproduced against a live Coder deployment.

Review metrics: 3 noteworthy metrics.

  • Change surface: 24 files, +3781/-5. The PR introduces a substantial new remote lifecycle and SSH integration surface.
  • Provider tests: 1,578 lines added. Coverage is extensive, but it is mock-based and currently asserts the destructive default rollback behavior.
  • Live smoke scenarios: 0 demonstrated. The PR states that authenticated workspace testing remains outstanding.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🦪 silver shellfish
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:

  • Make failed-acquisition rollback stop by default and delete only by explicit opt-in.
  • [P1] Add redacted authenticated terminal proof covering doctor, warmup, run, SSH, and stop.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR provides tests and automated review but explicitly lacks authenticated Coder smoke testing; add redacted terminal or live output, update the PR body to trigger review, or ask a maintainer to comment @clawsweeper re-review.

Risk before merge

  • [P1] A routine post-create acquisition failure can permanently delete the Coder workspace under default configuration instead of stopping it for inspection or reuse.
  • [P1] The mocked test suite does not establish that Coder CLI JSON parsing, proxy SSH, workspace readiness, and stop behavior work together against an authenticated deployment.

Maintainer options:

  1. Preserve stop-first rollback (recommended)
    Change failed-acquire rollback to stop by default and delete only when delete-on-release is explicitly selected, with focused tests for both modes.
  2. Approve destructive rollback explicitly
    Maintainers could intentionally retain unconditional deletion, but the provider documentation and command messaging would need to state this destructive acquisition-failure exception clearly.
  3. Pause pending live validation
    Hold the integration until an authenticated Coder deployment proves the complete lifecycle and clarifies whether deletion on failed acquisition is acceptable.

Next step before merge

  • [P1] The contributor should repair the rollback behavior and supply authenticated runtime proof; automation cannot produce evidence from the contributor's Coder deployment.

Security
Cleared: The branch keeps Coder credentials in the native CLI store, adds no token arguments or persisted secrets, and introduces no separate concrete supply-chain or security-boundary concern.

Review findings

  • [P1] Honor the configured release action during rollback — internal/providers/coder/backend.go:134
Review details

Best possible solution:

Route every post-create rollback through the configured or persisted release action, test default stop and opt-in deletion, then demonstrate doctor, warmup, run, SSH, and stop against an authenticated Coder workspace with redacted terminal output.

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

Yes for the review defect: source inspection establishes that every post-create rollback invokes coder delete regardless of coder.deleteOnRelease. The full provider workflow itself has not been reproduced against a live Coder deployment.

Is this the best way to solve the issue?

No. The narrow maintainable solution is to reuse the existing configured release action during rollback rather than introduce an undocumented destructive exception.

Full review comments:

  • [P1] Honor the configured release action during rollback — internal/providers/coder/backend.go:134
    After coder create succeeds, inventory and SSH-readiness failures call this helper, which always runs coder delete even when coder.deleteOnRelease is false. That contradicts the provider's stop-first lifecycle and can destroy a newly provisioned workspace after a transient failure; route rollback through the configured or persisted release action and test both modes.
    Confidence: 0.99

Overall correctness: patch is incorrect
Overall confidence: 0.98

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

  • add P2: The new provider has a bounded but potentially destructive lifecycle defect and lacks authenticated runtime proof.
  • add merge-risk: 🚨 compatibility: The branch promises a stop-first lifecycle but deletes newly created workspaces during failed acquisition under the default setting.
  • add rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦪 silver shellfish.
  • 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 provides tests and automated review but explicitly lacks authenticated Coder smoke testing; add redacted terminal or live output, update the PR body to trigger review, or ask a maintainer to comment @clawsweeper re-review.

Label justifications:

  • P2: The new provider has a bounded but potentially destructive lifecycle defect and lacks authenticated runtime proof.
  • merge-risk: 🚨 compatibility: The branch promises a stop-first lifecycle but deletes newly created workspaces during failed acquisition under the default setting.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦪 silver shellfish.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR provides tests and automated review but explicitly lacks authenticated Coder smoke testing; add redacted terminal or live output, update the PR body to trigger review, or ask a maintainer to comment @clawsweeper re-review.
Evidence reviewed

What I checked:

  • Destructive rollback path: After workspace creation, inventory or SSH-readiness failures enter a rollback helper that always invokes client.delete, bypassing the configured stop-versus-delete release policy. (internal/providers/coder/backend.go:134, f7dd30ab7b45)
  • Regression test encodes deletion: The acquisition rollback test expects delete --yes under the default configuration, confirming the destructive behavior is encoded by the branch rather than being an ambiguous source reading. (internal/providers/coder/backend_test.go:290, f7dd30ab7b45)
  • Normal release honors configuration: The ordinary release helper stops unless DeleteOnRelease is enabled, demonstrating the policy that failed-acquisition rollback should reuse. (internal/providers/coder/backend.go:141, f7dd30ab7b45)
  • Real behavior proof absent: The PR body explicitly states that authenticated live Coder workspace smoke testing still needs to be performed after coder login. (f7dd30ab7b45)
  • Current-main provider history: Recent merged provider integrations and adjacent config fields trace to Coy Geek, while the base configuration and latest release baseline include substantial work by Vincent Koc. (internal/cli/config.go:117, 836130bbcb03)

Likely related people:

  • coygeek: Prior merged current-main history includes several recent provider integrations and adjacent configuration work, independently of proposing this PR. (role: recent area contributor; confidence: high; commits: 49cac18f6527, e5d8c3e3d565, aff04c9f19a3; files: internal/providers, internal/cli/config.go)
  • Vincent Koc: Current base configuration lines and the latest release baseline trace to this contributor, with substantial history across the sampled provider and configuration paths. (role: feature-history owner; confidence: medium; commits: e9fa596eac8d; files: internal/cli/config.go, internal/providers)
  • Peter Steinberger: Git shortlog shows the largest long-term contribution history across the sampled provider and central configuration paths, though the routing signal is broad. (role: feature-history owner; confidence: medium; files: internal/providers/namespace, internal/providers/tenki, internal/providers/daytona)
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 12, 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.

Feature request: add Coder as a Crabbox SSH-lease provider

1 participant