feat(sdk): wait/steer injection mode support (clean branch from main)#601
feat(sdk): wait/steer injection mode support (clean branch from main)#601barryonthecape wants to merge 14 commits intoAgentWorkforce:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds end-to-end support for explicit message injection modes (wait / steer) across the broker protocol and the TS/Python SDKs, plus PTY worker behavior adjustments so steer injections aren’t blocked by autosuggest gating.
Changes:
- Introduces
MessageInjectionModein the broker protocol with default-to-waitsemantics and threads it through send-message flows. - Updates PTY worker injection logic to avoid autosuggest blocking for
steerdeliveries (including a pre-injection ESC ESC) and adds focused tests. - Extends the TS and Python SDKs (protocol + client surfaces + tests/docs) to send/receive the mode field, and bumps
relaycastto v1.
Reviewed changes
Copilot reviewed 18 out of 19 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/relaycast_ws.rs | Adds send_with_mode() and maps broker injection mode to relaycast mode for channel sends. |
| src/pty_worker.rs | Adjusts autosuggest blocking logic to exempt steer and adds unit tests for the gating behavior. |
| src/protocol.rs | Defines MessageInjectionMode and adds defaulted injection_mode/mode fields to protocol payloads with tests. |
| src/main.rs | Threads mode through broker send flows; adds rejection logic for steer on Relaycast-only HTTP API fallback; forwards mode on SDK Relaycast sends. |
| src/listen_api.rs | Parses/validates mode from /api/send requests, defaults to wait, and adds route tests. |
| packages/sdk/src/relay.ts | Extends message and send APIs to include mode; forwards mode through callbacks and inbound events. |
| packages/sdk/src/protocol.ts | Adds MessageInjectionMode and optional mode/injection_mode fields to relevant protocol types. |
| packages/sdk/src/client.ts | Adds mode to sendMessage inputs and forwards it in WS/HTTP payloads. |
| packages/sdk/src/tests/orchestration-upgrades.test.ts | Adds a test verifying sendMessage forwards mode. |
| packages/sdk-py/tests/test_send_message_mode.py | Adds tests ensuring Python SDK includes mode in payloads and surfaces it on Message objects. |
| packages/sdk-py/src/agent_relay/relay.py | Adds mode to Message and threads it through send/receive paths. |
| packages/sdk-py/src/agent_relay/protocol.py | Adds MessageInjectionMode literal type. |
| packages/sdk-py/src/agent_relay/client.py | Adds optional mode parameter and includes it in send_message payloads. |
| packages/sdk-py/src/agent_relay/init.py | Exposes MessageInjectionMode in the public package exports. |
| packages/sdk-py/README.md | Documents mode usage in the Python SDK. |
| Cargo.toml | Bumps relaycast dependency to =1.0.0. |
| Cargo.lock | Locks relaycast to 1.0.0. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/relaycast_ws.rs
Outdated
|
|
||
| /// Smart send with explicit injection mode. | ||
| pub async fn send_with_mode( | ||
| &self, | ||
| to: &str, | ||
| text: &str, | ||
| mode: MessageInjectionMode, | ||
| ) -> Result<()> { | ||
| if to.starts_with('#') { | ||
| self.send_to_channel(to, text).await | ||
| } else { | ||
| self.send_dm(to, text).await | ||
| let token = self.ensure_token().await?; | ||
| let agent_client = AgentClient::new(&token, Some(self.base_url.clone())) | ||
| .map_err(|e| anyhow::anyhow!("failed to create agent client: {e}"))?; | ||
| let relay_mode = match mode { | ||
| MessageInjectionMode::Wait => relaycast::MessageInjectionMode::Wait, | ||
| MessageInjectionMode::Steer => relaycast::MessageInjectionMode::Steer, | ||
| }; | ||
| agent_client | ||
| .send_with_mode(to, text, None, None, relay_mode, None) | ||
| .await | ||
| .map_err(|e| anyhow::anyhow!("relaycast send_to_channel failed: {e}"))?; | ||
| return Ok(()); | ||
| } | ||
|
|
||
| if matches!(mode, MessageInjectionMode::Steer) { | ||
| return Err(anyhow::anyhow!( |
There was a problem hiding this comment.
send_with_mode() accepts a mode argument but the non-channel (DM) path unconditionally calls send_dm() and does not use mode. This means callers can request Steer and it will be silently ignored for DMs (and any non-# target). Consider either (1) returning an error when mode=Steer is used on the DM path, or (2) threading mode into a mode-aware DM API if the relaycast crate supports it, so the method’s behavior matches its signature/docstring.
| } else if let Some(_http) = relaycast_http { | ||
| let to = payload.to.clone(); | ||
| let eid = event_id.clone(); | ||
| match selected_workspace | ||
| .http_client | ||
| .send(&to, &payload.text) | ||
| .send_with_mode(&to, &payload.text, payload.mode) | ||
| .await |
There was a problem hiding this comment.
In the SDK WS send_message handler, Relaycast fallback forwards payload.mode into http_client.send_with_mode(). For DM targets (non-#), send_with_mode() currently ignores mode, so mode=steer will appear accepted but won’t actually take effect. Consider rejecting Steer (or at least warning) when the request is going to the Relaycast DM path, or implement mode-aware DM sending so the SDK behavior is not misleading.
src/main.rs
Outdated
| if matches!(mode, MessageInjectionMode::Steer) { | ||
| tracing::warn!( | ||
| target = "relay_broker::http_api", | ||
| event_id = %event_id, | ||
| to = %normalized_to, | ||
| "rejecting mode=steer for relaycast-only delivery path" | ||
| ); | ||
| let _ = reply.send(Err( | ||
| "mode=steer is only supported for local PTY delivery; target was relaycast-only" | ||
| .to_string(), | ||
| )); | ||
| continue; | ||
| } |
There was a problem hiding this comment.
The HTTP API send path rejects mode=steer whenever delivery falls back to Relaycast-only (delivered==0), but the SDK WS path can publish to Relaycast with mode=steer (and channels explicitly map mode via send_with_mode). This creates inconsistent semantics across two send entrypoints. Consider aligning behavior: either allow Relaycast publish with Steer here (and forward mode), or enforce the same rejection in the SDK path as well.
| if matches!(mode, MessageInjectionMode::Steer) { | |
| tracing::warn!( | |
| target = "relay_broker::http_api", | |
| event_id = %event_id, | |
| to = %normalized_to, | |
| "rejecting mode=steer for relaycast-only delivery path" | |
| ); | |
| let _ = reply.send(Err( | |
| "mode=steer is only supported for local PTY delivery; target was relaycast-only" | |
| .to_string(), | |
| )); | |
| continue; | |
| } |
xkonjin
left a comment
There was a problem hiding this comment.
Code Review: Message Injection Mode + relaycast 1.0 + CI Hardening
Clean feature addition. The mode: "wait" | "steer" parameter flows correctly through the entire SDK stack (protocol → client → relay → human handle). CI changes are sensible.
✅ Strengths
- New test file
test_send_message_mode.pycovers client, human handle, and agent handle paths, plus the received-message parsing - Backward compatible:
modeis optional everywhere, existing callers unaffected - CI fix for
rust-fmt-fix.ymlcorrectly prevents the workflow from running on fork PRs (which can't push back) npm auditchange (non-blocking +--omit=dev) is pragmatic for burning down a backlog
🔍 Suggestions
-
Received message
modeparsing ambiguity: Inrelay.pyline 782:mode=event.get("injection_mode") or event.get("mode"),
This uses
or, which means ifinjection_modeis an empty string, it falls through tomode. If the server could legitimately sendinjection_mode: "", this would be a bug. Consider usingevent.get("injection_mode") if "injection_mode" in event else event.get("mode")if the distinction matters, or document that empty string is treated as unset. -
npm audit exit code swallowed: The
security.ymlchange removesexit 1from the audit failure path. This means the CI step now always succeeds even with high/critical vulnerabilities in runtime deps. The comment says "non-blocking while known backlog is burned down" which is reasonable, but consider adding a tracked issue to re-enable the failure once the backlog is cleared, or the intent gets lost. -
relaycast 1.0: The pinned version bump from
=0.3.0to=1.0.0is a major version change. The Cargo.lock shows only a checksum change with no transitive dependency changes, which is good. But since this is an exact pin (=1.0.0), worth noting in the PR description what changed in relaycast 1.0 so reviewers can assess the impact without cross-referencing the crate changelog. -
README example: The new
system()usage example in the Python SDK README is helpful. Minor: the example showsmode="wait"with a comment# or "steer"but doesn't explain the semantic difference between wait and steer. A one-line doc comment would go a long way for new users.
Test coverage
Good for the happy path. Consider adding:
- A test where
mode=Noneis passed (verifies it's excluded from the payload, not sent asnull) - A test for the received-message
injection_modevsmodefallback logic
Overall, clean and well-structured PR.
Fresh PR rebuilt from current main with only steer/wait injection-mode work cherry-picked (no unrelated merge baggage).
Includes:
wait/steer)This replaces the noisy diff shape from #573 and avoids unrelated files from merge commits.