feat(cast_agent): enforce the coven.daemon.v1 handshake#220
Conversation
GET /api/v1/health responses on the Unix daemon transport are now classified per docs/CLIENT-INTEGRATION.md: apiVersion must equal coven.daemon.v1 and the capabilities block is cached. The gateway tracks ConnectionState (Unknown | Unreachable | Incompatible | Ready) instead of a bare bool; is_available() derives from it, and connection_state() is exposed through CastAgent, CastAgentRuntime, and the ai facade. The legacy TCP bridge keeps its any-2xx semantics. The agent panel composer now picks an action-oriented placeholder for daemon conversations: offline names the real verb (coven daemon start) and version mismatch says to update Coven or CastCodes. Coverage: handshake classification unit tests; handshake_probe socket tests driving health_probe against an in-process stub daemon (ready / incompatible-then-recover / unreachable); and a full-app integration test (test_daemon_conversation_composer_names_daemon_fix) proving an opened daemon conversation binds LiveDaemon and surfaces the coven-daemon-start placeholder.
with_mac_key_binding and with_linux_or_windows_key_binding overwrite the binding trigger, so calling them after with_custom_action erased the Trigger::Custom that Mac menu building uses to resolve the item name for ToggleUnifiedAgentPanel. The lookup returned no description, tripping a debug_assert inside Cocoa menu construction and aborting every debug build (including all integration tests) via panic_cannot_unwind. Order the builder calls like every other custom-action binding: with_custom_action last. The menu keystroke comes from custom_tag_to_keystroke, which already maps the action to cmd-shift-U / ctrl-shift-U.
There was a problem hiding this comment.
Pull request overview
This PR hardens CastCodes’ Coven daemon integration by enforcing the mandatory coven.daemon.v1 handshake on the Unix-socket transport, upgrading availability tracking from a boolean to a structured ConnectionState, and wiring that state through cast_agent → ai facade → unified agent panel UI (including more actionable inactive-composer placeholders). It also adds unit/socket/integration coverage and fixes a Mac keybinding builder ordering issue that could abort debug builds.
Changes:
- Add
cast_agent::handshakewithDaemonHealth/DaemonCapabilitiesand pureclassify_health_response, and have the Unix transport health probe cache aConnectionState(Unknown | Unreachable | Incompatible | Ready) instead of a bool. - Expose
connection_state()throughCastAgent,CastAgentRuntime, andcrates/ai, and update the unified agent panel composer to select offline/incompatible placeholders via a pureDaemonLinkprojection. - Add new socket-level handshake probe tests and a full-app integration test asserting the daemon-offline composer placeholder names
coven daemon start.
Verdict
Request changes (per repo review guidance for UI-impacting changes): please include manual testing evidence (screenshot/screen recording) for the updated unified agent panel placeholder behavior. Additionally, address the GatewayClient::is_available() performance regression noted in review comments (it currently clones the full ConnectionState).
Reviewed changes
Copilot reviewed 18 out of 19 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/integration/tests/integration/ui_tests.rs | Registers the new agent panel integration test in the UI suite. |
| crates/integration/src/test/agent_panel.rs | Adds an end-to-end integration test for the daemon-offline composer placeholder flow. |
| crates/integration/src/test.rs | Wires the new agent_panel test module into the integration test harness exports. |
| crates/integration/src/bin/integration.rs | Registers the new integration test in the standalone integration binary. |
| crates/cast_agent/tests/handshake_probe.rs | Adds Unix-socket end-to-end tests asserting health_probe() enforces the handshake and caches capabilities/state. |
| crates/cast_agent/src/runtime.rs | Exposes CastAgentRuntime::connection_state() and a global connection_state() convenience accessor. |
| crates/cast_agent/src/lib.rs | Exports handshake types/constants and the new connection_state() API through the crate surface. |
| crates/cast_agent/src/handshake.rs | Introduces handshake models and classification logic for /api/v1/health. |
| crates/cast_agent/src/gateway.rs | Replaces AtomicBool availability with RwLock<ConnectionState> and enforces the handshake on Unix transport. |
| crates/cast_agent/src/agent.rs | Exposes CastAgent::connection_state() for UI-facing consumers. |
| crates/ai/src/lib.rs | Re-exports handshake state/types and connection_state() through the ::ai::cast_agent facade. |
| CAST-AGENT.md | Updates health/handshake documentation and failure-mode behavior. |
| app/src/workspace/mod.rs | Fixes keybinding builder ordering so with_custom_action() isn’t overwritten (Mac menu name resolution). |
| app/src/integration_testing/mod.rs | Exposes a new integration_testing::agent_panel helper module. |
| app/src/integration_testing/agent_panel/mod.rs | Adds integration-test helpers for seeding/opening daemon conversations and reading composer placeholder selection. |
| app/src/agent_panel/view/composer.rs | Adds DaemonLink projection and pure placeholder selection (inactive_placeholder_for) keyed off handshake outcome. |
| app/src/agent_panel/view_tests.rs | Adds unit test coverage for the new placeholder selection logic. |
| app/src/agent_panel/strings.rs | Adds new user-facing offline/incompatible daemon placeholder strings. |
| .gitignore | Ignores /.playwright-cli artifacts. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
is_available() runs on every poll/render; go through the RwLock read guard and match readiness in place instead of cloning the DaemonHealth strings via connection_state().
…he Windows build
Two review findings:
- The refresh loop polled /api/v1/sessions and the familiar catalog
unconditionally, so a daemon the app itself classified Incompatible
(or one that never completed the handshake) still got v1-shaped
requests every cycle — exactly the undefined-response dependency the
handshake exists to prevent. Each cycle now re-probes health first
and skips the fetches unless the daemon answered Ready; recovery
resumes on the cycle after the daemon speaks the contract again.
Regression-tested with a path-counting stub daemon: off-contract
health keeps /health polling and never touches /sessions.
- The handshake::{self, ...} import is only used inside cfg(unix)
branches; under -D warnings it broke the Windows clippy job. Split
it behind cfg(unix) like the existing anyhow import. Verified with
cargo check -p cast_agent --target x86_64-pc-windows-msvc.
The Incompatible state also covers a missing apiVersion and an unparseable health body, not just a version mismatch. Log a distinct message instead of 'speaks ""' when the version is empty, and reword the composer placeholder so it is accurate for every incompatible cause.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 19 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
crates/cast_agent/src/gateway.rs:241
- The doc comment on GatewayClient::connection_state() says it is "Cheap" to call on the UI thread, but the implementation clones the full ConnectionState (including Strings inside DaemonHealth/DaemonCapabilities). That clone can allocate on frequent UI calls, so the comment is misleading; either clarify that this accessor clones (and is_available() is the cheap readiness check), or introduce a lightweight projection accessor for render loops.
/// Latest handshake outcome. Cheap, sync, safe on the UI thread.
pub fn connection_state(&self) -> ConnectionState {
self.state
.read()
.unwrap_or_else(|poisoned| poisoned.into_inner())
| Ok(resp) => { | ||
| let state = handshake::classify_health_response(resp.status, &resp.body); | ||
| if let ConnectionState::Incompatible { api_version } = &state { | ||
| // An empty version means the health body was | ||
| // unparseable or omitted apiVersion — not a real | ||
| // version mismatch, so don't log `speaks ""`. | ||
| if api_version.is_empty() { | ||
| log::warn!( | ||
| "cast_agent: Coven daemon at {} returned an invalid health response (missing or unparseable apiVersion), expected {:?} — update Coven or CastCodes", | ||
| socket.display(), | ||
| handshake::DAEMON_API_VERSION, | ||
| ); | ||
| } else { | ||
| log::warn!( | ||
| "cast_agent: Coven daemon at {} speaks {:?}, expected {:?} — update Coven or CastCodes", | ||
| socket.display(), | ||
| api_version, | ||
| handshake::DAEMON_API_VERSION, | ||
| ); | ||
| } | ||
| } | ||
| state | ||
| } |
| /// Latest `coven.daemon.v1` handshake outcome. Cheap, sync, safe to | ||
| /// call from the UI render thread — reads the state cached by the | ||
| /// most recent [`Self::health_probe`]. | ||
| pub fn connection_state(&self) -> crate::handshake::ConnectionState { | ||
| self.gateway.connection_state() |
| /// Latest `coven.daemon.v1` handshake outcome. Cheap, sync, safe to | ||
| /// call on the UI thread on every render. | ||
| pub fn connection_state(&self) -> crate::handshake::ConnectionState { | ||
| self.agent.connection_state() | ||
| } |
Description
What: Enforces the mandatory
coven.daemon.v1handshake on the Unix daemon transport.GET /api/v1/healthresponses are now classified perdocs/CLIENT-INTEGRATION.mdin OpenCoven/coven:apiVersionmust equal"coven.daemon.v1", and thecapabilitiesblock is parsed and cached. The gateway tracks aConnectionState(Unknown | Unreachable | Incompatible | Ready) instead of a bare bool;is_available()derives from it, andconnection_state()is exposed throughCastAgent,CastAgentRuntime, and theaifacade.Why: Skipping the handshake means depending on undefined response shapes from a future daemon — any 2xx (even an HTML error page or a v2 daemon) previously read as "healthy". The version gate turns that into a distinct, user-explainable state. This also unblocks capability-gated features: panel surfaces can now ask
connection_state().capabilities()instead of probing endpoints blind.How:
crates/cast_agent/src/handshake.rs(new) — response shapes (DaemonHealth,DaemonCapabilities) and the pureclassify_health_response(status, body); additive daemon fields are ignored, missing fields degrade to "capability absent", never a parse error.crates/cast_agent/src/gateway.rs—health_probe()runs the classification on Unix and stores the outcome in anRwLock<ConnectionState>(wasAtomicBool). The legacy TCP bridge predates the versioned contract and keeps its any-2xx semantics with conservatively empty capabilities.app/src/agent_panel/view/composer.rs— a daemon conversation's inactive composer now names the real fix: offline →Start it with `coven daemon start`.; version mismatch → "Update Coven or CastCodes." (selection is pure viaDaemonLink+inactive_placeholder_for).CAST-AGENT.md— health endpoint + failure-mode docs updated.app/src/workspace/mod.rs— one-line ordering fix: the key-binding builder calls were overwriting theTrigger::Customon theToggleUnifiedAgentPanelbinding, so Mac menu construction couldn't resolve the item name and adebug_assertaborted every debug build at boot (all UI integration tests included).with_custom_actionnow comes last, matching every other custom-action binding; the menu keystroke comes fromcustom_tag_to_keystroke.Linked Issue
No tracked GitHub issue — this lands the handshake requirement from the Coven daemon client-integration contract (
docs/CLIENT-INTEGRATION.md, OpenCoven/coven). Follows up #156 (wire CastCodes to the live daemon) and builds on the unified agent panel surface (#210, #216).ready-to-specorready-to-implement. (n/a — no tracked issue)Testing
Three layers of automated coverage, all CI-runnable:
handshake::tests(8 tests): version match/mismatch/missing, unparseable body, non-2xx, additive-field tolerance, capability defaults.crates/cast_agent/tests/handshake_probe.rs(new, 4 tests):GatewayClient::health_probe()against an in-process stub daemon on a real Unix socket — contract health →Readywith cached capabilities; wrongapiVersion→Incompatible, recovering toReadyon the next probe; missing socket / HTTP 500 →Unreachable.test_daemon_conversation_composer_names_daemon_fix(new,crates/integration/src/test/agent_panel.rs, registered inui_tests.rs): boots the app, seeds a daemon conversation through the production refresh path, dispatchesWorkspaceAction::OpenChatSession, and asserts the panel opens, bindsLiveDaemon, and the composer placeholder namescoven daemon start. Helpers live inapp/src/integration_testing/agent_panel/.Verified locally:
cargo nextest run -p cast_agent— 58 passed (incl. the 4 new socket tests)cargo nextest run -p warp-app -E 'test(/agent_panel/)'— 4 passed (default features, i.e. thenot(cast-agent)cfg)cargo clippy -p cast_agent -p ai --all-targets --tests -- -D warnings— cleancargo clippy -p warp-app --features cast-agent --all-targets -- -D warnings— clean (thecast-agentcfg)cargo clippy -p warp-app --features integration_tests,cast-agent --all-targets -- -D warnings— cleancargo clippy -p integration --all-targets -- -D warnings+cargo check -p integration --all-targets— cleancargo fmt -- --check,./script/check_ai_attribution,./script/check_rebrand— cleanIntegration test executed locally: the abort that previously blocked every UI integration test on this machine was root-caused to the
ToggleUnifiedAgentPanelbinding losing itsTrigger::Custom(see the workspace fix above). With the fix,test_daemon_conversation_composer_names_daemon_fixpasses 3 consecutive runs via the integration binary, and the pre-existing baselinetest_add_many_sessionspasses again. Full workspace presubmit was not run; the targeted gates above cover every touched crate and bothcast-agentcfg branches../script/runpass — the default./script/runbuild does not enable thecast-agentfeature; the end-to-end integration test boots the real app UI instead):GatewayClientover the auto-detected~/.coven/coven.sockagainst the running coven daemon —health_probe()→Ready,api_version: coven.daemon.v1, full capabilities cached;is_available()flipped false→true.test_daemon_conversation_composer_names_daemon_fixboots the app and asserts the composer namescoven daemon start.Incompatible→ recovery on a real Unix socket;inactive_placeholder_names_the_daemon_fixpins the "Update Coven or CastCodes." copy.Review Follow-ups
81052466—is_available()reads readiness through theRwLockguard instead of cloning the fullConnectionStateon every poll/render (review thread).084af73c— two fixes from a deeper review pass:/api/v1/sessionsunconditionally, even against a daemon classifiedIncompatible. Each cycle now re-probes health first and skips the fetches unless the daemon answeredReady. New regression testrefresh_loop_never_polls_sessions_while_daemon_is_off_contract(path-counting stub daemon:/healthkeeps polling,/sessionsis never touched while off-contract).handshake::{self, ...}was only used insidecfg(unix)branches and tripped-D warningson the Windows clippy job; the module import is now#[cfg(unix)], matching the existinganyhowpattern. Verified withcargo check -p cast_agent --target x86_64-pc-windows-msvc.Updated verification at HEAD:
cargo nextest run -p cast_agent63/63, clippy (-D warnings) clean, Windows cross-check clean, integration test passing.Agent Mode
CHANGELOG-IMPROVEMENT: The agent panel now tells you exactly how to recover when the Coven daemon is offline (
coven daemon start) or speaks an incompatible API version, instead of showing a generic message.