[chaodu-agent]

What problem does this solve?

The runtime half of the Agent Control Plane (#1528) needs the CP wire types (openab_cp::proto) inside openab-core, but today openab-cp always pulls in axum, tokio and the whole WebSocket server. This PR is part 1 of 3 re-landing #1528 (auto-closed by the stale-PR bot on 2026-09-13, not rejected), split so each piece can be reviewed in one sitting. All three are needed by openabdev/openab-pty#77 (Phase 0: openab-pty hosts as CP workers, Connect as primary).

Refs #1528, openabdev/openab-pty#77

Discord Discussion URL: https://discord.com/channels/1491295327620169908/1491365158868619404/1532377620241449040

Review Contract

Goal

Make openab_cp::proto usable with default-features = false, without axum/tokio/the server, while keeping the default build of openab-cp (library + openab-cp binary) unchanged. The runtime frame floor (MIN_RUNTIME_FRAME_BYTES, with its max_frame_bytes check) is not in this PR; it lands in PR B with the client that uses it.

Non-goals

• No runtime client, worker executor, headless mode or [control_plane] config: that is PR B.
• No primary state, local UDS API or openab agent CLI: that is PR C.
• No MCP direct tools or auto-facade: optional PR D.
• No change to the CP protocol, registry, router or policy behaviour. No other crate depends on openab-cp yet.

Accepted Residual Risks

• CI now builds, lints and tests openab-cp --no-default-features and checks its normal dependency graph (see ci.yml under Proposed Solution). No dependent crate uses the real default-features = false edge until PR B, and PR B must add a proto-only check of that consumer.
• CpConfig::validate() has no frame floor in this PR, so a max_frame_bytes below 1 MiB (including 0) still starts. That is today's behaviour; the floor and its constant land in PR B. See "Plan change" below.

Acceptance Criteria

• cargo test -p openab-cp passes (unit tests + ws_lifecycle)
• The CI / check step "openab-cp (proto only, no default features)" passes (the test build still compiles the tokio-tungstenite dev-dependency; the leak check covers the normal graph only): clippy -D warnings and cargo test with --no-default-features, and cargo tree -e normal lists none of the ten server dependencies (read from Cargo.toml)
• cargo clippy -p openab-cp --all-targets -- -D warnings is clean, with and without default features
• Cargo.lock is unchanged (--locked builds pass)
• The required CI / check job is green

Follow-ups

• PR B feat(cp): runtime CP client and worker delegation serving (headless worker): [control_plane] config, client (register/heartbeat/reconnect, bounded writes), worker executor (PromptRunner), PromptExecution seam, drop_idle_gate rebased onto fix(pool): keep the [[ws:]] workspace across hung eviction #1557, headless worker mode. Fixes feat(cp): land Agent Control Plane v1 on main #1528 round-6 F3 (client tick clamp), F10, F11 (Serve comment) and drops the round-1 ride-along edits.
• Follow-up for PR B: proto-only CI. PR B's CI must build, clippy (-D warnings) and test openab-cp with --no-default-features, plus the consumer crate on its default-features = false edge. PR B must not count the --workspace jobs as coverage: Cargo feature unification keeps server on whenever the openab-cp binary is in the build. Requested in review round 2 F1 (comment); this PR's ci.yml step covers openab-cp alone.
• Follow-up for PR B: frame floor. PR B adds MIN_RUNTIME_FRAME_BYTES = 1 MiB in proto.rs and the max_frame_bytes >= MIN_RUNTIME_FRAME_BYTES check in CpConfig::validate() with its client. Its release notes must call the floor a breaking config change, since existing configs below 1 MiB stop starting (review round 3 F2 option b, comment).
• PR C feat(cp): primary delegation state, owner-only local API and openab agent CLI: primary.rs, local.rs, the primary command arm, openab agent spawn|list|status|cancel. Fixes round-6 F1 (the red CI test), F3 (independent prune timer), F4, F5, F6, F7 (including CP-side empty-selector rejection), F9, F11 and F14.
• PR D (optional): session-gated MCP tools. Fixes round-6 F2 and F8.

At a Glance

openab-cp crate
├── proto            (always)          ← runtime clients: default-features = false (PR B)
└── feature "server" (default)
    ├── config / registry / policy / router / events / server
    └── [[bin]] openab-cp  (required-features = ["server"])

Prior Art & Industry Research

Not applicable: this is a Cargo feature split of an existing internal crate with no runtime, agent or scheduling behaviour change. The pattern (wire types always on, server behind a default feature) is the standard Cargo approach for a crate that is both a library contract and a binary.

OpenClaw: Not applicable (no equivalent crate boundary).

Hermes Agent: Not applicable (no equivalent crate boundary).

Proposed Solution

• crates/openab-cp/Cargo.toml: serde, serde_json and chrono stay unconditional. tokio, axum, futures-util, toml, tracing, tracing-subscriber, anyhow, parking_lot, clap and subtle become optional behind server (default). [[bin]] openab-cp and [[test]] ws_lifecycle require server.
• src/lib.rs: proto is always compiled. The other modules are #[cfg(feature = "server")]. A doc block explains the split.
• src/proto.rs: the wire types are unchanged. The frame floor is not part of this PR.
• .github/workflows/ci.yml: the check job builds, lints and tests openab-cp --no-default-features and fails if any dependency that the server feature enables (the optional deps in Cargo.toml, read by script so the list cannot drift) reaches its normal dependency graph, or if cargo tree gives no usable output. --workspace always enables server through the binary, so nothing else in CI covers this configuration.
• src/config.rs tests: the max_outbound_queue_bytes <= max_frame_bytes rule has two cases, queue 1048576 with the default frame (equal) and queue 1048576 with frame 2097152 (strictly below), and every rejection case asserts its error text instead of a bare is_err().

This is the code reviewed as part of #1528 (commit 9455b50, which round 6 F12 found clean), carried onto current main with no conflicts. The changes to it are the test tightening above, deferring the frame-floor validation to PR B, and doc wording that describes the runtime client as a follow-up PR rather than already bundled.

Plan change: the frame floor moves from A to B

The #1528 closure plan put the CpConfig::validate() check max_frame_bytes >= MIN_RUNTIME_FRAME_BYTES, its test and the cp.toml.example paragraph in this PR. Review round 1 (comment) asked to drop it: nothing on main consumes the floor yet, it would stop existing configs below 1 MiB from starting, and its message described a frame-size negotiation the protocol does not have. The owner agreed, so A ships neither the constant nor the floor. PR B adds both together with the client that relies on them, with an ASCII message, an exact-message test, a >= 1 lower bound and no claim of negotiation.

Why this approach?

It keeps one crate as the single source of the wire contract and avoids a separate openab-cp-proto crate before a second consumer exists. The default build is unchanged, so cargo run -p openab-cp and the docs keep working. Landing it first gives B and C a small, already-reviewed base.

Alternatives Considered

• Separate openab-cp-proto crate: a cleaner boundary, but it adds a crate and a path move with no consumer that needs it yet. It can be extracted later if openab-pty embeds the client.
• Reopen feat(cp): land Agent Control Plane v1 on main #1528 as one 10k-line PR: rejected by the owner in favour of the split; see the PR B/C follow-ups.

#1528 review findings addressed here

• Round 6: none of F1–F14 are in this PR's files. F12 verified this split as clean. Every other round-6 finding maps to B, C or D (see Follow-ups).
• Round 3 F7 (deferred to refactor(cp): post-v1 hardening follow-ups from #1528 self-review #1530, since closed): the max_frame_bytes bail! string and its substring-only test. Moves to PR B together with the floor validation itself; fix it there (ASCII message, exact-message test, no claim of frame-size negotiation).
• Also: tests/ws_lifecycle.rs is gated on server so cargo test --no-default-features runs the proto tests instead of failing to compile.

Validation

The full local runs below were on 3fbe402. Later heads change docs, drop the floor check and one unit test (98626dc), and add the CI step (0916de6, 9837a1a). The CI step script was rerun on macmini for 9837a1a (exit 0, 16 proto tests); CI is the gate for the final head.

Run on macOS (M4, rustc 1.99.1) in a clean copy of this head:

• cargo build -p openab-cp --locked
• cargo test -p openab-cp --locked: 142 unit + 9 integration passed
• cargo check -p openab-cp --no-default-features --locked; cargo test -p openab-cp --no-default-features --locked: 16 proto tests passed
• cargo tree -p openab-cp --no-default-features -e normal: no axum/tokio/hyper
• cargo clippy -p openab-cp --all-targets -- -D warnings (default and --no-default-features); cargo fmt -p openab-cp --check clean
• CI parity: cargo check --workspace, cargo clippy --workspace [--features unified] -- -D warnings, cargo test -p openab-gateway --features acp (417), cargo test -p openab-core --features acp-mcp {acp_mcp::,acp::pool::} (5 / 24), cargo test --features acp (42), cargo build --features unified
• cargo test --workspace: all pass on macOS except openab-core secrets::tests::resolve_exec_nonzero_exit, which spawns /bin/false (macOS only has /usr/bin/false). This PR does not touch openab-core; Linux CI covers it.
• Docker dummy-layer check: manifests plus empty lib.rs files and no crates/openab-cp/src/main.rs → cargo metadata --locked and root cargo check --locked succeed
• git diff origin/main -- Cargo.lock is empty

[chaodu-agent]

Important

CHANGES REQUESTED ⚠️ - The round-2 CI gap is closed and the feature split is correct, but the new proto-only leak check can pass without checking anything, and MIN_RUNTIME_FRAME_BYTES is added with no consumer.

What This PR Does

openab-cp always pulled in axum, tokio and the WebSocket server, so a runtime that only needs the wire types (openab_cp::proto) would inherit a whole HTTP server. This PR (part 1 of 3 re-landing #1528) makes proto usable with default-features = false and adds a shared, not-yet-enforced MIN_RUNTIME_FRAME_BYTES constant for the runtime-client PR.

How It Works

• Cargo.toml: serde, serde_json and chrono stay unconditional. The ten server dependencies are optional behind a default server feature (dep: syntax). The openab-cp binary and the ws_lifecycle integration test declare required-features = ["server"].
• lib.rs: proto is always compiled; config, events, policy, registry, router and server are #[cfg(feature = "server")].
• proto.rs: adds MIN_RUNTIME_FRAME_BYTES = 1024 * 1024, documented as not enforced yet.
• config.rs: rejection tests assert the specific error text instead of a bare is_err().
• New in 0916de60: the check job in ci.yml runs clippy and tests for openab-cp --no-default-features, then greps cargo tree -e normal for server crates.

Findings

#	Severity	Finding	Location
1	🟡	The proto-only leak check misses four server deps and passes when cargo tree fails or prints nothing	.github/workflows/ci.yml:105-106
2	🟡	MIN_RUNTIME_FRAME_BYTES has no consumer, enforcement or test in this PR	crates/openab-cp/src/proto.rs:46
3	🟢	Round-2 F1 resolved: CI now builds, lints and tests the proto-only configuration, and the step passed on this head	.github/workflows/ci.yml:96-111
4	🟢	Feature split and target gating are correct, with no fail-open path	crates/openab-cp/Cargo.toml, crates/openab-cp/src/lib.rs
5	🟢	Config rejection tests assert the specific error and cover both the equal and strictly-below queue/frame cases	crates/openab-cp/src/config.rs:639-660
6	🟢	Follow-up suggestion: extract openab-cp-proto once a second consumer appears	-

Finding Details

🟡 F1: The leak check is incomplete and can pass silently

The check rejects only ^(tokio|axum|hyper|tower|tracing|clap|toml) . The server-only dependency list also has futures-util, tracing-subscriber, anyhow, parking_lot and subtle. With env-filter, tracing-subscriber depends on tracing, so a leak through it would still match ^tracing . A leak of futures-util, anyhow, parking_lot or subtle would not be caught.

The || true also covers more than "grep found nothing". Under pipefail, a failing cargo tree (or a change in its output format) leaves leaked empty, and the step passes without having checked anything.

Requested change: check the dependency closure against an allowlist, and fail closed. For example:

• capture cargo tree -p openab-cp --no-default-features -e normal --prefix none into a variable without || true, so a cargo tree failure fails the step;
• fail if that output is empty;
• compare the package names against the expected closure of serde, serde_json and chrono, or at minimum add futures-util|anyhow|parking_lot|subtle|tracing-subscriber to the denylist.

The current manifest marks all ten server dependencies optional, so this is a CI false-negative gap, not a current runtime leak.

🟡 F2: MIN_RUNTIME_FRAME_BYTES lands before anything uses it

A repository search finds only the definition. Today the constant has no effect, so it cannot wrongly reject or accept anything. But it announces a future breaking change: once the runtime-client PR enforces it, an existing CP config with max_frame_bytes below 1 MiB that starts today will fail validation after upgrade.

Requested change (either option):

• (a) Preferred: drop the constant here and add it in the runtime-client PR together with the validate() floor and its test, so the constant, its enforcement and its test land in one change; or
• (b) Keep it, and add to this PR's Follow-ups that the runtime-client PR must call out the max_frame_bytes floor as a breaking config change in its release notes.

🟢 F3: Round-2 F1 resolved

0916de60 adds the openab-cp (proto only, no default features) step to the check job, as requested in round 2. On this head it runs cargo clippy --no-default-features --all-targets -D warnings, cargo test --no-default-features and the dependency check, and the step passed.

🟢 F4: Feature split is correct

All server modules are gated. The binary and ws_lifecycle declare required-features = ["server"], so --no-default-features skips them instead of failing to build, and the server tests are absent rather than reported as passed. proto.rs imports only serde, serde_json and chrono. Cargo.lock is unchanged and the default build is unchanged.

🟢 F5: Tests assert specific errors

Each rejection case pairs its input with an expected error substring. The strictly-below case uses max_frame_bytes = 2097152, so the queue rule is what rejects it. The equal case (queue == frame) is still rejected.

🟢 F6: Long-term option, not a blocker

The manifest and crate docs correctly state that Cargo feature unification brings server back into any build that also enables it (for example cargo build --workspace). A separate openab-cp-proto crate would make the boundary structural instead of depending on default-features = false and a CI leak check. The PR already weighs this under Alternatives Considered. We suggest extracting the crate once a second consumer appears (for example if openab-pty embeds the client), rather than adding more behind the feature flag.

Reviewer Verdicts

Reviewer	Verdict	Findings
Reviewer A	CHANGES REQUESTED ⚠️	F1, F4, F5, F6
Reviewer B	CHANGES REQUESTED ⚠️	F1, F2, F4, F5, F6
Coordinator	CHANGES REQUESTED ⚠️	F1, F2, F3

There are no inline review comments, submitted reviews or external reviewer comments on this PR to address.

Baseline Check

• PR opened: 2026-10-30
• Reviewed head: 0916de60b02ad5bb82d37f4ece05f113f43300a2; base main; merge-base 017ed21b3d4f2befffa2fb47ab522ee041f97ba7 (not stacked)
• Diff: 5 files, +122/-16 (crates/openab-cp/ plus .github/workflows/ci.yml)
• Main already has: openab-cp as a single always-server crate
• Net-new value: a server-free proto build for runtime clients, a CI gate for it, and a shared frame-ceiling constant
• CI on this head: all checks pass, including check and its new proto-only step

What's Good (🟢)

• Small, reversible first step of the feat(cp): land Agent Control Plane v1 on main #1528 split, with no change for existing openab-cp users (default features unchanged)
• Round-1 and round-2 feedback was addressed quickly and without changing runtime behavior
• Comments explain why each gate exists, including the feature-unification caveat

5. Three Reasons We Might Not Need This PR

1. No in-tree consumer yet - nothing in the workspace builds openab-cp with --no-default-features until the runtime-client PR, so this could land together with it.
2. Workspace builds still unify features - the common cargo build --workspace path keeps the server graph, so the benefit only appears when the runtime package is built on its own.
3. A separate protocol crate may age better - it would give an unconditional dependency boundary and remove the need for the feature gate and the leak check.

[chaodu-agent]

/review

[chaodu-agent]

/review

[chaodu-agent]

/review