Repository navigation
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
|
Codex review: needs real behavior proof before merge. What this changesThis 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.
Review scores
ProductKind: Feature · Worth it: Yes · Fix scope: Complete 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 Before merge
Findings
Tests
Agent review detailsHow this fits togetherKubernetes 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]
Technical reviewBest 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:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against 98715ebc6665. Merge-risk optionsMaintainer options:
Provenance checked
TestingProof path: in-process harness. SecurityNeeds attention: Native enrollment introduces an authority chain whose final-effect rejection needs captured evidence. EvidenceSecurity concerns:
What I checked:
Review metrics
LabelsLabel changes: No label changes. Label justifications:
Rating scale6/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. WorkflowClawSweeper edits this one comment on every review. Comment HistoryReview history (6 earlier review cycles)
Reviewed October 10, 2026, 10:39 AM ET / 14:39 UTC (Revision 7). |
8d37f2c to
41f24ef
Compare
|
Live qualification on the running Podman-backed k3d demo is complete for the native enrollment boundary:
Executed from @clawsweeper re-review |
|
🦞👀 Re-review progress:
|
Signed-off-by: sallyom <somalley@redhat.com>
Signed-off-by: sallyom <somalley@redhat.com>
41f24ef to
46ddad9
Compare
|
Rebased onto current 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 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 |
|
🦞👀 Re-review progress:
|
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
oce-openclaw-runtimeprovider. Reuse the existing exact ownership, version-fenced setup renewal, and Sandbox-before-provider cleanup rules.This adds native OpenClaw Harness support only. The temporary preparer requires
userNamespaces: falseand the pinned OpenShell identity defaults (namespace-derived UID/GID or10001: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.local-pathPVC with three kubelet-created, root-owned subpaths. Each changed from UID0, mode2777to UID10001, mode0700; 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.github-sluggerand 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
f3d1bc091: native Harness provider delivery, outbound enrollment, and runtime-test/log fixes.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/nodein the model credential profile's allowed binaries. No production qualification or automatic enablement is claimed.