Skip to content

feat(openshell): deliver native OpenClaw Harness setup through providers - #1982

Draft
sallyom wants to merge 2 commits into
mainfrom
feat/openshell-native-harness
Draft

sallyom wants to merge 2 commits into
mainfrom
feat/openshell-native-harness

Conversation

@sallyom

@sallyom sallyom commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Problem

Native OpenClaw Harness provisioning through OpenShell still depended on an integration-only bootstrap Job and PVC credential-copy bridge. The regular Driver did not supply the node setup envelope or public Gateway CA to the native worker, and its temporary directory could be owned by another Sandbox identity.

Change

  • Deliver native node setup and the public CA through a revision-owned oce-openclaw-runtime provider. Reuse the existing exact ownership, version-fenced setup renewal, and Sandbox-before-provider cleanup rules.
  • Read provider files in the native worker startup program and use a process-owned temporary directory under the revision runtime home. Native Harnesses request no inbound service exposure.
  • Run the native integration through unmodified Compute requirements and the production Driver; remove its test-only Job/PVC credential bridge and copied workload-token checks.
  • Keep native enrollment outbound-only: preserve supervisor/Gateway enrollment policies without requiring Codex's inbound provider route.
  • Temporary, separate commit: automatically prepare native writable PVC subpaths using a revision-owned, non-root Deployment. Reuse existing Deployment permissions; add no direct Pod permissions, Linux capabilities, service-account token, or credential copies. Verify ownership/mode and wait for foreground deletion before Sandbox creation. Remove this workaround if OpenShell adds support for preparing private subpaths and OCE adopts that support.
  • Update the Driver reference, provisioning flow, and testing guide. The reference and flow remain single coherent contract/lifecycle pages below the 2,500-word limit.

This adds native OpenClaw Harness support only. The temporary preparer requires userNamespaces: false and the pinned OpenShell identity defaults (namespace-derived UID/GID or 10001:10001, with gateway overrides unset). Codex retains its existing provider path. No OpenShift deployment examples, image pins, Codex listener workarounds, or default native-image qualification gates change.

Verification

Runtime code tested: 17508b0fba9c49742250b5136021f644b7082877; latest test-only fix: 5a64737184e3b66b1c24fe195e7e87b09681ee39, macOS, Node.js 24.18.0; Podman-backed k3d/Kubernetes for the filesystem proof.

  • OpenShell startup integration: 47 passed, 0 failed/skipped. These cases use controlled Kubernetes/provider transport except their real loopback gRPC checks; they do not establish live model execution.
  • Focused Kubernetes Compute conformance: 5 passed, covering native enrollment without an inbound route, unchanged Codex routing, and policy cleanup. The new route regression fails on the previous implementation with the reported exact-provider-route error.
  • Preparation denial regression fails before the temporary fix and passes with it: a denied preparer cannot start a native Sandbox.
  • Harness-specific privilege-check regression passes: native workers have no Codex verifier, Codex still requires its verifier, and neither may expose a raw app-server token. The previous helper failed this case. The latest full k3d trial reached provider-Harness readiness before this test-only mismatch stopped it; a complete rerun is pending.
  • Live filesystem stage: ran the generated production preparation Deployment against a disposable local-path PVC with three kubelet-created, root-owned subpaths. Each changed from UID 0, mode 2777 to UID 10001, mode 0700; all seeded owner edits remained intact. Foreground deletion left no preparation Pods before the next stage. The probe namespace was removed. No model credentials were used.
  • Full ESLint, TypeScript build, changed-file Prettier checks, workspace isolation, provisioning-flow validation, Markdown length limits, and diff whitespace checks passed. Existing dependencies were reused; no dependencies were installed. The reference and flow remain coherent pages under the 2,500-word limit.
  • The user reported the complete native k3d integration passed before the subsequent demo-log checks were added. Live authenticated demo API checks then returned HTTP 200 and ten sanitized records for both Agent and Gateway logs after adding the missing namespace-scoped read rules. OpenShell policy-log forwarding and its new integration assertions need the next demo/test run; no image rebuild is required for these test-setup changes.
  • Full docs build/link/spec checks remain blocked locally by missing isolated docs dependencies (github-slugger and site assets); those checks must pass in CI.

Pending before merge: run the complete native provider workflow on this final head, including enrollment, workspace access, successive model turns, cleanup, foreign-Agent credential rejection, expired setup, and revoked identity at the final I/O boundary. The live PVC-stage proof above does not qualify that authority chain or OpenShift admission. Resolve the existing main conflict and obtain review/CI clearance. This PR remains a draft.

Commits

  1. f3d1bc091: native Harness provider delivery, outbound enrollment, and runtime-test/log fixes.
  2. 41f24efd5: separately removable, temporary automatic PVC preparation. Its removal is conditional on OpenShell adding private-subpath preparation and OCE adopting it; no upstream option name or acceptance is assumed.

Both commits carry DCO signoff. The squash preserves the exact source tree of the previously tested branch.

Risks

The expiring node-setup token remains visible inside the Sandbox through its provider file, matching the existing development-only Codex delivery boundary. Model keys remain in the Credential Gateway. Projected workload identity stays unsupported and must be disabled; native inference requires an explicitly selected custom image with worker support and /usr/local/bin/node in the model credential profile's allowed binaries. No production qualification or automatic enablement is claimed.

@clawsweeper

clawsweeper Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@clawsweeper clawsweeper Bot added P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. 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 Oct 9, 2026
@clawsweeper

clawsweeper Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge.

What this changes

This PR delivers native OpenClaw setup through OpenShell providers and prepares private PVC directories before Sandbox creation.

Example: A dedicated native OpenClaw Harness enrolls through OpenShell.

  • Before: The integration copies setup credentials through a test-only bootstrap Job and PVC bridge.
  • After: The production Driver supplies node-setup.json and node-ca.pem through oce-openclaw-runtime, without an inbound Harness service.

Review scores

Measure Result What it means
Overall readiness 🧂 unranked krab (1/6) Useful platform integration remains blocked by a concrete lifecycle defect and unverified live authority and storage results.
Proof confidence 🧂 unranked krab (1/6) Real behavior proof is necessary before merge. See Before merge.
Patch quality 🦪 silver shellfish (2/6) Security review found an item that needs attention.

Product

Kind: Feature · Worth it: Yes · Fix scope: Complete
User problem: Native OpenClaw provisioning through OpenShell depends on test-only credential delivery instead of the regular Driver workflow.
Reason: The maintainer-authored direction extends an established native Harness capability through its existing platform owner and removes a competing test-only path.

Merge readiness

⛔ Blocked before merge - 6 items remain

Keep this PR open: it delivers useful native Harness integration, but the preparation lifecycle defect and misleading flow diagram remain unresolved.

Priority: P2
Reviewed head: 46ddad96a28249ae8e2b82375281da37a72b6a3b

Before merge

  • Add real behavior proof - Authority-chain proof required: the author's live qualification comment reports the relevant Podman-backed k3d API scenarios, including positive controls, foreign-Agent rejection, genuine expiry, and revoked-worker failures, but provides no captured terminal output, logs, or linked artifact. Attach the existing redacted trace tying the changed provider/native consumer to rejection before workspace or model I/O; redact IPs, API keys, phone numbers, and non-public endpoints. Captured preservation and interrupted-recovery evidence is also needed for the preparer's persisted-data rewrite. 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.
  • Retire preparation Deployments during revision cleanup (P2) - If the controller exits after this create succeeds, or the create succeeds remotely but its response is lost, the preparation Deployment survives with no owner reference. Its program remains running after readiness. OpenShell revision cleanup deletes only the Sandbox and runtime provider, and Compute delegates this cleanup while waiting only for agent-role Pods. Retiring the failed revision therefore leaves the preparer and PVC mount alive, potentially consuming quota or blocking a replacement on single-Pod storage. Add ownership-checked, replay-safe preparer deletion and wait for its Pods during revision cleanup, including when no Sandbox was created. This finding remains unresolved from the previous review.
  • Give the native Harness decision a distinct diagram ID (P3) - The new decision uses P, but the existing teardown edge also defines P as Delete Namespace. Mermaid treats both as one node, so the rendered lifecycle joins provisioning to namespace deletion and gives teardown the native yes/no branches. Rename the inserted decision and its edges to a unique ID. This finding remains unresolved from the previous review.
  • Resolve security concern: Demonstrate invalidation before native workspace and model effects - Exact Secret ownership, bounded setup validation, provider ownership, and version-fenced renewal are present in source; these checks alone do not establish downstream rejection after enrollment creates persisted device authority.
  • Resolve merge risk - Provider-delivered setup becomes persisted native device authority; rejection of foreign, expired, and revoked authority before final workspace or model I/O remains unverified.
  • Add data-model compatibility proof - The review found that existing stored data may not work after upgrade. Show that existing data still loads and works with this change.

Findings

  • [P2] Retire preparation Deployments during revision cleanup — apps/controller/src/drivers/sandbox/openshell.ts:1320-1321
  • [P3] Give the native Harness decision a distinct diagram ID — docs/flows/openshell-sandbox-provisioning.md:57-60
  • [medium] Demonstrate invalidation before native workspace and model effects — apps/controller/src/drivers/compute/kubernetes/runtime-entrypoints.ts:3975

Tests

  • Missing end-to-end proof: Attach captured native provider workflow results covering enrollment, workspace access, successive model turns, cleanup, forbidden authority, and retained-data recovery; reported focused regressions are not independently executed in this read-only review.
Agent review details

How this fits together

Kubernetes Compute supplies admitted revision requirements to the OpenShell SandboxDriver, which prepares storage, creates revision-owned providers and Sandboxes, and enables outbound enrollment into the Agent Gateway.

flowchart TB
  A[Admitted Agent revision] --> B[Kubernetes Compute]
  B --> C[OpenShell SandboxDriver]
  C --> D[Temporary PVC preparation]
  C --> E[Revision runtime provider]
  D --> F[Native Harness Sandbox]
  E --> F
  F --> G[Agent Gateway]
Loading

Technical review

Best possible solution:

Extend the existing provider path with replay-safe preparation cleanup and retain outbound-only native enrollment within the documented development boundary.

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

Current-main source creates runtime providers only for Codex, while native startup expects inline setup; the introduced diff supplies the missing native delivery path without executing target code during review.

Is this the best way to solve the issue?

Extending the existing revision-owned provider mechanism and removing the credential bridge fits the established ownership model, provided the temporary preparer participates in retirement and recovery.

Full review comments:

  • [P2] Retire preparation Deployments during revision cleanup — apps/controller/src/drivers/sandbox/openshell.ts:1320-1321
    If the controller exits after this create succeeds, or the create succeeds remotely but its response is lost, the preparation Deployment survives with no owner reference. Its program remains running after readiness. OpenShell revision cleanup deletes only the Sandbox and runtime provider, and Compute delegates this cleanup while waiting only for agent-role Pods. Retiring the failed revision therefore leaves the preparer and PVC mount alive, potentially consuming quota or blocking a replacement on single-Pod storage. Add ownership-checked, replay-safe preparer deletion and wait for its Pods during revision cleanup, including when no Sandbox was created. This finding remains unresolved from the previous review.
    Confidence: 0.99
  • [P3] Give the native Harness decision a distinct diagram ID — docs/flows/openshell-sandbox-provisioning.md:57-60
    The new decision uses P, but the existing teardown edge also defines P as Delete Namespace. Mermaid treats both as one node, so the rendered lifecycle joins provisioning to namespace deletion and gives teardown the native yes/no branches. Rename the inserted decision and its edges to a unique ID. This finding remains unresolved from the previous review.
    Confidence: 1

Overall correctness: patch is incorrect
Overall confidence: 0.98

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 98715ebc6665.

Merge-risk options

Maintainer options:

  1. Establish the native authority boundary (recommended)
    Attach the existing redacted production-path trace covering allowed enrollment and rejection of foreign-Agent, expired setup, and revoked device authority before workspace or model I/O.
  2. Keep native delivery deferred
    Retain the draft until the native provider authority boundary can be demonstrated.

Provenance checked

Testing

Proof path: in-process harness.

Security

Needs attention: Native enrollment introduces an authority chain whose final-effect rejection needs captured evidence.

Evidence

Security concerns:

  • [medium] Demonstrate invalidation before native workspace and model effects — apps/controller/src/drivers/compute/kubernetes/runtime-entrypoints.ts:3975
    Exact Secret ownership, bounded setup validation, provider ownership, and version-fenced renewal are present in source; these checks alone do not establish downstream rejection after enrollment creates persisted device authority.
    Confidence: 0.95

What I checked:

Review metrics

Metric Value Why it matters
Production and test scope Production +399/-84 lines; tests and helpers +354/-860 lines. The production increase delivers the provider path and temporary preparer while removing substantial test-only credential delivery.

Labels

Label changes:

No label changes.

Label justifications:

  • P2: This improves an explicitly selected experimental native Harness path with limited deployment scope.
  • merge-risk: 🚨 availability: An orphan preparation Deployment can retain a PVC mount and interfere with later revision startup.
  • merge-risk: 🚨 security-boundary: The change delivers enrollment authority into the native Sandbox and persists resulting device identity.
  • merge-risk: 🚨 compatibility: The public SandboxDriver endpoint contract now permits undefined, and the preparer rewrites existing PVC directory ownership.
  • 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.

Rating scale

6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior.

Workflow

ClawSweeper edits this one comment on every review. Comment @clawsweeper re-review for a fresh review only; repair and merge need explicit maintainer commands such as @clawsweeper autofix or @clawsweeper automerge.

History

Review history (6 earlier review cycles)
  • reviewed 2026-10-09T20:51:06.470Z sha 4926e24 :: needs real behavior proof before merge. :: [P2] Keep native workers out of Codex-only endpoint resolution
  • reviewed 2026-10-09T22:09:40.170Z sha aeb7d91 :: needs real behavior proof before merge. :: none
  • reviewed 2026-10-10T12:54:33.512Z sha 17508b0 :: needs real behavior proof before merge. :: [P2] [P2] Retire preparation Deployments during revision cleanup | [P3] [P3] Give the native Harness decision a distinct diagram ID
  • reviewed 2026-10-10T13:22:04.571Z sha 5a64737 :: needs real behavior proof before merge. :: [P2] Retire preparation Deployments during revision cleanup | [P3] Give the native Harness decision a distinct diagram ID
  • reviewed 2026-10-10T14:02:38.508Z sha 41f24ef :: needs real behavior proof before merge. :: [P2] Retire preparation Deployments during revision cleanup | [P3] Give the native Harness decision a distinct diagram ID
  • reviewed 2026-10-10T14:32:28.408Z sha 41f24ef :: needs real behavior proof before merge. :: [P2] Retire preparation Deployments during revision cleanup | [P3] Give the native Harness decision a distinct diagram ID

Reviewed October 10, 2026, 10:39 AM ET / 14:39 UTC (Revision 7).

@clawsweeper clawsweeper Bot added the merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. label Oct 10, 2026
@sallyom
sallyom force-pushed the feat/openshell-native-harness branch from 8d37f2c to 41f24ef Compare October 10, 2026 13:56
@sallyom

sallyom commented Oct 10, 2026

Copy link
Copy Markdown
Contributor Author

Live qualification on the running Podman-backed k3d demo is complete for the native enrollment boundary:

  • A separate native Agent created through the supported OCC API passed workspace write/read and two genuine model turns (HTTP 200; unique response nonces matched).
  • Agent B rejected Agent A's setup credential (AUTH_BOOTSTRAP_TOKEN_INVALID) and signed device token (AUTH_DEVICE_TOKEN_MISMATCH). Positive controls accepted each Gateway's own enrollment credential.
  • An unused, genuinely issued setup credential was rejected after its real 10-minute expiry; no clock/database manipulation.
  • Revocation disconnected a sacrificial node and rejected its persisted token on reconnect. Revoking the test Agent's actual native worker also blocked its previously working workspace read (503 DEPENDENCY_UNAVAILABLE) and model turn (HTTP 500).
  • Real local-path PVC directories went from root-owned/2777 to UID 10001/0700, preserving files. Remount and interrupted directory-replacement recovery passed; preparation workloads were removed.
  • Console API log reads returned HTTP 200 and 10 actual lines each for Agent, Gateway, and Sandbox.

Executed from openshift-poc at 3360c74d5; the native consumer, OpenShell SandboxDriver, and affected integration helper/test files match PR head 41f24efd5 exactly. This is manual live proof, not a claim that the entire PR-head suite or every storage class passed. The reported orphan-preparer lifecycle and Mermaid-ID findings remain separate, unresolved review items.

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

🦞👀
Exact review queued.

Re-review progress:

@clawsweeper clawsweeper Bot added the merge-risk: 🚨 availability 🚨 Merging this PR could cause crashes, hangs, restart loops, stalls, or process outages. label Oct 10, 2026
Signed-off-by: sallyom <somalley@redhat.com>
@sallyom
sallyom force-pushed the feat/openshell-native-harness branch from 41f24ef to 46ddad9 Compare October 10, 2026 14:35
@sallyom
sallyom deployed to integration-qa-pr October 10, 2026 14:35 — with GitHub Actions Active
@sallyom
sallyom deployed to integration-qa-pr October 10, 2026 14:35 — with GitHub Actions Active
@sallyom

sallyom commented Oct 10, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto current origin/main (279cf04fa), resolving the integration-setup and flow-doc conflicts while preserving main's Codex OAuth/CA coverage and Sandbox HOME support. GitHub now reports MERGEABLE. The PR still has exactly two DCO-signed commits, with the temporary PVC workaround separate; new head: 46ddad96a.

Post-rebase checks passed: startup integration 48/48, focused Compute conformance 5/5, OpenShell helper integration 10/10, TypeScript build, full ESLint, changed-file formatting, workspace boundary, flow validation, and documentation length check. The flow remains one complete lifecycle document within the 2,500-word limit. Full docs build/link checks are blocked locally by missing installed github-slugger; no dependencies were installed.

The earlier live qualification comment records pre-rebase evidence; I have not rerun the full credentialed k3d lane on this new head. Existing lifecycle/diagram findings remain open.

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

🦞👀
Exact review queued.

Re-review progress:

This branch was successfully deployed

1 active deployment
integration-qa-pr — 46ddad96 Deployed Oct 10, 2026 by sallyom via QA advisory (kubernetes) #110
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 availability 🚨 Merging this PR could cause crashes, hangs, restart loops, stalls, or process outages. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. 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.

1 participant