refactor(agents): extract OpenClaw from BenchFlow core - #1093
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Thermo-nuclear review of exact head c3e770f. The central ownership change is a net simplification: the 973-line OpenClaw implementation and its feature-specific registry policy leave core, while generic manifest selection/loading remains in focused modules. I found and repaired one cross-feature regression: the new fail-closed launch boundary also tried to resolve the non-agent task-runtime as an ACP launch during default setup. Setup now models agentless modes once and skips both web policy and launch lookup; a regression test exercises real default rollout planes. Validation on the exact tree: 5,911 tests passed with 87 skipped and 7 deselected; ty and Ruff passed; the paired agents catalog gate passed 28 tests; local list/show resolved OpenClaw from the external manifest. The supplied DeepSeek key was first validated with a direct HTTP 200 call. A complete Docker hello-world run then installed the manifest-owned OpenClaw shim, produced two proxied HTTP 200 model exchanges with provider-reported usage, emitted one completed write tool call, scored reward 1, and recorded no agent or verifier error. Merge gate: the behavior and structure are acceptable, but agents #71 and #72 must land first, this PR must be refreshed against their immutable merged source, exact-head CI must finish green, and independent human approval is still required before squash merge. |
There was a problem hiding this comment.
Devin Review found 2 potential issues.
2 flags not posted on this PR by your GitHub settings — view them in Devin Review. (Configure)
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c3e770fa05
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Exact-head follow-up for f7de9af: the local-catalog override now activates before a directly registered built-in can return, and expected remote/core parity overlaps no longer become permanent warnings. I reran the real default resolution/task-runtime focused gate: 21 passed with 3 skipped; ty and Ruff pass, and every exact-head GitHub test, parity, rollout-smoke, and fixture-scenario check is green. The separate reader/writer atomicity limitation is pre-existing on main and remains tracked by #1089; it should not be misrepresented as solved here. Merge still depends on agents #71, #72, and the new streaming fix #73 landing first, followed by immutable-source parity, exact-final-head real E2E, and independent human approval. |
|
Exact-head follow-up for 24c5074. The new generic discovery path correctly accepts an advertised ACP A real Docker run then paired this head with agents #73 at 279aa61538a3d494bb6dfc1affa40c5696c001c7: BenchFlow discovered OpenClaw effort dynamically, delivered No new merge-blocking defect found. Merge still requires agents #71, #72, and #73 to land first so their immutable sources can be pinned, followed by final parity/CI and independent human approval. |
Closes #1090: #1090
Summary
Agents PR #72 must merge first.
Validation
main.