improve docs and add ability to be a "proxying conductor"#11
Merged
nikomatsakis merged 13 commits intoagentclientprotocol:mainfrom Nov 5, 2025
Merged
improve docs and add ability to be a "proxying conductor"#11nikomatsakis merged 13 commits intoagentclientprotocol:mainfrom
nikomatsakis merged 13 commits intoagentclientprotocol:mainfrom
Conversation
This commit fixes critical misconceptions in the documentation and adds comprehensive proxy mode documentation. Key fixes: - Corrected the fundamental architecture: conductor sits between EVERY component, not just at the edge. Components never talk directly to each other. - Clarified bidirectional nature of _proxy/successor/request protocol: - Proxy→Conductor: "send TO my successor" (downstream) - Conductor→Proxy: "this is FROM your successor" (upstream) - Fixed incorrect statement that "components talk directly via stdio pipes" New content: - Added conceptual vs actual flow diagrams in architecture.md - Added comprehensive proxy mode documentation explaining how conductor can itself act as a proxy component - Documented tree-structured proxy chains and hierarchical architectures - Added detailed message flow examples for both request and response paths - Separated normal mode (conductor as root) from proxy mode (conductor as proxy) in capability handshake documentation Documentation improvements: - Removed duplication between architecture.md and conductor.md - conductor.md now focuses on implementation details and binary usage - architecture.md provides conceptual understanding and protocol details - Added cross-references between documents for better navigation This documentation update prepares for the upcoming implementation of proxy mode support in the conductor code. Co-authored-by: Claude <claude@anthropic.com>
- Implement ComponentProvider trait for AcpAgent type - Add proxy mode detection during conductor initialization - Modify capability offering logic (normal: all except last, proxy: all including last) - Add successor message forwarding for proxy mode - Use AtomicBool for thread-safe proxy mode state In proxy mode, the conductor itself acts as a proxy component in a larger chain, enabling tree-structured proxy chains where conductors can be nested. Co-authored-by: Claude <claude@anthropic.com>
Rename the crate from sacp-doc-test to sacp-test to better reflect its purpose as a test utilities package rather than just documentation tests. - Rename directory from src/sacp-doc-test to src/sacp-test - Update package name in Cargo.toml - Update workspace members and dependencies - Update release-plz configuration - Update all documentation references Co-authored-by: Claude <claude@anthropic.com>
Add a simple test proxy that prepends '>' to session update messages. This proxy demonstrates basic proxy functionality and will be used in conductor integration tests. - Add sacp-proxy and tokio-util dependencies to sacp-test - Implement arrow_proxy module with run_arrow_proxy function - Intercepts SessionNotification from successor and modifies content - Adds '>' prefix to text content in AgentMessageChunk updates - Includes basic compilation test Co-authored-by: Claude <claude@anthropic.com>
Add end-to-end test verifying conductor can orchestrate a proxy chain with the arrow proxy and eliza agent. The test confirms that: - Conductor successfully launches and connects arrow_proxy -> eliza - Session updates from eliza get '>' prefix from arrow proxy - Full proxy chain works correctly end-to-end Test output shows: Eliza: "Hello. How are you feeling today?" After arrow proxy: ">Hello. How are you feeling today?" - Add arrow_proxy example binary for standalone execution - Add conductor integration test spawning both processes - Add sacp-test and sacp-tokio as dev-dependencies - Add tracing dependencies to sacp-test Co-authored-by: Claude <claude@anthropic.com>
Creates arrow_proxy_eliza.rs integration test that verifies proxy chain functionality with a > prefix-adding proxy followed by the eliza agent. Uses the new yolo_prompt helper from sacp-test to simplify test setup, reducing boilerplate from ~80 lines to ~15 lines of test code.
Creates nested_arrow_proxy.rs integration test that verifies multiple proxies work correctly in a chain. The test creates a chain with: arrow_proxy1 -> arrow_proxy2 -> eliza Each arrow proxy adds a '>' prefix to session updates, so the final response has '>>' prefix, demonstrating that both proxies are working correctly in sequence.
- Fix CommandComponentProvider to parse command strings with shell_words - Add shell-words dependency to sacp-conductor - Add nested_conductor.rs test for hierarchical proxy chains - Test currently failing - inner conductor proxy mode needs investigation
When forwarding messages to successor in proxy mode, wrap them as _proxy/successor/request to maintain proper protocol. The nested conductor test is still failing - the outer conductor's initialize request to the inner conductor never receives a response. Need to investigate why the response chain breaks.
All Conductor::run and Conductor::run_with_command calls now include the name parameter that was added to the conductor.
Instrument the serve function's main future with a tracing span that includes the conductor's name. This makes it easier to distinguish between multiple conductors (e.g., outer and inner) in logs.
- Extract elizacp logic into run_elizacp() function in lib.rs for reuse - Refactor main.rs to use the extracted run_elizacp() function - Add MockArrowProxy, MockEliza, and MockInnerConductor component providers - Implement test_nested_conductor_with_arrow_proxies using mock components - Implement test_nested_conductor_with_external_arrow_proxies using cargo run - Fix Send bounds in run_elizacp by cloning agent into closures - Add elizacp as dev-dependency for sacp-conductor tests The mock component approach enables unified tracing in a single process, making it much easier to debug message flow through nested conductors. The external test validates that the same behavior works with actual subprocess spawning. Co-authored-by: Claude <claude@anthropic.com>
Remove all tracing_subscriber::fmt() initialization blocks from test files. Tracing can be re-enabled on demand when debugging specific tests by temporarily adding the setup back. Affected files: - src/sacp-conductor/tests/arrow_proxy_eliza.rs - src/sacp-conductor/tests/initialization_sequence.rs (3 instances) - src/sacp-conductor/tests/mcp-integration.rs (2 instances) - src/sacp-conductor/tests/nested_arrow_proxy.rs - src/sacp-conductor/tests/nested_conductor.rs Co-authored-by: Claude <claude@anthropic.com>
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.
The docs are now accurate and we also support a "tree" of proxies.