Split into acp-mux core + rooms collaboration layer#72
Merged
Conversation
Introduce a Cargo workspace with two crates and a compiler-enforced boundary: - acp-mux (lib acp_mux, bin acp-mux): the standalone generic 1->N ACP multiplexer. Id translation, first-writer-wins agent-request fan-in, initialize/session/new caching, fs/* and terminal/* safety, plain replay/late-join, and an RFD 533 baseline session/attach + session/detach. Attaches on ?mux=. Carries no rooms/* knowledge and does not depend on the rooms crate. - rooms (lib rooms, bin rooms): the collaboration protocol, implemented as a MuxExtension plugged into the core actor. Turns, queue/steer/cancel, presence, segments, rooms/* frames, and _meta.rooms attach enrichment. Attaches on ?room=. Depends on acp-mux. The boundary is a MuxExtension trait + MuxCtx capability surface in core (core ships a no-op extension). There is a single multiplexer implementation; the old src/room/state.rs monolith is removed.
Add a mock ACP agent (mock_acp_core) and crates/acp-mux/tests/conformance.rs covering session/attach roster + history policies (full / none / pending_only), session/detach shape, first-writer-wins permission resolution, and the load-bearing invariant that the core path never emits a rooms/* frame.
…pace Update README, ROADMAP, CHANGELOG, and the design docs for the two-crate workspace. Rename the namespace spec and client-contract fixtures to the rooms/* namespace (the collaboration layer previously used amux/*), and add the refactor plan under docs/design/.
Remove the unused `room_id` helper and the empty `permissions` stub module, then drop the crate-wide `#![allow(dead_code)]` from the rooms crate. With that mask gone the crate is clean under `clippy -D warnings`, matching the core crate (any future dead code now surfaces instead of being hidden). The one remaining allow is a targeted field-level annotation in the registry.
Reflow lines that exceeded the width limit after the amux -> rooms rename (rooms is one character longer), and format the conformance test. No logic changes.
The split is complete; the outcome is captured in README, ROADMAP, and CHANGELOG. Drop the working plan doc and its ROADMAP link.
…e docs The namespace spec, fixture, and README document rooms/cancel_active_turn as a JSON-RPC request addressed to the mux, but only the notification shape was intercepted. A request-shaped cancel (with an id) fell through on_subscriber_request and was forwarded to the agent, which a standards-compliant agent rejects with method-not-found — so the turn was never cancelled, violating the "addressed to the mux, not the agent" contract. Add a shared cancel_active_turn helper and call it from on_subscriber_request: return a result ack, or -32002 when there is no active turn (consistent with the other rooms/* control requests). The notification shape keeps working as a fire-and-forget alias, so neither shape ever reaches the agent. Add request-shape tests (active turn -> mux ack and no agent forward; no active turn -> -32002) that close the coverage gap which hid the mismatch.
…-segment carry The split rewrote the `full` history filter to key off `replay_generation` (`generation == 0` -> include everything; else active-segment-only with no turn-bookend carry). That diverged from the documented contract two ways: - after a NOTIFICATION-driven sessionId rotation, `replay_generation` stays 0 (only session/load bumps it), so `full` returned every segment and behaved like `full_lineage`; - after a load-driven rotation, the active-segment-only filter dropped the prior-segment `rooms/turn_started` needed to bracket a current-segment `rooms/turn_complete`. Restore the original semantics in `should_include_replay_entry`, driven purely by per-frame segment tags (`ext_tag`) like the pre-split `history_full`: include pre-segment bootstrap (tag 0) + active-segment frames + `rooms/turn_*` lifecycle bookends whose `roomsTurnId` is in the cross-segment carry set. This is correct after both rotation kinds and never depends on `replay_generation`. The filter governs the attach response, streamed history, and the legacy transport replay (via the replay_frame hook) uniformly. Add a `MOCK_ACP_ROTATE_SESSION_ID` knob to the mock agent (mid-turn notification rotation) and a regression test asserting `full` carries the spanning turn's bookends while excluding prior-segment agent chunks, and that `full_lineage` still includes them. These were unit-tested in the pre-split monolith; that coverage was lost in the move.
…tion When an agent notification carried a params.sessionId different from the established canonical id, the rooms layer rotated its segment but core's canonical_session_id stayed pinned to the old id. session/attach resolves and validates against that core canonical id, so after such a rotation a late attach reported the stale sessionId and rejected the actually-current one with "session not found". (Pre-existing: the pre-split monolith rotated via rotate_segment without updating canonical_session_id either.) Make core authoritative for canonical session-id tracking: when an agent notification's sessionId differs from the established canonical id, core calls set_canonical_session_id(..), which updates its own canonical id AND fires on_canonical_session_id so the extension rotates segments through the existing path. This fixes the standalone core too. Remove the rooms extension's independent on_agent_notification rotation so rotation is driven once, through on_canonical_session_id, for session/new, session/load, and notification-driven changes alike. Add a regression test: after a notification-driven rotation, session/attach reports the new canonical id and accepts it.
…t the gap The replay store rehydrates the broadcast frames on restart, but not the segment lineage layered on top, so after a restart only `full_lineage` is correct (current-segment `full`, the attach snapshot lineage, replayGeneration, and the resolved sessionId reset until new agent activity). The CHANGELOG and rooms.md previously claimed segment bookends / lineage / canonical session-id were restored on restart. Correct those claims and add a "Known limitation: cross-restart segment fidelity" section to rooms.md with a concrete failure-mode example (a multi-day, multi-segment room redeployed nightly, where a reconnecting client asking for historyPolicy:full gets a near-empty view). Reconstructing segment state from the persisted rooms/segment_* frames remains a tracked follow-up.
…snapshot shape
The client-contract fixture showed _meta.rooms.connectedClients as
{peerId, peerName, role} (the peer_joined shape), but the attach roster is
ConnectedClient = {clientId, name} (no role). The snapshot block was also stale:
it carried a non-existent `roomId`, omitted the always-present `selfPeer`,
`activeTurn`, `queue`, `pendingPermissions`, `replayBoundarySeq`, and
`replayGeneration` keys, and its segment entry dropped the required
`openedReplaySeq`.
Rewrite _meta.rooms to match what on_attach actually emits (verified against
the rooms attach test and the on_attach builder). Code was correct; only the
fixture had drifted. Fixtures are not validated against the code, which is how
this drifted — adding a fixture-shape test is a worthwhile follow-up.
…gnals coincide If an agent emits loaded-session session/update frames before its session/load response, the mux observes the sessionId change first and labels the closed segment acp_session_id_changed rather than session_load. Clarify that endReason is diagnostic, not a strict priority contract, so this is expected.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Splits the project into a two-crate Cargo workspace with a compiler-enforced boundary:
acp-mux(libacp_mux, binacp-mux) — the standalone generic 1→N ACP multiplexer: per-client JSON-RPC id translation, first-writer-wins agent-request fan-in,initialize/session/newcaching for late joiners,fs/*/terminal/*safety defaults, plain replay/late-join, and an RFD 533 baselinesession/attach/session/detach. Attaches on?mux=. Carries norooms/*knowledge and does not depend on theroomscrate.rooms(librooms, binrooms) — the collaboration protocol, implemented as aMuxExtensionplugged into the core actor: turns, queue/steer/cancel, presence, segments,rooms/*frames, and_meta.roomsattach enrichment. Attaches on?room=. Depends onacp-mux.The boundary is a
MuxExtensiontrait +MuxCtxcapability surface in core (core ships a no-op extension). There is now a single multiplexer implementation; the oldsrc/room/state.rsmonolith is gone. The core also structurally guarantees only standard ACP ever reaches the agent — the extension can request sanctioned ACP actions but cannot write raw bytes to the agent.Breaking changes
amux→rooms, including the wire namespace:amux/*methods →rooms/*,_meta.amux→_meta.rooms, and theamuxTurnIdfield →roomsTurnId. Clients written against the previousamux/*releases must update method names and_metakeys. The lower-level core crate/binary staysacp-mux.Notes
acp-muxis provider-neutral;--agent-cmdlaunches any stdio ACP agent.rooms/*.Test plan
cargo test --workspace— 145 passing (core: 21 unit + 6 RFD 533 conformance; rooms: 35 unit + 83 integration).cargo clippy --workspace --all-targets -- -D warnings— clean.cargo fmt --all --check— clean.