From 621ec3b98835960029104121734b15d38753363a Mon Sep 17 00:00:00 2001 From: Michael Feth Date: Wed, 12 Aug 2026 22:22:28 -0400 Subject: [PATCH 01/34] docs(wayfinder): design for external project dispatch + Twilio SMS operator agent Research-backed plan for routing Buzz messages/SMS into agent dispatch against bidcraft (BuildBid) and construct-pro, plus the Twilio inbound/ outbound SMS bridge and operator-agent disambiguation flow. Signed-off-by: Michael Feth --- .../map-external-project-sms-integration.md | 81 +++++++++++++++++++ 1 file changed, 81 insertions(+) create mode 100644 docs/wayfinder/map-external-project-sms-integration.md diff --git a/docs/wayfinder/map-external-project-sms-integration.md b/docs/wayfinder/map-external-project-sms-integration.md new file mode 100644 index 00000000000..eaffa957e8f --- /dev/null +++ b/docs/wayfinder/map-external-project-sms-integration.md @@ -0,0 +1,81 @@ +# Buzz External Project Integration + SMS Operator Agent + +## Goal + +Let a message arriving in Buzz (from a channel or, eventually, an SMS) trigger an agent dispatch that operates against an *external* repo (bidcraft/BuildBid, construct-pro) instead of only the harness's own working directory. Reuse Buzz's existing NIP-MP `project` primitive for repo scoping rather than inventing a new one, and add a Twilio SMS front door that resolves to the same dispatch path once messages land in Buzz as normal channel events. + +**Note:** `mfethe1/bidcraft` (GitHub) is the same project as the local `E:\Projects\buildbid` checkout — it's BuildBid, a construction-estimating SaaS (AI vision/estimation pipeline, Supabase auth, Stripe billing, deployed on Railway, live at buildbid.app). The two names refer to one project. + +## Part A: Buzz ↔ bidcraft/construct-pro agent dispatch + +**Project representation — reuse `KIND_PROJECT` (30621, NIP-MP), don't overload community.** Community is the relay-tenant boundary (one relay/host); project is already the right-sized primitive for "which repo(s) does this dispatch target" — it groups `kind:30617` git-repo announcements via `a`-tag coordinates and already has CLI surface (`buzz-cli/src/lib.rs:215-220`, `projects`/`repos` subcommands). No new event kind needed for project scoping itself. + +**New/changed pieces:** + +1. **`cwd` plumbing (the actual gap).** `AcpClient::session_new_full` already accepts an arbitrary absolute `cwd` per the ACP wire protocol (`acp.rs:621`, `acp.rs:638`), but every caller hardcodes `std::env::current_dir()` (`lib.rs:1600`, `lib.rs:4149`). Add `cwd: Option` resolution to the prompt-dispatch path in `lib.rs`: if the triggering event (or its channel's persona config) carries a project reference, resolve it to a local worktree path and pass that; otherwise fall back to current behavior. Validate the resolved path is absolute and exists before passing it into `session_new_full` — don't trust a tag value directly into a filesystem path (untrusted input → path traversal risk). +2. **Project → local-path resolution.** Add a config table/section (in `config.rs`, near `resolve_channel_filters`, config.rs:1241) mapping a project's `d`-tag identifier to a local checkout path, e.g. `bidcraft` → `E:/Projects/buildbid`, `construct-pro` → a `.claude/worktrees/buzz-` path (avoid the taken `mack/`, `honey/`, `winnie/`, `airy/`, `fizz/`, `parity/`, `dispatch-` namespaces already in use in construct-pro). This is harness-side config, not new protocol — the mapping doesn't need to live on-relay. +3. **Tagging convention.** Channel-message events that should dispatch against an external project carry an `a`-tag pointing at the `kind:30621` project coordinate (NIP-MP's existing addressing), or the channel's own persona/config is statically bound to one project. Prefer the static binding for v1 (simpler, matches "one operator channel per external project" from Part B) and treat per-message `a`-tag override as a stretch goal. +4. **`buzz-workflow` / `buzz-cli` changes.** No `buzz-workflow` changes required for this part — dispatch already happens through `buzz-acp`'s own event loop, not through workflow webhooks. `buzz-cli` needs one addition per AGENTS.md's "agent-facing operations go in `buzz-cli`" convention: a subcommand to inspect/set a channel's bound project (thin wrapper over the existing `projects`/`repos` subcommands, `buzz-cli/src/lib.rs:215-220`), so an operator can configure the binding without hand-editing harness config. +5. **Worktree hygiene for construct-pro specifically.** New worktrees go under `.claude/worktrees/buzz-` in construct-pro, never reusing an active agent's handle prefix. +6. **bidcraft/BuildBid specifically.** No clone needed — it's already local at `E:/Projects/buildbid`. Confirm it's a valid git checkout before binding it as a dispatch target (an earlier check found no `.git` at the top level; may need `git init`/re-clone or point at the actual nested repo root). + +**Files:** `crates/buzz-acp/src/{lib.rs,acp.rs,config.rs}`, `crates/buzz-cli/src/lib.rs:212-220`, `crates/buzz-core/src/kind.rs` (no change — reusing 30621), new config section for project→path mapping. + +## Part B: Twilio SMS + Operator Agent + +1. **Inbound webhook — dedicated route, not `/hooks/{id}`.** New `crates/buzz-relay/src/api/sms.rs`, wired at `POST /hooks/sms/inbound` in `router.rs` (alongside router.rs:121). Twilio's form-encoded body (`From`, `Body`, `To`, `MessageSid`) needs its own extractor — `buzz-workflow`'s `Webhook` trigger (`bridge.rs:1800`) is bound to a specific pre-authored `WorkflowDef` UUID and doesn't fit "arbitrary inbound SMS → new event." +2. **Signature validation.** New `crates/buzz-relay/src/twilio_auth.rs` implementing Twilio's HMAC-SHA1(URL + sorted params, AuthToken) scheme — this is the sole trust boundary, since Twilio can't hold a Nostr key and NIP-42/NIP-98 don't apply. +3. **Phone allow-list + project-default mapping.** New migration: + ```sql + CREATE TABLE sms_identities ( + phone_number TEXT PRIMARY KEY, -- E.164 + community_id UUID NOT NULL REFERENCES communities(id), + allowed BOOLEAN NOT NULL DEFAULT false, + linked_pubkey BYTEA, -- 32-byte pubkey, nullable + default_project TEXT, -- NIP-MP project d-tag, e.g. "bidcraft" | "construct-pro" | NULL + created_at, updated_at + ); + ``` + Enforced inside `sms.rs` before synthesizing any event: missing row or `allowed = false` → 403, no event, no reply (closes the spam/oracle vector). `default_project` is a string identifier matching a `kind:30621` project's `d`-tag — this is Part A's primitive reused, not a new scoping concept. +4. **Synthesized event.** Reuse `KIND_STREAM_MESSAGE_V2` (40002, `kind.rs:468`) — no new kind. Author = `linked_pubkey` if set, else a per-community "SMS relay" service pubkey. Tags: `["sms_from", phone_number]`, `["sms_sid", MessageSid]`, `h` = the community's fixed SMS-inbox channel group id. +5. **Operator persona — decision logic.** New `crates/buzz-persona/sms-operator.toml`, subscribed only to the SMS-inbox channel (`h`-tag filter): + - Fast path: `default_project` on the `sms_identities` row resolves directly → dispatch via Part A's `cwd`-resolution path, scoped to that project. + - Ambiguous (no default, or content contradicts default): persona posts a reply event in SMS-inbox ("Reply 1 for bidcraft, 2 for construct-pro"), which flows out through the outbound sink (step 6) as a real SMS; the next inbound reply resolves it. + - Once resolved, dispatch is a normal `buzz-acp` `session_new_full` call — no relay-side change beyond the event plumbing above. +6. **Outbound SMS sink.** New `crates/buzz-relay/src/sms_sink.rs`, mirroring `workflow_sink.rs`'s pattern: subscribes server-side to the SMS-inbox channel, on any reply event tagged (`e`-tag) back to an inbound `sms_from` message, calls Twilio's `POST /Messages`. +7. **Secrets.** Twilio Account SID + Auth Token as relay config/env secrets — never in the `sms_identities` table. + +**Files:** `crates/buzz-relay/src/api/sms.rs`, `crates/buzz-relay/src/twilio_auth.rs`, `crates/buzz-relay/src/sms_sink.rs`, `crates/buzz-relay/src/router.rs`, `migrations/00XX_sms_identities.sql`, `crates/buzz-persona/sms-operator.toml`. + +## Dependencies / things only the user can provide + +- **Twilio account + AuthToken/Account SID** — needed before `twilio_auth.rs` can be tested against real signatures. +- **A purchased/rented Twilio phone number** — ~$1/mo recurring, required for both inbound routing and outbound sends; not auto-provisionable. +- **Per-message SMS cost** — roughly $0.0079+/message in the US, scales with volume; ongoing operating cost, not a one-time build cost. +- **BuildBid/bidcraft local checkout state** — confirm `E:/Projects/buildbid` is a proper git checkout (or point at wherever its actual `.git` root is) before binding it as a dispatch target. +- **construct-pro worktree slot** — needs a decision on which unclaimed namespace (`buzz-`) to standardize on, and whether Buzz dispatch should target the open `map.md` WF-01 queue as its first live test. + +## Phased implementation plan (vertical slices) + +1. **Project→path config resolution (no dispatch yet)** → verify: add a `[projects]` mapping entry in harness config pointing a fake project id at a scratch directory; write a unit test that `config.rs`'s resolver returns the expected path for a known id and `None` for unknown. +2. **Thread `cwd` through `session_new_full` callers** → verify: manually trigger a channel message with a bound project, observe (via ACP subprocess launch args/log) that the spawned agent's working directory is the resolved external path, not `std::env::current_dir()`; confirm a message on an unbound channel still uses the old default (regression check). +3. **`buzz-cli` project-binding subcommand** → verify: run the new subcommand against a running relay, confirm it reads/writes the channel↔project binding and that a subsequent dispatch (slice 2) picks it up without a harness restart, if config is live-reloaded — otherwise document the restart requirement. +4. **First live external dispatch — bidcraft/BuildBid (read-only)** → verify: bind a test channel to bidcraft's project id, post a message asking the agent to summarize open work in `E:/Projects/buildbid`, confirm the response reflects real repo state. +5. **First live external dispatch — construct-pro (code change)** → verify: bind a channel to construct-pro, dispatch against WF-01 (open, unblocked, smallest ticket per `docs/wayfinder/map.md`), confirm the agent works inside a fresh `.claude/worktrees/buzz-` worktree and produces a diff addressing the ticket's cited evidence lines. +6. **`sms_identities` migration + allow-list enforcement** → verify: run the migration locally, hit `/hooks/sms/inbound` with a forged Twilio-shaped POST for an unlisted number, confirm 403 and no event written; add an allowed row and confirm the same request now produces a `KIND_STREAM_MESSAGE_V2` event with correct tags. +7. **Twilio signature validation** → verify: send a request with a wrong/missing `X-Twilio-Signature`, confirm 403; send one with a correctly computed signature (using a test Auth Token), confirm it passes and reaches the allow-list check. +8. **Operator persona — fast path (default_project set)** → verify: seed an `sms_identities` row with `default_project` = bidcraft or construct-pro, post a synthetic inbound-SMS event into the SMS-inbox channel, confirm the persona dispatches via the Part A path (slice 2) into the correct project's `cwd`. +9. **Operator persona — ambiguous path** → verify: seed a row with `default_project = NULL`, post an inbound event, confirm the persona posts a disambiguation reply event instead of dispatching. +10. **Outbound SMS sink** → verify: with real (or Twilio test-credential sandbox) SID/token configured, post a reply event tagged back to an inbound message, confirm `sms_sink.rs` calls Twilio's API and the test phone number (or Twilio's magic test number) receives/logs the outbound message. +11. **End-to-end SMS → project dispatch → reply** → verify: full loop — real inbound SMS from an allow-listed number with a clear `default_project`, agent dispatches and completes, outbound reply SMS arrives back at the sending number. + +## Open questions for Michael + +- Should `linked_pubkey` be required for `allowed = true` rows, or is the shared "SMS relay" service pubkey acceptable for allow-listed-but-unlinked numbers? +- Confirm Buzz dispatch's first live test target: construct-pro's `map.md` WF-01 (open, unblocked, small), or a Buzz-specific ticket filed in that same queue instead? +- Do you already have a Twilio account and a number, or does that need to be set up before slice 6 can be tested against real signatures (vs. a stubbed/test AuthToken)? +- Is `E:/Projects/buildbid` the real checkout for `mfethe1/bidcraft`, or a separate/stale local copy that should be re-pointed at the actual repo? + +--- +Sources: buzz-external-integration-design workflow (5 research agents), run 2026-08-12 +Last updated: 2026-08-12 From f9163ece498437aa2424118a8ee4042959ce31ff Mon Sep 17 00:00:00 2001 From: Michael Feth Date: Wed, 12 Aug 2026 22:52:22 -0400 Subject: [PATCH 02/34] feat(acp): resolve per-channel cwd for external project dispatch Add --project-paths (project id -> local checkout path) and --channel-projects (channel UUID -> project id) config, resolved once at startup into PromptContext.channel_cwd. Dispatched sessions in a bound channel now launch with that project's cwd instead of always the harness's own working directory; unbound channels are unaffected. Slices 1+2 of docs/wayfinder/map-external-project-sms-integration.md, combined because an unconsumed resolver trips this crate's deny(dead_code) clippy gate. Signed-off-by: Michael Feth --- crates/buzz-acp/src/config.rs | 281 ++++++++++++++++++++++++++++++++++ crates/buzz-acp/src/lib.rs | 5 + crates/buzz-acp/src/pool.rs | 50 +++++- 3 files changed, 334 insertions(+), 2 deletions(-) diff --git a/crates/buzz-acp/src/config.rs b/crates/buzz-acp/src/config.rs index f9e7bf1ed8a..12d3753ac21 100644 --- a/crates/buzz-acp/src/config.rs +++ b/crates/buzz-acp/src/config.rs @@ -489,6 +489,21 @@ pub struct CliArgs { /// Requires `--lazy-pool`; ignored otherwise. 0 disables idle re-sleep. #[arg(long, env = "BUZZ_ACP_IDLE_POOL_SLEEP", default_value_t = 0)] pub idle_pool_sleep: u64, + + /// Maps a NIP-MP project `d`-tag id to a local checkout path this harness + /// can dispatch agent work into, e.g. `bidcraft=E:/Projects/buildbid`. + /// Comma-separated `id=path` pairs. A channel bound to a project (see + /// `--channel-projects`) resolves through this map to pick the + /// spawned agent's working directory instead of the harness's own cwd. + #[arg(long, env = "BUZZ_ACP_PROJECT_PATHS", value_delimiter = ',')] + pub project_paths: Option>, + + /// Binds a channel (UUID) to a project id from `--project-paths`, e.g. + /// `3fa85f64-...=bidcraft`. Comma-separated `channel_uuid=project_id` + /// pairs. Agent turns dispatched in a bound channel run with that + /// project's cwd instead of the harness's own working directory. + #[arg(long, env = "BUZZ_ACP_CHANNEL_PROJECTS", value_delimiter = ',')] + pub channel_projects: Option>, } /// Merged NIP-01 subscription filter for a single channel. @@ -579,6 +594,13 @@ pub struct Config { /// `from_cli()`. `None` when using the compiled-in default or when /// `--no-base-prompt` is set. pub base_prompt_content: Option, + /// NIP-MP project `d`-tag id -> local checkout path this harness may + /// dispatch agent work into. Populated from `--project-paths`. + pub project_paths: HashMap, + /// Channel id -> project id, populated from `--channel-projects`. + /// Resolved through `project_paths` at `PromptContext` build time to + /// produce the effective per-channel cwd. + pub channel_projects: HashMap, } /// Maximum length, in characters, of a session title sent to the adapter. @@ -1071,6 +1093,9 @@ impl Config { validate_multiple_event_handling(args.multiple_event_handling, args.dedup)?; + let project_paths = parse_project_paths(args.project_paths)?; + let channel_projects = parse_channel_projects(args.channel_projects)?; + let config = Config { keys, relay_url: args.relay_url, @@ -1122,6 +1147,8 @@ impl Config { agent_owner: args.agent_owner.map(|s| s.trim().to_ascii_lowercase()), no_base_prompt: args.no_base_prompt, base_prompt_content, + project_paths, + channel_projects, }; Ok(config) @@ -1249,6 +1276,116 @@ pub fn load_rules(path: &std::path::Path) -> Result, Confi Ok(config.rules) } +/// Parse `--project-paths` entries (`id=path` pairs) into a validated map. +/// +/// Rejects an empty id, an empty path, or a duplicate id — all indicate a +/// typo'd flag rather than a deliberate mapping, and failing fast here beats +/// silently dropping (or overwriting) a project binding at dispatch time. +fn parse_project_paths(raw: Option>) -> Result, ConfigError> { + let mut map = HashMap::new(); + for entry in raw.into_iter().flatten() { + let Some((id, path)) = entry.split_once('=') else { + return Err(ConfigError::ConfigFile(format!( + "--project-paths entry '{entry}' is not in 'id=path' form" + ))); + }; + let (id, path) = (id.trim(), path.trim()); + if id.is_empty() || path.is_empty() { + return Err(ConfigError::ConfigFile(format!( + "--project-paths entry '{entry}' has an empty id or path" + ))); + } + if map.insert(id.to_string(), PathBuf::from(path)).is_some() { + return Err(ConfigError::ConfigFile(format!( + "--project-paths has duplicate id '{id}'" + ))); + } + } + Ok(map) +} + +/// Resolve a NIP-MP project id to the local checkout path an agent dispatched +/// against that project should run in, per `--project-paths` config. +/// Returns `None` for an unbound/unknown project id — callers fall back to +/// the harness's own working directory. +pub fn resolve_project_path(config: &Config, project_id: &str) -> Option { + config.project_paths.get(project_id).cloned() +} + +/// Parse `--channel-projects` entries (`channel_uuid=project_id` pairs). +/// +/// Rejects a malformed entry, a non-UUID channel id, or a duplicate channel +/// binding — the same fail-fast rationale as [`parse_project_paths`]. Does +/// NOT validate the project id exists in `project_paths`; an unbound-at-startup +/// project id is caught later when the harness resolves cwds and warns instead +/// of crashing (a project path config change shouldn't require restarting a +/// harness whose channel bindings didn't move). +fn parse_channel_projects(raw: Option>) -> Result, ConfigError> { + let mut map = HashMap::new(); + for entry in raw.into_iter().flatten() { + let Some((channel, project_id)) = entry.split_once('=') else { + return Err(ConfigError::ConfigFile(format!( + "--channel-projects entry '{entry}' is not in 'channel_uuid=project_id' form" + ))); + }; + let (channel, project_id) = (channel.trim(), project_id.trim()); + let channel_id = channel.parse::().map_err(|_| { + ConfigError::ConfigFile(format!( + "--channel-projects entry '{entry}' has an invalid channel UUID" + )) + })?; + if project_id.is_empty() { + return Err(ConfigError::ConfigFile(format!( + "--channel-projects entry '{entry}' has an empty project id" + ))); + } + if map.insert(channel_id, project_id.to_string()).is_some() { + return Err(ConfigError::ConfigFile(format!( + "--channel-projects has duplicate channel id '{channel_id}'" + ))); + } + } + Ok(map) +} + +/// Precompute the effective cwd for every channel bound to a project, once at +/// harness startup. A channel absent from the returned map has no override — +/// callers fall back to the harness's own default cwd. +/// +/// Validates each resolved path is absolute and exists on disk; an invalid or +/// unbound-project entry is logged and skipped rather than crashing startup — +/// a stale `--channel-projects` binding (e.g. a project path config change) +/// shouldn't take down an otherwise-healthy harness. Skipping instead of +/// silently trusting an unresolvable path also closes the path-traversal +/// concern of handing an agent subprocess a cwd derived from unvalidated +/// config. +pub fn build_channel_cwd_map(config: &Config) -> HashMap { + let mut result = HashMap::new(); + for (channel_id, project_id) in &config.channel_projects { + let Some(path) = resolve_project_path(config, project_id) else { + tracing::warn!( + %channel_id, + project_id, + "channel is bound to an unknown project id (not in --project-paths) — \ + dispatches in this channel will use the harness's default cwd" + ); + continue; + }; + if !path.is_absolute() || !path.exists() { + tracing::warn!( + %channel_id, + project_id, + path = %path.display(), + "resolved project path is not an absolute, existing directory — \ + dispatches in this channel will use the harness's default cwd" + ); + continue; + } + result.insert(*channel_id, path.to_string_lossy().into_owned()); + } + result +} + /// Resolve per-channel NIP-01 filters from config + discovered channels. pub fn resolve_channel_filters( config: &Config, @@ -1494,6 +1631,8 @@ mod tests { agent_owner: None, no_base_prompt: false, base_prompt_content: None, + project_paths: HashMap::new(), + channel_projects: HashMap::new(), } } @@ -2982,4 +3121,146 @@ channels = "ALL" Add `hide_env_values = true` to each: {violations:?}" ); } + + #[test] + fn parse_project_paths_none_is_empty_map() { + let map = parse_project_paths(None).unwrap(); + assert!(map.is_empty()); + } + + #[test] + fn parse_project_paths_parses_id_equals_path_pairs() { + let raw = vec![ + "bidcraft=E:/Projects/buildbid".to_string(), + "construct-pro=E:/Projects/construct-pro".to_string(), + ]; + let map = parse_project_paths(Some(raw)).unwrap(); + assert_eq!(map.len(), 2); + assert_eq!( + map.get("bidcraft").unwrap(), + &PathBuf::from("E:/Projects/buildbid") + ); + assert_eq!( + map.get("construct-pro").unwrap(), + &PathBuf::from("E:/Projects/construct-pro") + ); + } + + #[test] + fn parse_project_paths_rejects_missing_equals() { + let err = parse_project_paths(Some(vec!["bidcraft-only-id".to_string()])).unwrap_err(); + assert!(matches!(err, ConfigError::ConfigFile(_))); + } + + #[test] + fn parse_project_paths_rejects_empty_id_or_path() { + assert!(parse_project_paths(Some(vec!["=E:/Projects/buildbid".to_string()])).is_err()); + assert!(parse_project_paths(Some(vec!["bidcraft=".to_string()])).is_err()); + } + + #[test] + fn parse_project_paths_rejects_duplicate_id() { + let raw = vec![ + "bidcraft=E:/Projects/buildbid".to_string(), + "bidcraft=E:/Projects/other".to_string(), + ]; + assert!(parse_project_paths(Some(raw)).is_err()); + } + + #[test] + fn resolve_project_path_known_and_unknown_ids() { + let mut config = test_config(SubscribeMode::All); + config.project_paths.insert( + "bidcraft".to_string(), + PathBuf::from("E:/Projects/buildbid"), + ); + + assert_eq!( + resolve_project_path(&config, "bidcraft"), + Some(PathBuf::from("E:/Projects/buildbid")) + ); + assert_eq!(resolve_project_path(&config, "unknown-project"), None); + } + + #[test] + fn parse_channel_projects_none_is_empty_map() { + assert!(parse_channel_projects(None).unwrap().is_empty()); + } + + #[test] + fn parse_channel_projects_parses_uuid_equals_project_id() { + let channel_id = Uuid::new_v4(); + let raw = vec![format!("{channel_id}=bidcraft")]; + let map = parse_channel_projects(Some(raw)).unwrap(); + assert_eq!(map.get(&channel_id).unwrap(), "bidcraft"); + } + + #[test] + fn parse_channel_projects_rejects_invalid_uuid() { + assert!(parse_channel_projects(Some(vec!["not-a-uuid=bidcraft".to_string()])).is_err()); + } + + #[test] + fn parse_channel_projects_rejects_empty_project_id() { + let channel_id = Uuid::new_v4(); + assert!(parse_channel_projects(Some(vec![format!("{channel_id}=")])).is_err()); + } + + #[test] + fn parse_channel_projects_rejects_duplicate_channel() { + let channel_id = Uuid::new_v4(); + let raw = vec![ + format!("{channel_id}=bidcraft"), + format!("{channel_id}=construct-pro"), + ]; + assert!(parse_channel_projects(Some(raw)).is_err()); + } + + #[test] + fn build_channel_cwd_map_resolves_bound_existing_project() { + let mut config = test_config(SubscribeMode::All); + let channel_id = Uuid::new_v4(); + // Use the current crate dir as a real, existing, absolute path stand-in. + let existing_dir = std::env::current_dir().unwrap(); + config + .project_paths + .insert("bidcraft".to_string(), existing_dir.clone()); + config + .channel_projects + .insert(channel_id, "bidcraft".to_string()); + + let map = build_channel_cwd_map(&config); + assert_eq!( + map.get(&channel_id).unwrap(), + &existing_dir.to_string_lossy().into_owned() + ); + } + + #[test] + fn build_channel_cwd_map_skips_channel_bound_to_unknown_project() { + let mut config = test_config(SubscribeMode::All); + let channel_id = Uuid::new_v4(); + config + .channel_projects + .insert(channel_id, "no-such-project".to_string()); + + let map = build_channel_cwd_map(&config); + assert!(!map.contains_key(&channel_id)); + } + + #[test] + fn build_channel_cwd_map_skips_nonexistent_path() { + let mut config = test_config(SubscribeMode::All); + let channel_id = Uuid::new_v4(); + config.project_paths.insert( + "bidcraft".to_string(), + PathBuf::from("Z:/definitely/does/not/exist/anywhere"), + ); + config + .channel_projects + .insert(channel_id, "bidcraft".to_string()); + + let map = build_channel_cwd_map(&config); + assert!(!map.contains_key(&channel_id)); + } } diff --git a/crates/buzz-acp/src/lib.rs b/crates/buzz-acp/src/lib.rs index 7fd40b83db1..1093ad824c1 100644 --- a/crates/buzz-acp/src/lib.rs +++ b/crates/buzz-acp/src/lib.rs @@ -2182,6 +2182,7 @@ async fn tokio_main() -> Result<()> { .unwrap_or_else(|_| std::path::PathBuf::from("/")) .to_string_lossy() .to_string(), + channel_cwd: crate::config::build_channel_cwd_map(&config), rest_client: relay.rest_client(), channel_info: pool::ChannelInfoResolver::new(channel_info_map, relay.rest_client()), context_message_limit: config.context_message_limit, @@ -6765,6 +6766,8 @@ mod build_mcp_servers_tests { agent_owner: None, no_base_prompt: false, base_prompt_content: None, + project_paths: std::collections::HashMap::new(), + channel_projects: std::collections::HashMap::new(), } } @@ -6988,6 +6991,8 @@ mod error_outcome_emission_tests { agent_owner: None, no_base_prompt: false, base_prompt_content: None, + project_paths: std::collections::HashMap::new(), + channel_projects: std::collections::HashMap::new(), } } diff --git a/crates/buzz-acp/src/pool.rs b/crates/buzz-acp/src/pool.rs index 2efacce2b19..6da8402507a 100644 --- a/crates/buzz-acp/src/pool.rs +++ b/crates/buzz-acp/src/pool.rs @@ -577,6 +577,11 @@ pub struct PromptContext { /// (`include_str!`) is inherently `'static`. pub base_prompt: Option<&'static str>, pub cwd: String, + /// Per-channel cwd override for channels bound to an external project via + /// `--channel-projects` / `--project-paths`, precomputed once at startup + /// by [`crate::config::build_channel_cwd_map`]. A channel absent from + /// this map dispatches with `cwd` (the harness's own default). + pub channel_cwd: HashMap, /// REST client for pre-prompt context fetches (thread/DM history). pub rest_client: RestClient, /// Shared channel metadata for startup-known and dynamically joined channels. @@ -952,6 +957,16 @@ async fn resolve_new_session_channel_context( (is_dm, title_channel, Some(info.channel_type)) } +/// Resolve the cwd a new session in `channel_id` should launch with: the +/// channel's bound-project override if one is configured, otherwise the +/// harness's own default cwd. +fn resolve_effective_cwd(ctx: &PromptContext, channel_id: Option) -> &str { + channel_id + .and_then(|id| ctx.channel_cwd.get(&id)) + .map(String::as_str) + .unwrap_or(&ctx.cwd) +} + /// Create a new ACP session via `session_new_full()`, populate model capabilities /// on the agent (first session only), and apply `desired_model` if set. /// @@ -979,11 +994,12 @@ async fn create_session_and_apply_model( // its own `[Agent Memory — core]` header, and canvas carries its own // `[Channel Canvas]` header; both are appended with a blank-line separator. let is_goose = agent.agent_name == "goose"; + let effective_cwd = resolve_effective_cwd(ctx, channel.id); let combined_system_prompt = with_canvas( with_huddle_instructions( with_core( with_team( - framed_system_prompt(&ctx.cwd, ctx.base_prompt, ctx.system_prompt.as_deref()), + framed_system_prompt(effective_cwd, ctx.base_prompt, ctx.system_prompt.as_deref()), ctx.team_instructions.as_deref(), ), agent_core, @@ -1007,7 +1023,7 @@ async fn create_session_and_apply_model( let resp = agent .acp .session_new_full( - &ctx.cwd, + effective_cwd, mcp_servers, session_new_system_prompt( is_goose, @@ -7577,6 +7593,7 @@ printf '%s\n' '{{"jsonrpc":"2.0","id":0,"result":{{"stopReason":"end_turn"}}}}'" heartbeat_prompt: None, base_prompt: None, cwd: ".".to_string(), + channel_cwd: HashMap::new(), rest_client: RestClient { http: reqwest::Client::new(), base_url: "http://127.0.0.1:0".to_string(), @@ -7656,6 +7673,35 @@ printf '%s\n' '{{"jsonrpc":"2.0","id":0,"result":{{"stopReason":"end_turn"}}}}'" .is_none()); } + // ── resolve_effective_cwd ──────────────────────────────────────────────── + + #[test] + fn resolve_effective_cwd_falls_back_to_default_when_channel_unbound() { + let ctx = make_prompt_context_no_owner(); + assert_eq!(resolve_effective_cwd(&ctx, Some(Uuid::new_v4())), ctx.cwd); + } + + #[test] + fn resolve_effective_cwd_falls_back_to_default_when_no_channel() { + let ctx = make_prompt_context_no_owner(); + assert_eq!(resolve_effective_cwd(&ctx, None), ctx.cwd); + } + + #[test] + fn resolve_effective_cwd_uses_bound_project_path() { + let mut ctx = make_prompt_context_no_owner(); + let channel_id = Uuid::new_v4(); + ctx.channel_cwd + .insert(channel_id, "E:/Projects/buildbid".to_string()); + + assert_eq!( + resolve_effective_cwd(&ctx, Some(channel_id)), + "E:/Projects/buildbid" + ); + // A different, unbound channel is unaffected. + assert_eq!(resolve_effective_cwd(&ctx, Some(Uuid::new_v4())), ctx.cwd); + } + // ── render_canvas_section ──────────────────────────────────────────────── #[test] From d01318a9ede3b8efbc1881aba813e5309f87ff8e Mon Sep 17 00:00:00 2001 From: Michael Feth Date: Wed, 12 Aug 2026 23:02:01 -0400 Subject: [PATCH 03/34] docs(wayfinder): correct design doc after verifying against real code/paths - buzz-cli already supports channel<->project binding (projects update --channel), verified by reading the persist/read path and its existing test - no new CLI subcommand needed, correcting slice 3's scope. - E:/Projects/buildbid has no top-level .git; the real bidcraft checkout is E:/Projects/buildbid/bidcraft-repo (verified: origin points at mfethe1/bidcraft, real commit history). - Flag slices 4/5/11 (live agent dispatch) as needing a supervised session rather than unattended background execution - they spin up a real relay + harness + agent subprocess and, for construct-pro, write into a shared repo other agents are actively working in. Signed-off-by: Michael Feth --- .../map-external-project-sms-integration.md | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/docs/wayfinder/map-external-project-sms-integration.md b/docs/wayfinder/map-external-project-sms-integration.md index eaffa957e8f..b67a2af7c31 100644 --- a/docs/wayfinder/map-external-project-sms-integration.md +++ b/docs/wayfinder/map-external-project-sms-integration.md @@ -4,7 +4,9 @@ Let a message arriving in Buzz (from a channel or, eventually, an SMS) trigger an agent dispatch that operates against an *external* repo (bidcraft/BuildBid, construct-pro) instead of only the harness's own working directory. Reuse Buzz's existing NIP-MP `project` primitive for repo scoping rather than inventing a new one, and add a Twilio SMS front door that resolves to the same dispatch path once messages land in Buzz as normal channel events. -**Note:** `mfethe1/bidcraft` (GitHub) is the same project as the local `E:\Projects\buildbid` checkout — it's BuildBid, a construction-estimating SaaS (AI vision/estimation pipeline, Supabase auth, Stripe billing, deployed on Railway, live at buildbid.app). The two names refer to one project. +**Note:** `mfethe1/bidcraft` (GitHub) is the same project as BuildBid — a construction-estimating SaaS (AI vision/estimation pipeline, Supabase auth, Stripe billing, deployed on Railway, live at buildbid.app). The two names refer to one project. + +**Corrected local path (verified 2026-08-12):** `E:/Projects/buildbid` itself is NOT a git checkout — it's a parent folder holding many separate clones/worktrees (`bidcraft-repo`, `bidcraft-repo-claude2`, `bidcraft-cruft-cleanup`, etc.), each with its own `.git`. The canonical checkout is **`E:/Projects/buildbid/bidcraft-repo`** (`origin` = `git@github.com:mfethe1/bidcraft.git`, confirmed via `git remote -v`). Any `--project-paths` entry for `bidcraft` must point here, not at the bare `E:/Projects/buildbid` folder. ## Part A: Buzz ↔ bidcraft/construct-pro agent dispatch @@ -13,11 +15,11 @@ Let a message arriving in Buzz (from a channel or, eventually, an SMS) trigger a **New/changed pieces:** 1. **`cwd` plumbing (the actual gap).** `AcpClient::session_new_full` already accepts an arbitrary absolute `cwd` per the ACP wire protocol (`acp.rs:621`, `acp.rs:638`), but every caller hardcodes `std::env::current_dir()` (`lib.rs:1600`, `lib.rs:4149`). Add `cwd: Option` resolution to the prompt-dispatch path in `lib.rs`: if the triggering event (or its channel's persona config) carries a project reference, resolve it to a local worktree path and pass that; otherwise fall back to current behavior. Validate the resolved path is absolute and exists before passing it into `session_new_full` — don't trust a tag value directly into a filesystem path (untrusted input → path traversal risk). -2. **Project → local-path resolution.** Add a config table/section (in `config.rs`, near `resolve_channel_filters`, config.rs:1241) mapping a project's `d`-tag identifier to a local checkout path, e.g. `bidcraft` → `E:/Projects/buildbid`, `construct-pro` → a `.claude/worktrees/buzz-` path (avoid the taken `mack/`, `honey/`, `winnie/`, `airy/`, `fizz/`, `parity/`, `dispatch-` namespaces already in use in construct-pro). This is harness-side config, not new protocol — the mapping doesn't need to live on-relay. +2. **Project → local-path resolution.** ✅ Done (`--project-paths` CLI/env config, `crates/buzz-acp/src/config.rs`) mapping a project's `d`-tag identifier to a local checkout path, e.g. `bidcraft` → `E:/Projects/buildbid/bidcraft-repo`, `construct-pro` → a `.claude/worktrees/buzz-` path (avoid the taken `mack/`, `honey/`, `winnie/`, `airy/`, `fizz/`, `parity/`, `dispatch-` namespaces already in use in construct-pro). This is harness-side config, not new protocol — the mapping doesn't need to live on-relay. 3. **Tagging convention.** Channel-message events that should dispatch against an external project carry an `a`-tag pointing at the `kind:30621` project coordinate (NIP-MP's existing addressing), or the channel's own persona/config is statically bound to one project. Prefer the static binding for v1 (simpler, matches "one operator channel per external project" from Part B) and treat per-message `a`-tag override as a stretch goal. -4. **`buzz-workflow` / `buzz-cli` changes.** No `buzz-workflow` changes required for this part — dispatch already happens through `buzz-acp`'s own event loop, not through workflow webhooks. `buzz-cli` needs one addition per AGENTS.md's "agent-facing operations go in `buzz-cli`" convention: a subcommand to inspect/set a channel's bound project (thin wrapper over the existing `projects`/`repos` subcommands, `buzz-cli/src/lib.rs:215-220`), so an operator can configure the binding without hand-editing harness config. +4. **`buzz-workflow` / `buzz-cli` changes.** No `buzz-workflow` changes required for this part — dispatch already happens through `buzz-acp`'s own event loop, not through workflow webhooks. ~~`buzz-cli` needs one addition...~~ **Correction (verified 2026-08-12): no new `buzz-cli` subcommand needed.** `buzz projects update --channel ` already writes a `["buzz-channel", uuid]` tag onto the kind:30621 project event (`buzz-cli/src/commands/projects.rs:477-478`, with an existing unit test asserting exactly-one-tag replace semantics at `projects.rs:921-941`), and `projects get`/`list` already print the raw event JSON including that tag. The relay-side channel↔project binding surface already existed; only the harness-local project-id→filesystem-path layer (`--project-paths`, slice 1) and the harness-local channel-id→project-id layer (`--channel-projects`, slice 2 — kept as a separate static flag rather than fetched from the relay, since it's simpler for v1 and avoids a relay dependency at harness startup) were actually missing. 5. **Worktree hygiene for construct-pro specifically.** New worktrees go under `.claude/worktrees/buzz-` in construct-pro, never reusing an active agent's handle prefix. -6. **bidcraft/BuildBid specifically.** No clone needed — it's already local at `E:/Projects/buildbid`. Confirm it's a valid git checkout before binding it as a dispatch target (an earlier check found no `.git` at the top level; may need `git init`/re-clone or point at the actual nested repo root). +6. **bidcraft/BuildBid specifically.** ✅ Verified — the canonical checkout is `E:/Projects/buildbid/bidcraft-repo` (not the bare `E:/Projects/buildbid` folder, which has no top-level `.git` and just holds many separate clones). Confirmed real: `origin` = `git@github.com:mfethe1/bidcraft.git`, real commit history, currently on branch `design/tokens-refresh-2026-08-01` with a couple untracked files (pre-existing, not from this work). **Files:** `crates/buzz-acp/src/{lib.rs,acp.rs,config.rs}`, `crates/buzz-cli/src/lib.rs:212-220`, `crates/buzz-core/src/kind.rs` (no change — reusing 30621), new config section for project→path mapping. @@ -52,7 +54,8 @@ Let a message arriving in Buzz (from a channel or, eventually, an SMS) trigger a - **Twilio account + AuthToken/Account SID** — needed before `twilio_auth.rs` can be tested against real signatures. - **A purchased/rented Twilio phone number** — ~$1/mo recurring, required for both inbound routing and outbound sends; not auto-provisionable. - **Per-message SMS cost** — roughly $0.0079+/message in the US, scales with volume; ongoing operating cost, not a one-time build cost. -- **BuildBid/bidcraft local checkout state** — confirm `E:/Projects/buildbid` is a proper git checkout (or point at wherever its actual `.git` root is) before binding it as a dispatch target. +- **BuildBid/bidcraft local checkout state** — ✅ resolved: use `E:/Projects/buildbid/bidcraft-repo`, not the bare `E:/Projects/buildbid` folder. +- **Live dispatch testing (slices 4, 5, 11)** — these need a running relay + a real `buzz-acp` harness process + a real agent turn. That's a meaningfully bigger and more consequential action than a code change (real subprocess execution, real writes into shared repos other agents are actively working in, particularly construct-pro). Recommend running these as a supervised session rather than unattended background execution. - **construct-pro worktree slot** — needs a decision on which unclaimed namespace (`buzz-`) to standardize on, and whether Buzz dispatch should target the open `map.md` WF-01 queue as its first live test. ## Phased implementation plan (vertical slices) @@ -60,8 +63,8 @@ Let a message arriving in Buzz (from a channel or, eventually, an SMS) trigger a 1. **Project→path config resolution (no dispatch yet)** → verify: add a `[projects]` mapping entry in harness config pointing a fake project id at a scratch directory; write a unit test that `config.rs`'s resolver returns the expected path for a known id and `None` for unknown. 2. **Thread `cwd` through `session_new_full` callers** → verify: manually trigger a channel message with a bound project, observe (via ACP subprocess launch args/log) that the spawned agent's working directory is the resolved external path, not `std::env::current_dir()`; confirm a message on an unbound channel still uses the old default (regression check). 3. **`buzz-cli` project-binding subcommand** → verify: run the new subcommand against a running relay, confirm it reads/writes the channel↔project binding and that a subsequent dispatch (slice 2) picks it up without a harness restart, if config is live-reloaded — otherwise document the restart requirement. -4. **First live external dispatch — bidcraft/BuildBid (read-only)** → verify: bind a test channel to bidcraft's project id, post a message asking the agent to summarize open work in `E:/Projects/buildbid`, confirm the response reflects real repo state. -5. **First live external dispatch — construct-pro (code change)** → verify: bind a channel to construct-pro, dispatch against WF-01 (open, unblocked, smallest ticket per `docs/wayfinder/map.md`), confirm the agent works inside a fresh `.claude/worktrees/buzz-` worktree and produces a diff addressing the ticket's cited evidence lines. +4. **First live external dispatch — bidcraft/BuildBid (read-only)** → verify: bind a test channel to bidcraft's project id (`E:/Projects/buildbid/bidcraft-repo`), post a message asking the agent to summarize open work, confirm the response reflects real repo state. **Deferred pending a supervised session** — needs a running relay + real harness process, a bigger step than a code change. +5. **First live external dispatch — construct-pro (code change)** → verify: bind a channel to construct-pro, dispatch against WF-01 (open, unblocked, smallest ticket per `docs/wayfinder/map.md`), confirm the agent works inside a fresh `.claude/worktrees/buzz-` worktree and produces a diff addressing the ticket's cited evidence lines. **Deferred pending a supervised session** — this dispatches a real agent that writes into a shared, actively-worked repo (multiple other agents have branches in flight there); not something to run unattended. 6. **`sms_identities` migration + allow-list enforcement** → verify: run the migration locally, hit `/hooks/sms/inbound` with a forged Twilio-shaped POST for an unlisted number, confirm 403 and no event written; add an allowed row and confirm the same request now produces a `KIND_STREAM_MESSAGE_V2` event with correct tags. 7. **Twilio signature validation** → verify: send a request with a wrong/missing `X-Twilio-Signature`, confirm 403; send one with a correctly computed signature (using a test Auth Token), confirm it passes and reaches the allow-list check. 8. **Operator persona — fast path (default_project set)** → verify: seed an `sms_identities` row with `default_project` = bidcraft or construct-pro, post a synthetic inbound-SMS event into the SMS-inbox channel, confirm the persona dispatches via the Part A path (slice 2) into the correct project's `cwd`. @@ -74,7 +77,8 @@ Let a message arriving in Buzz (from a channel or, eventually, an SMS) trigger a - Should `linked_pubkey` be required for `allowed = true` rows, or is the shared "SMS relay" service pubkey acceptable for allow-listed-but-unlinked numbers? - Confirm Buzz dispatch's first live test target: construct-pro's `map.md` WF-01 (open, unblocked, small), or a Buzz-specific ticket filed in that same queue instead? - Do you already have a Twilio account and a number, or does that need to be set up before slice 6 can be tested against real signatures (vs. a stubbed/test AuthToken)? -- Is `E:/Projects/buildbid` the real checkout for `mfethe1/bidcraft`, or a separate/stale local copy that should be re-pointed at the actual repo? +- ~~Is `E:/Projects/buildbid` the real checkout for `mfethe1/bidcraft`~~ — resolved: it's `E:/Projects/buildbid/bidcraft-repo`. +- Which `.claude/worktrees/buzz-` name to use for the construct-pro live dispatch test, and whether to run slices 4/5/11 (live dispatch) as a supervised session rather than unattended. --- Sources: buzz-external-integration-design workflow (5 research agents), run 2026-08-12 From 8e111530092175fa0f227ec9a7eb2bb6a9627a12 Mon Sep 17 00:00:00 2001 From: Michael Feth Date: Wed, 12 Aug 2026 23:05:38 -0400 Subject: [PATCH 04/34] feat(db): add sms_identities allow-list + project-routing table MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Schema for the Twilio SMS bridge (slice 6 of the external-integration design doc): phone_number is the allow-list key, default_project names the NIP-MP project d-tag the sms-operator persona should dispatch into when unambiguous. E.164 and 32-byte-pubkey CHECK constraints catch malformed inserts at the DB layer rather than trusting callers. Verified: crate builds clean (sqlx::migrate! embeds the file without macro errors). Not yet applied against a live Postgres in this pass — enforcement logic (the actual allow-list check) lands with the inbound webhook handler next, since that's where it's consumed. Signed-off-by: Michael Feth --- migrations/0031_sms_identities.sql | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 migrations/0031_sms_identities.sql diff --git a/migrations/0031_sms_identities.sql b/migrations/0031_sms_identities.sql new file mode 100644 index 00000000000..c776c902b66 --- /dev/null +++ b/migrations/0031_sms_identities.sql @@ -0,0 +1,19 @@ +-- Twilio SMS allow-list + phone->project routing default. `allowed` gates +-- whether an inbound SMS is admitted at all (closes the anonymous-spam / +-- oracle vector); `default_project` names a NIP-MP project `d`-tag the +-- sms-operator persona dispatches into when the number has no ambiguity. +CREATE TABLE sms_identities ( + phone_number TEXT PRIMARY KEY, + community_id UUID NOT NULL REFERENCES communities(id), + allowed BOOLEAN NOT NULL DEFAULT false, + linked_pubkey BYTEA, + default_project TEXT, + created_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), + updated_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), + CONSTRAINT chk_sms_identities_phone_number_e164 + CHECK (phone_number ~ '^\+[1-9][0-9]{1,14}$'), + CONSTRAINT chk_sms_identities_linked_pubkey_len + CHECK (linked_pubkey IS NULL OR octet_length(linked_pubkey) = 32) +); + +CREATE INDEX idx_sms_identities_community ON sms_identities (community_id); From aa1efdc6fc19644dd2f3d9ddcc9e1b6b786a71a7 Mon Sep 17 00:00:00 2001 From: Michael Feth Date: Wed, 12 Aug 2026 23:38:45 -0400 Subject: [PATCH 05/34] feat(relay): Twilio inbound SMS webhook with signature validation + allow-list Adds POST /hooks/sms/inbound: validates X-Twilio-Signature (HMAC-SHA1 per Twilio's documented scheme, crates/buzz-relay/src/twilio_auth.rs), then requires the sender's phone number be allowed=true in the sms_identities table (crates/buzz-db/src/sms.rs) before doing anything else. Both failures return the same generic 403 so the endpoint can't be used as an oracle for probing valid signatures or registered numbers. twilio_auth's test vectors were computed independently outside this codebase (openssl dgst -hmac + Python's hmac/hashlib, both agreeing) rather than hand-derived from the implementation, so the tests verify the algorithm, not just internal self-consistency. Stops at "allowed -> 200 OK acknowledged" -- does not yet synthesize a relay event (KIND_STREAM_MESSAGE_V2) from the message, which is the next slice. Not yet verified against a live Postgres or a real Twilio account (TWILIO_AUTH_TOKEN/TWILIO_WEBHOOK_URL unset fails closed). Also updates docs/wayfinder/map-external-project-sms-integration.md with a Privacy section: NIP-MP project visibility defaults to `listed` (buzz-cli/src/lib.rs:1262-1263) and channel visibility defaults to `open` (channel_templates.rs:60) -- both must be explicitly overridden to `unlisted`/`private` for bidcraft/construct-pro given they're private repos. Also flags the fetch-before-dispatch freshness gap as a new tracked task rather than silently leaving it undocumented. Signed-off-by: Michael Feth --- Cargo.lock | 1 + Cargo.toml | 1 + crates/buzz-db/src/lib.rs | 9 + crates/buzz-db/src/sms.rs | 55 ++++++ crates/buzz-relay/Cargo.toml | 1 + crates/buzz-relay/src/api/mod.rs | 1 + crates/buzz-relay/src/api/sms.rs | 91 ++++++++++ crates/buzz-relay/src/config.rs | 23 +++ crates/buzz-relay/src/lib.rs | 2 + crates/buzz-relay/src/router.rs | 2 + crates/buzz-relay/src/twilio_auth.rs | 156 ++++++++++++++++++ .../map-external-project-sms-integration.md | 51 +++++- 12 files changed, 390 insertions(+), 3 deletions(-) create mode 100644 crates/buzz-db/src/sms.rs create mode 100644 crates/buzz-relay/src/api/sms.rs create mode 100644 crates/buzz-relay/src/twilio_auth.rs diff --git a/Cargo.lock b/Cargo.lock index 6c46beedf2f..7a604670c00 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1302,6 +1302,7 @@ dependencies = [ "serde", "serde_json", "serde_yaml", + "sha1 0.11.0", "sha2 0.11.0", "sqlx", "subtle", diff --git a/Cargo.toml b/Cargo.toml index 78816ff4827..35f1463beb3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -104,6 +104,7 @@ chrono = { version = "0.4", features = ["serde"] } reqwest = { version = "0.13", features = ["json", "rustls"], default-features = false } # Cryptography +sha1 = "0.11" sha2 = "0.11" hex = "0.4" hmac = "0.13" diff --git a/crates/buzz-db/src/lib.rs b/crates/buzz-db/src/lib.rs index 330525d310d..d2c8d48649c 100644 --- a/crates/buzz-db/src/lib.rs +++ b/crates/buzz-db/src/lib.rs @@ -47,6 +47,8 @@ pub mod relay_invite; pub mod relay_members; /// Replica freshness fence for keyset-cursor read routing. pub mod replica_fence; +/// Twilio SMS allow-list and project-routing persistence. +pub mod sms; /// Thread metadata persistence. pub mod thread; /// Per-community usage rollup queries for Prometheus gauges. @@ -3842,6 +3844,13 @@ impl Db { .await } + /// Look up a phone number's SMS allow-list / project-routing state. + /// Returns `Ok(None)` for an unknown number. + #[datastore_span(name = "get_sms_identity", system = "postgresql")] + pub async fn get_sms_identity(&self, phone_number: &str) -> Result> { + sms::get_sms_identity(&self.pool, phone_number).await + } + /// Fetch a single workflow by ID, scoped to its community. #[datastore_span(name = "get_workflow", system = "postgresql")] pub async fn get_workflow( diff --git a/crates/buzz-db/src/sms.rs b/crates/buzz-db/src/sms.rs new file mode 100644 index 00000000000..b2418caf511 --- /dev/null +++ b/crates/buzz-db/src/sms.rs @@ -0,0 +1,55 @@ +//! Twilio SMS allow-list -- the `sms_identities` table. +//! +//! `allowed` gates whether an inbound SMS is admitted at all; `default_project` +//! is the NIP-MP project `d`-tag the sms-operator persona dispatches into when +//! the sender is unambiguous. See migrations/0031_sms_identities.sql. + +use sqlx::{PgPool, Row}; + +use buzz_core::CommunityId; + +use crate::error::Result; + +/// A phone number's allow-list and project-routing state. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct SmsIdentity { + /// E.164 phone number, the allow-list key. + pub phone_number: String, + /// Community this identity belongs to. + pub community_id: CommunityId, + /// Whether inbound SMS from this number is admitted at all. + pub allowed: bool, + /// Nostr pubkey this number is linked to, if any. + pub linked_pubkey: Option>, + /// NIP-MP project `d`-tag to dispatch into when unambiguous. + pub default_project: Option, +} + +/// Look up a phone number's allow-list state. Returns `Ok(None)` for an +/// unknown number -- callers must treat "unknown" the same as "not allowed" +/// (fail closed), not as a distinct case worth a different response, so the +/// endpoint doesn't become an oracle for which numbers are registered. +pub async fn get_sms_identity(pool: &PgPool, phone_number: &str) -> Result> { + let row = sqlx::query( + r#" + SELECT phone_number, community_id, allowed, linked_pubkey, default_project + FROM sms_identities + WHERE phone_number = $1 + "#, + ) + .bind(phone_number) + .fetch_optional(pool) + .await?; + + let Some(row) = row else { + return Ok(None); + }; + + Ok(Some(SmsIdentity { + phone_number: row.try_get("phone_number")?, + community_id: CommunityId::from_uuid(row.try_get("community_id")?), + allowed: row.try_get("allowed")?, + linked_pubkey: row.try_get("linked_pubkey")?, + default_project: row.try_get("default_project")?, + })) +} diff --git a/crates/buzz-relay/Cargo.toml b/crates/buzz-relay/Cargo.toml index deb2e7e16a5..8997d833bb7 100644 --- a/crates/buzz-relay/Cargo.toml +++ b/crates/buzz-relay/Cargo.toml @@ -69,6 +69,7 @@ tempfile = "3" bytes = "1" infer = "0.19" serde_yaml = { workspace = true } +sha1 = { workspace = true } sha2 = { workspace = true } hmac = { workspace = true } subtle = { workspace = true } diff --git a/crates/buzz-relay/src/api/mod.rs b/crates/buzz-relay/src/api/mod.rs index 2a942bc8039..9dfb8b4ada8 100644 --- a/crates/buzz-relay/src/api/mod.rs +++ b/crates/buzz-relay/src/api/mod.rs @@ -9,6 +9,7 @@ pub mod media; pub mod mesh_demo; pub mod nip05; pub mod operator; +pub mod sms; pub mod workflows; // Re-export imeta helpers used by ingest pipeline. diff --git a/crates/buzz-relay/src/api/sms.rs b/crates/buzz-relay/src/api/sms.rs new file mode 100644 index 00000000000..71c3549d922 --- /dev/null +++ b/crates/buzz-relay/src/api/sms.rs @@ -0,0 +1,91 @@ +//! Twilio inbound SMS webhook — `POST /hooks/sms/inbound`. +//! +//! Trust chain for an inbound SMS, in order, each step fatal on failure: +//! 1. `X-Twilio-Signature` validates the request actually came from Twilio +//! ([`crate::twilio_auth::validate_signature`]). +//! 2. The sender's phone number must be `allowed = true` in `sms_identities` +//! (closes the anonymous-spam / oracle vector — an unlisted number gets +//! the exact same rejection as a signature failure, so a prober can't +//! distinguish "bad signature" from "signature fine, number not allowed"). +//! +//! This slice stops at "allowed → 200 OK, acknowledged". Turning an allowed +//! message into a `KIND_STREAM_MESSAGE_V2` relay event, and the sms-operator +//! persona that reads it, are follow-up slices. + +use std::collections::BTreeMap; +use std::sync::Arc; + +use axum::extract::{Form, State}; +use axum::http::{HeaderMap, StatusCode}; +use axum::response::Json; + +use crate::state::AppState; +use crate::twilio_auth::validate_signature; + +use super::{api_error, internal_error}; + +/// Generic rejection for both "bad signature" and "number not allowed" — +/// deliberately identical so the endpoint isn't an oracle for either fact. +fn rejected() -> (StatusCode, Json) { + api_error(StatusCode::FORBIDDEN, "request not accepted") +} + +/// Handle an inbound Twilio SMS webhook POST. Validates the request +/// signature, then requires the sender's phone number be allow-listed; +/// on success, currently just acknowledges (event synthesis is a follow-up). +pub async fn twilio_inbound( + State(state): State>, + headers: HeaderMap, + Form(params): Form>, +) -> Result<(StatusCode, Json), (StatusCode, Json)> { + let (Some(auth_token), Some(webhook_url)) = ( + state.config.twilio_auth_token.as_deref(), + state.config.twilio_webhook_url.as_deref(), + ) else { + // Not configured — fail closed rather than skip validation. + return Err(rejected()); + }; + + let signature = headers + .get("X-Twilio-Signature") + .and_then(|v| v.to_str().ok()) + .unwrap_or(""); + + if !validate_signature(webhook_url, ¶ms, auth_token, signature) { + return Err(rejected()); + } + + let from = params.get("From").map(String::as_str).unwrap_or(""); + if from.is_empty() { + return Err(rejected()); + } + + let identity = state + .db + .get_sms_identity(from) + .await + .map_err(|_| internal_error("sms identity lookup failed"))?; + + if !identity.is_some_and(|i| i.allowed) { + return Err(rejected()); + } + + // TODO(slice 8+): synthesize a KIND_STREAM_MESSAGE_V2 event into the + // community's SMS-inbox channel here instead of just acknowledging. + Ok(( + StatusCode::OK, + Json(serde_json::json!({ "status": "accepted" })), + )) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn rejected_is_forbidden_and_generic() { + let (status, Json(body)) = rejected(); + assert_eq!(status, StatusCode::FORBIDDEN); + assert_eq!(body["error"], "request not accepted"); + } +} diff --git a/crates/buzz-relay/src/config.rs b/crates/buzz-relay/src/config.rs index 037c6b1dd3d..72a86a5d1cf 100644 --- a/crates/buzz-relay/src/config.rs +++ b/crates/buzz-relay/src/config.rs @@ -269,6 +269,20 @@ pub struct Config { /// Used to authenticate internal policy endpoint requests. pub git_hook_hmac_secret: String, + /// Twilio Auth Token, used to validate `X-Twilio-Signature` on inbound + /// SMS webhook requests (`POST /hooks/sms/inbound`). When unset, the + /// inbound SMS route rejects every request — there is no permissive + /// fallback, unlike `git_hook_hmac_secret`'s auto-generated default, + /// because this secret must match Twilio's own console configuration + /// exactly and a locally-generated substitute could never do that. + pub twilio_auth_token: Option, + /// The exact, full URL configured in the Twilio console for the inbound + /// SMS webhook (e.g. `https://relay.example.com/hooks/sms/inbound`). + /// Twilio signs requests against this exact string, so it's taken from + /// config rather than reconstructed from request headers (which can + /// disagree with what Twilio actually signed behind a proxy/LB). + pub twilio_webhook_url: Option, + /// Descriptor key identifier accepted in kind:30350 `exec` tags. pub push_executor_key_id: String, /// Exact HTTPS gateway endpoint used to submit client-authorized APNs delivery capabilities. @@ -986,6 +1000,13 @@ impl Config { )); } + let twilio_auth_token = std::env::var("TWILIO_AUTH_TOKEN") + .ok() + .filter(|s| !s.trim().is_empty()); + let twilio_webhook_url = std::env::var("TWILIO_WEBHOOK_URL") + .ok() + .filter(|s| !s.trim().is_empty()); + Ok(Self { bind_addr, database_url, @@ -1034,6 +1055,8 @@ impl Config { git_max_repos_per_pubkey, git_max_concurrent_ops, git_hook_hmac_secret, + twilio_auth_token, + twilio_webhook_url, push_executor_key_id, push_gateway_delivery_url, push_gateway_timeout, diff --git a/crates/buzz-relay/src/lib.rs b/crates/buzz-relay/src/lib.rs index 314adad92e0..0511a4c37e9 100644 --- a/crates/buzz-relay/src/lib.rs +++ b/crates/buzz-relay/src/lib.rs @@ -46,6 +46,8 @@ pub mod telemetry; pub mod tenant; /// Relay-side tunnel session directory and routing. pub mod tunnel; +/// Twilio inbound request signature validation. +pub mod twilio_auth; /// Webhook secret generation and constant-time comparison. pub mod webhook_secret; /// Workflow action sink — relay-side implementation of [`buzz_workflow::ActionSink`]. diff --git a/crates/buzz-relay/src/router.rs b/crates/buzz-relay/src/router.rs index 1dce66e91e4..481b9e05b00 100644 --- a/crates/buzz-relay/src/router.rs +++ b/crates/buzz-relay/src/router.rs @@ -127,6 +127,8 @@ pub fn build_router(state: Arc) -> Router { ) // Webhook trigger (secret-authenticated, no NIP-98) .route("/hooks/{id}", post(api::bridge::workflow_webhook)) + // Twilio inbound SMS (Twilio-signature-authenticated, no NIP-98) + .route("/hooks/sms/inbound", post(api::sms::twilio_inbound)) // Mesh demo echo probe — testbed-only; 404 unless BUZZ_MESH=on and // BUZZ_MESH_DEMO_ECHO=on (see api::mesh_demo). .route("/_mesh/demo/echo", post(api::mesh_demo::demo_echo)) diff --git a/crates/buzz-relay/src/twilio_auth.rs b/crates/buzz-relay/src/twilio_auth.rs new file mode 100644 index 00000000000..4c9ec4c52d9 --- /dev/null +++ b/crates/buzz-relay/src/twilio_auth.rs @@ -0,0 +1,156 @@ +//! Twilio request signature validation. +//! +//! Twilio signs every webhook request it sends: HMAC-SHA1 over the full +//! request URL with each POST parameter's key+value appended directly +//! (sorted by key, no separators), keyed by the account's Auth Token, +//! base64-encoded into the `X-Twilio-Signature` header. +//! +//! This is the sole trust boundary for inbound SMS — Twilio cannot hold a +//! Nostr key, so NIP-42/NIP-98 don't apply here. A request that fails this +//! check must be rejected before any relay event is synthesized from it. +//! +//! Reference: + +use std::collections::BTreeMap; + +use hmac::{Hmac, KeyInit, Mac}; +use sha1::Sha1; + +type HmacSha1 = Hmac; + +/// Build the string Twilio signs: the request URL followed by each POST +/// parameter's key and value concatenated directly, in ascending key order. +/// +/// `params` must already be the parsed (not URL-encoded) form values — a +/// `BTreeMap` is used by the caller specifically so this sees keys in sorted +/// order without needing to sort here. +fn signing_string(url: &str, params: &BTreeMap) -> String { + let mut s = String::with_capacity( + url.len() + params.iter().map(|(k, v)| k.len() + v.len()).sum::(), + ); + s.push_str(url); + for (k, v) in params { + s.push_str(k); + s.push_str(v); + } + s +} + +/// Validate an inbound Twilio request's `X-Twilio-Signature` header. +/// +/// `url` must be the exact full URL Twilio was configured to POST to +/// (scheme + host + path, matching what Twilio itself signed — a mismatched +/// scheme or a stray trailing slash produces a signature mismatch even for a +/// genuine Twilio request). `provided_signature_b64` is the raw header value. +/// +/// Returns `false` for a malformed (non-base64) signature as well as a +/// genuine mismatch — callers should map both to the same rejection so the +/// endpoint doesn't become an oracle for probing signature validity. +pub fn validate_signature( + url: &str, + params: &BTreeMap, + auth_token: &str, + provided_signature_b64: &str, +) -> bool { + use base64::engine::general_purpose::STANDARD; + use base64::Engine; + + let Ok(provided_bytes) = STANDARD.decode(provided_signature_b64) else { + return false; + }; + let msg = signing_string(url, params); + let Ok(mut mac) = HmacSha1::new_from_slice(auth_token.as_bytes()) else { + // HMAC accepts any key length, so this is unreachable in practice — + // kept as a graceful `false` rather than `.expect()` per the + // no-unwrap-in-prod-paths convention. + return false; + }; + mac.update(msg.as_bytes()); + mac.verify_slice(&provided_bytes).is_ok() +} + +#[cfg(test)] +mod tests { + use super::*; + + /// Test vector independently computed two ways outside this codebase + /// (`openssl dgst -sha1 -hmac ... -binary | openssl base64`, and Python's + /// `hmac`/`hashlib` standard library) — both agree on + /// `driuZp25+vgewC8363biLUvmnmI=` for this input, so this is a real + /// verification of the algorithm, not a self-fulfilling assertion. + fn known_good() -> ( + &'static str, + BTreeMap, + &'static str, + &'static str, + ) { + let url = "https://example.com/hooks/sms/inbound"; + let mut params = BTreeMap::new(); + params.insert("From".to_string(), "+15551234567".to_string()); + params.insert("Body".to_string(), "Hello".to_string()); + let auth_token = "test-auth-token"; + let expected_sig = "driuZp25+vgewC8363biLUvmnmI="; + (url, params, auth_token, expected_sig) + } + + #[test] + fn signing_string_sorts_params_and_concatenates_without_separators() { + let (url, params, _, _) = known_good(); + assert_eq!( + signing_string(url, ¶ms), + "https://example.com/hooks/sms/inboundBodyHelloFrom+15551234567" + ); + } + + #[test] + fn validate_signature_accepts_independently_computed_vector() { + let (url, params, auth_token, expected_sig) = known_good(); + assert!(validate_signature(url, ¶ms, auth_token, expected_sig)); + } + + #[test] + fn validate_signature_rejects_wrong_auth_token() { + let (url, params, _, expected_sig) = known_good(); + assert!(!validate_signature( + url, + ¶ms, + "wrong-token", + expected_sig + )); + } + + #[test] + fn validate_signature_rejects_tampered_params() { + let (url, mut params, auth_token, expected_sig) = known_good(); + params.insert("Body".to_string(), "Goodbye".to_string()); + assert!(!validate_signature(url, ¶ms, auth_token, expected_sig)); + } + + #[test] + fn validate_signature_rejects_wrong_url() { + let (_, params, auth_token, expected_sig) = known_good(); + assert!(!validate_signature( + "https://example.com/hooks/sms/other", + ¶ms, + auth_token, + expected_sig + )); + } + + #[test] + fn validate_signature_rejects_malformed_base64() { + let (url, params, auth_token, _) = known_good(); + assert!(!validate_signature( + url, + ¶ms, + auth_token, + "not-valid-base64!!" + )); + } + + #[test] + fn validate_signature_rejects_empty_signature() { + let (url, params, auth_token, _) = known_good(); + assert!(!validate_signature(url, ¶ms, auth_token, "")); + } +} diff --git a/docs/wayfinder/map-external-project-sms-integration.md b/docs/wayfinder/map-external-project-sms-integration.md index b67a2af7c31..bc680c50e6e 100644 --- a/docs/wayfinder/map-external-project-sms-integration.md +++ b/docs/wayfinder/map-external-project-sms-integration.md @@ -8,6 +8,50 @@ Let a message arriving in Buzz (from a channel or, eventually, an SMS) trigger a **Corrected local path (verified 2026-08-12):** `E:/Projects/buildbid` itself is NOT a git checkout — it's a parent folder holding many separate clones/worktrees (`bidcraft-repo`, `bidcraft-repo-claude2`, `bidcraft-cruft-cleanup`, etc.), each with its own `.git`. The canonical checkout is **`E:/Projects/buildbid/bidcraft-repo`** (`origin` = `git@github.com:mfethe1/bidcraft.git`, confirmed via `git remote -v`). Any `--project-paths` entry for `bidcraft` must point here, not at the bare `E:/Projects/buildbid` folder. +## Privacy (bidcraft/construct-pro are private repos — verified 2026-08-12) + +Nothing in this design requires making either repo public. Repo *content* never +needs to transit a public surface — the harness just points an agent's working +directory at your existing private local clone. But two Buzz primitives used +by this design default to discoverable, and must be overridden explicitly: + +1. **NIP-MP project visibility defaults to `listed`.** `ProjectVisibility::Listed` + is documented as "Project appears in public listings **(default)**" + (`crates/buzz-cli/src/lib.rs:1262-1263`). Creating a `bidcraft` or + `construct-pro` project via `buzz projects create`/`update` without + `--visibility unlisted` makes the *existence* of that project binding + discoverable to anyone who can browse projects on the relay — not the + code, but the fact that you have a project by that name wired up. + **Action: always pass `--visibility unlisted` when creating or updating + the bidcraft/construct-pro project bindings.** +2. **Channel visibility defaults to `open`.** `default_visibility()` in + `crates/buzz-cli/src/commands/channel_templates.rs:60` returns `"open"` + — "searchable, anyone can join without an invite" + (`ChannelVisibility::Open`, `crates/buzz-core/src/channel.rs:20-27`), vs. + `Private` ("hidden, requires an invite"). Any channel bound to bidcraft/ + construct-pro, and the future SMS-inbox channel, **must be created with + `visibility: private`**, not left at the open default. +3. **"Unlisted"/"private" are discoverability controls, not encryption.** + Events are still stored plaintext in the relay's own database. The real + privacy boundary is who can authenticate to *this* relay/community at + all (NIP-42 auth, `require_auth_token`, community host-scoping) — these + two settings only prevent casual browse-discovery by users who already + have access to the relay, they don't add a second layer of access + control on top of it. + +**Latest code, not a stale snapshot:** dispatch already points at your live +local checkout, so it always sees whatever's currently on disk. Nothing here +forks or mirrors the repo into a Buzz-controlled copy. The one open gap: +`resolve_effective_cwd`/`build_channel_cwd_map` (`crates/buzz-acp/src/pool.rs`, +`config.rs`) don't currently `git fetch`/pull before dispatch, so a checkout +that's fallen behind its remote (e.g. someone else pushed) won't be +automatically refreshed — tracked as a new task (see plan below). + +**New capability lands in Buzz itself:** the project→path and channel→project +config added in slices 1+2 is first-class `buzz-acp`/`buzz-cli` config, not a +one-off script bolted on the side — any future project gets the same +treatment for free. + ## Part A: Buzz ↔ bidcraft/construct-pro agent dispatch **Project representation — reuse `KIND_PROJECT` (30621, NIP-MP), don't overload community.** Community is the relay-tenant boundary (one relay/host); project is already the right-sized primitive for "which repo(s) does this dispatch target" — it groups `kind:30617` git-repo announcements via `a`-tag coordinates and already has CLI surface (`buzz-cli/src/lib.rs:215-220`, `projects`/`repos` subcommands). No new event kind needed for project scoping itself. @@ -65,12 +109,13 @@ Let a message arriving in Buzz (from a channel or, eventually, an SMS) trigger a 3. **`buzz-cli` project-binding subcommand** → verify: run the new subcommand against a running relay, confirm it reads/writes the channel↔project binding and that a subsequent dispatch (slice 2) picks it up without a harness restart, if config is live-reloaded — otherwise document the restart requirement. 4. **First live external dispatch — bidcraft/BuildBid (read-only)** → verify: bind a test channel to bidcraft's project id (`E:/Projects/buildbid/bidcraft-repo`), post a message asking the agent to summarize open work, confirm the response reflects real repo state. **Deferred pending a supervised session** — needs a running relay + real harness process, a bigger step than a code change. 5. **First live external dispatch — construct-pro (code change)** → verify: bind a channel to construct-pro, dispatch against WF-01 (open, unblocked, smallest ticket per `docs/wayfinder/map.md`), confirm the agent works inside a fresh `.claude/worktrees/buzz-` worktree and produces a diff addressing the ticket's cited evidence lines. **Deferred pending a supervised session** — this dispatches a real agent that writes into a shared, actively-worked repo (multiple other agents have branches in flight there); not something to run unattended. -6. **`sms_identities` migration + allow-list enforcement** → verify: run the migration locally, hit `/hooks/sms/inbound` with a forged Twilio-shaped POST for an unlisted number, confirm 403 and no event written; add an allowed row and confirm the same request now produces a `KIND_STREAM_MESSAGE_V2` event with correct tags. -7. **Twilio signature validation** → verify: send a request with a wrong/missing `X-Twilio-Signature`, confirm 403; send one with a correctly computed signature (using a test Auth Token), confirm it passes and reaches the allow-list check. -8. **Operator persona — fast path (default_project set)** → verify: seed an `sms_identities` row with `default_project` = bidcraft or construct-pro, post a synthetic inbound-SMS event into the SMS-inbox channel, confirm the persona dispatches via the Part A path (slice 2) into the correct project's `cwd`. +6. **`sms_identities` migration + allow-list enforcement** ✅ done — `migrations/0031_sms_identities.sql`, `crates/buzz-db/src/sms.rs`. Not yet applied against a live Postgres (no infra spun up this pass). +7. **Twilio signature validation + inbound webhook route** ✅ done — `crates/buzz-relay/src/twilio_auth.rs` (HMAC-SHA1 validation, test vectors independently cross-checked via `openssl` and Python outside this codebase), `crates/buzz-relay/src/api/sms.rs` wired at `POST /hooks/sms/inbound`. Verified: unit tests pass, `cargo clippy -D warnings` clean. **Stops at "allowed → 200 OK acknowledged"** — does not yet synthesize a `KIND_STREAM_MESSAGE_V2` event; that lands with slice 8. +8. **Operator persona — fast path (default_project set)** → verify: seed an `sms_identities` row with `default_project` = bidcraft or construct-pro, post a synthetic inbound-SMS event into the SMS-inbox channel, confirm the persona dispatches via the Part A path (slice 2) into the correct project's `cwd`. Also where `sms.rs`'s handler grows the actual `KIND_STREAM_MESSAGE_V2` event synthesis it currently stops short of. 9. **Operator persona — ambiguous path** → verify: seed a row with `default_project = NULL`, post an inbound event, confirm the persona posts a disambiguation reply event instead of dispatching. 10. **Outbound SMS sink** → verify: with real (or Twilio test-credential sandbox) SID/token configured, post a reply event tagged back to an inbound message, confirm `sms_sink.rs` calls Twilio's API and the test phone number (or Twilio's magic test number) receives/logs the outbound message. 11. **End-to-end SMS → project dispatch → reply** → verify: full loop — real inbound SMS from an allow-listed number with a clear `default_project`, agent dispatches and completes, outbound reply SMS arrives back at the sending number. +12. **Fetch-before-dispatch freshness check (new — from privacy/freshness review)** → verify: point `--project-paths` at a checkout that's behind its remote, dispatch into it, confirm the harness either fast-forwards it first or at minimum logs a clear staleness warning rather than silently working off outdated code. ## Open questions for Michael From 57a7e2eba9f763e81df73d2e8d6344b736a20254 Mon Sep 17 00:00:00 2001 From: Michael Feth Date: Wed, 12 Aug 2026 23:54:39 -0400 Subject: [PATCH 06/34] feat(relay): synthesize a real relay event from an allowed inbound SMS Replaces the prior "allowed -> 200 OK, stop" placeholder in crates/buzz-relay/src/api/sms.rs with real event synthesis: builds a KIND_STREAM_MESSAGE (kind 9) event tagged h (SMS-inbox channel), sms_from, sms_sid, p (sender's linked pubkey when known), and project (the sender's default_project when set), signs it with the relay's own keypair, and inserts it via Db::insert_event_with_thread_metadata. Uses KIND_STREAM_MESSAGE (9), not KIND_STREAM_MESSAGE_V2 (40002) as the design doc originally guessed -- corrected after checking buzz-acp's default Mentions-mode subscribe filter (resolve_channel_filters in buzz-acp/src/config.rs), which only wakes agents on kind 9. A V2-only event would never trigger the sms-operator persona meant to read it. Mirrors workflow_sink.rs's proven pattern for a relay-authored message (same signing/tagging/insert approach), rather than inventing a new one. New crates/buzz-relay/src/config.rs field: twilio_sms_inbox_channel (env TWILIO_SMS_INBOX_CHANNEL) -- a single global inbox channel for v1; multi-community routing is a documented future enhancement. Verified: build_tags() is a pure, unit-tested function (6 tests covering h/sms_from/sms_sid presence, p/project inclusion/omission, and malformed-pubkey handling) -- cargo clippy -D warnings and cargo test both confirmed clean with explicitly-checked exit codes, not piped. NOT verified: the actual DB insert path (insert_event_with_thread_metadata) against a live Postgres -- no DB infra was available this session. Signed-off-by: Michael Feth --- crates/buzz-relay/src/api/sms.rs | 164 +++++++++++++++++++++++++++++-- crates/buzz-relay/src/config.rs | 10 ++ 2 files changed, 165 insertions(+), 9 deletions(-) diff --git a/crates/buzz-relay/src/api/sms.rs b/crates/buzz-relay/src/api/sms.rs index 71c3549d922..a0a19b06c4b 100644 --- a/crates/buzz-relay/src/api/sms.rs +++ b/crates/buzz-relay/src/api/sms.rs @@ -8,9 +8,14 @@ //! the exact same rejection as a signature failure, so a prober can't //! distinguish "bad signature" from "signature fine, number not allowed"). //! -//! This slice stops at "allowed → 200 OK, acknowledged". Turning an allowed -//! message into a `KIND_STREAM_MESSAGE_V2` relay event, and the sms-operator -//! persona that reads it, are follow-up slices. +//! On success, synthesizes a `KIND_STREAM_MESSAGE` (kind 9) event into the +//! configured SMS-inbox channel — the same kind [`crate::workflow_sink`] uses +//! to post a system-authored message, not `KIND_STREAM_MESSAGE_V2`: buzz-acp's +//! default (Mentions-mode) subscribe filter only wakes agents on kind 9 (see +//! `buzz-acp/src/config.rs`'s `resolve_channel_filters`), so a V2-only event +//! would never trigger the sms-operator persona that's meant to read it. +//! The sms-operator persona itself (reading this channel and dispatching) is +//! a follow-up slice — this only gets the message onto the relay correctly. use std::collections::BTreeMap; use std::sync::Arc; @@ -18,6 +23,12 @@ use std::sync::Arc; use axum::extract::{Form, State}; use axum::http::{HeaderMap, StatusCode}; use axum::response::Json; +use chrono::Utc; +use nostr::{EventBuilder, Kind, Tag}; + +use buzz_core::kind::KIND_STREAM_MESSAGE; +use buzz_db::event::ThreadMetadataParams; +use buzz_db::sms::SmsIdentity; use crate::state::AppState; use crate::twilio_auth::validate_signature; @@ -30,9 +41,36 @@ fn rejected() -> (StatusCode, Json) { api_error(StatusCode::FORBIDDEN, "request not accepted") } +/// Build the tag set for a synthesized inbound-SMS event: `h` scopes it to +/// the SMS-inbox channel, `p` attributes it to the sender's linked pubkey +/// when known, `sms_from`/`sms_sid` carry the Twilio identifiers, and +/// `project` carries the sender's routing default when set (read by the +/// sms-operator persona's fast path — see module docs). +fn build_tags( + channel_id: uuid::Uuid, + identity: &SmsIdentity, + message_sid: &str, +) -> Result, String> { + let mut tags = vec![ + Tag::parse(["h", &channel_id.to_string()]).map_err(|e| format!("h tag: {e}"))?, + Tag::parse(["sms_from", &identity.phone_number]) + .map_err(|e| format!("sms_from tag: {e}"))?, + Tag::parse(["sms_sid", message_sid]).map_err(|e| format!("sms_sid tag: {e}"))?, + ]; + if let Some(pubkey_bytes) = &identity.linked_pubkey { + if let Ok(pk) = nostr::PublicKey::from_slice(pubkey_bytes) { + tags.push(Tag::parse(["p", &pk.to_hex()]).map_err(|e| format!("p tag: {e}"))?); + } + } + if let Some(project) = &identity.default_project { + tags.push(Tag::parse(["project", project]).map_err(|e| format!("project tag: {e}"))?); + } + Ok(tags) +} + /// Handle an inbound Twilio SMS webhook POST. Validates the request -/// signature, then requires the sender's phone number be allow-listed; -/// on success, currently just acknowledges (event synthesis is a follow-up). +/// signature, requires the sender's phone number be allow-listed, then +/// publishes the message into the SMS-inbox channel. pub async fn twilio_inbound( State(state): State>, headers: HeaderMap, @@ -66,12 +104,54 @@ pub async fn twilio_inbound( .await .map_err(|_| internal_error("sms identity lookup failed"))?; - if !identity.is_some_and(|i| i.allowed) { + let Some(identity) = identity.filter(|i| i.allowed) else { return Err(rejected()); - } + }; + + let Some(channel_id) = state.config.twilio_sms_inbox_channel else { + // No inbox channel configured — reject rather than silently drop. + return Err(internal_error("sms inbox channel not configured")); + }; + + let body = params.get("Body").cloned().unwrap_or_default(); + let message_sid = params.get("MessageSid").map(String::as_str).unwrap_or(""); + + let tags = build_tags(channel_id, &identity, message_sid) + .map_err(|e| internal_error(&format!("sms event tag build failed: {e}")))?; + + let event = EventBuilder::new(Kind::from(KIND_STREAM_MESSAGE as u16), &body) + .tags(tags) + .sign_with_keys(&state.relay_keypair) + .map_err(|e| internal_error(&format!("sms event signing failed: {e}")))?; + + let event_created_at = { + let ts = event.created_at.as_secs() as i64; + chrono::DateTime::from_timestamp(ts, 0).unwrap_or_else(Utc::now) + }; + let event_id_bytes = event.id.as_bytes().to_vec(); + let thread_meta = Some(ThreadMetadataParams { + event_id: &event_id_bytes, + event_created_at, + channel_id, + parent_event_id: None, + parent_event_created_at: None, + root_event_id: None, + root_event_created_at: None, + depth: 0, + broadcast: false, + }); + + state + .db + .insert_event_with_thread_metadata( + identity.community_id, + &event, + Some(channel_id), + thread_meta, + ) + .await + .map_err(|e| internal_error(&format!("sms event insert failed: {e}")))?; - // TODO(slice 8+): synthesize a KIND_STREAM_MESSAGE_V2 event into the - // community's SMS-inbox channel here instead of just acknowledging. Ok(( StatusCode::OK, Json(serde_json::json!({ "status": "accepted" })), @@ -88,4 +168,70 @@ mod tests { assert_eq!(status, StatusCode::FORBIDDEN); assert_eq!(body["error"], "request not accepted"); } + + fn test_identity() -> SmsIdentity { + SmsIdentity { + phone_number: "+15551234567".to_string(), + community_id: buzz_core::CommunityId::from_uuid(uuid::Uuid::new_v4()), + allowed: true, + linked_pubkey: None, + default_project: None, + } + } + + fn tag_value<'a>(tags: &'a [Tag], name: &str) -> Option<&'a str> { + tags.iter() + .find(|t| t.as_slice().first().map(String::as_str) == Some(name)) + .and_then(|t| t.as_slice().get(1)) + .map(String::as_str) + } + + #[test] + fn build_tags_includes_h_from_and_sid() { + let channel_id = uuid::Uuid::new_v4(); + let identity = test_identity(); + let tags = build_tags(channel_id, &identity, "SM123").unwrap(); + + assert_eq!( + tag_value(&tags, "h"), + Some(channel_id.to_string()).as_deref() + ); + assert_eq!(tag_value(&tags, "sms_from"), Some("+15551234567")); + assert_eq!(tag_value(&tags, "sms_sid"), Some("SM123")); + } + + #[test] + fn build_tags_omits_p_and_project_when_unset() { + let tags = build_tags(uuid::Uuid::new_v4(), &test_identity(), "SM123").unwrap(); + assert_eq!(tag_value(&tags, "p"), None); + assert_eq!(tag_value(&tags, "project"), None); + } + + #[test] + fn build_tags_includes_project_when_default_project_set() { + let mut identity = test_identity(); + identity.default_project = Some("bidcraft".to_string()); + let tags = build_tags(uuid::Uuid::new_v4(), &identity, "SM123").unwrap(); + assert_eq!(tag_value(&tags, "project"), Some("bidcraft")); + } + + #[test] + fn build_tags_includes_p_when_linked_pubkey_valid() { + let mut identity = test_identity(); + let keys = nostr::Keys::generate(); + identity.linked_pubkey = Some(keys.public_key().to_bytes().to_vec()); + let tags = build_tags(uuid::Uuid::new_v4(), &identity, "SM123").unwrap(); + assert_eq!( + tag_value(&tags, "p"), + Some(keys.public_key().to_hex()).as_deref() + ); + } + + #[test] + fn build_tags_omits_p_when_linked_pubkey_malformed() { + let mut identity = test_identity(); + identity.linked_pubkey = Some(vec![1, 2, 3]); // not a valid 32-byte pubkey + let tags = build_tags(uuid::Uuid::new_v4(), &identity, "SM123").unwrap(); + assert_eq!(tag_value(&tags, "p"), None); + } } diff --git a/crates/buzz-relay/src/config.rs b/crates/buzz-relay/src/config.rs index 72a86a5d1cf..3e701fc1a29 100644 --- a/crates/buzz-relay/src/config.rs +++ b/crates/buzz-relay/src/config.rs @@ -6,6 +6,7 @@ use std::time::Duration; use sha2::{Digest, Sha256}; use thiserror::Error; use tracing::warn; +use uuid::Uuid; /// Default maximum inbound WebSocket frame size in bytes. /// @@ -282,6 +283,11 @@ pub struct Config { /// config rather than reconstructed from request headers (which can /// disagree with what Twilio actually signed behind a proxy/LB). pub twilio_webhook_url: Option, + /// UUID of the channel inbound SMS messages are posted into. v1 + /// simplification: one global inbox channel for the whole relay, not + /// per-community — multi-community SMS routing is a future enhancement. + /// Inbound SMS is rejected (rather than silently dropped) when unset. + pub twilio_sms_inbox_channel: Option, /// Descriptor key identifier accepted in kind:30350 `exec` tags. pub push_executor_key_id: String, @@ -1006,6 +1012,9 @@ impl Config { let twilio_webhook_url = std::env::var("TWILIO_WEBHOOK_URL") .ok() .filter(|s| !s.trim().is_empty()); + let twilio_sms_inbox_channel = std::env::var("TWILIO_SMS_INBOX_CHANNEL") + .ok() + .and_then(|s| Uuid::parse_str(s.trim()).ok()); Ok(Self { bind_addr, @@ -1057,6 +1066,7 @@ impl Config { git_hook_hmac_secret, twilio_auth_token, twilio_webhook_url, + twilio_sms_inbox_channel, push_executor_key_id, push_gateway_delivery_url, push_gateway_timeout, From d5e5b3100afda9f3e76191e1628b145615068329 Mon Sep 17 00:00:00 2001 From: Michael Feth Date: Wed, 12 Aug 2026 23:55:35 -0400 Subject: [PATCH 07/34] docs(wayfinder): record event synthesis landing + re-scope task 8 Slice 7's event synthesis is real and unit-tested now (see c3a70ce). Re-scopes task 8 precisely: the sms-operator persona is a real .persona.md pack (not .toml), and buzz-acp needs new per-message project-tag routing since the SMS-inbox is one shared channel across all senders -- today's --channel-projects binding is per-channel only, not per-message. Signed-off-by: Michael Feth --- docs/wayfinder/map-external-project-sms-integration.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/wayfinder/map-external-project-sms-integration.md b/docs/wayfinder/map-external-project-sms-integration.md index bc680c50e6e..536f095b833 100644 --- a/docs/wayfinder/map-external-project-sms-integration.md +++ b/docs/wayfinder/map-external-project-sms-integration.md @@ -110,8 +110,9 @@ treatment for free. 4. **First live external dispatch — bidcraft/BuildBid (read-only)** → verify: bind a test channel to bidcraft's project id (`E:/Projects/buildbid/bidcraft-repo`), post a message asking the agent to summarize open work, confirm the response reflects real repo state. **Deferred pending a supervised session** — needs a running relay + real harness process, a bigger step than a code change. 5. **First live external dispatch — construct-pro (code change)** → verify: bind a channel to construct-pro, dispatch against WF-01 (open, unblocked, smallest ticket per `docs/wayfinder/map.md`), confirm the agent works inside a fresh `.claude/worktrees/buzz-` worktree and produces a diff addressing the ticket's cited evidence lines. **Deferred pending a supervised session** — this dispatches a real agent that writes into a shared, actively-worked repo (multiple other agents have branches in flight there); not something to run unattended. 6. **`sms_identities` migration + allow-list enforcement** ✅ done — `migrations/0031_sms_identities.sql`, `crates/buzz-db/src/sms.rs`. Not yet applied against a live Postgres (no infra spun up this pass). -7. **Twilio signature validation + inbound webhook route** ✅ done — `crates/buzz-relay/src/twilio_auth.rs` (HMAC-SHA1 validation, test vectors independently cross-checked via `openssl` and Python outside this codebase), `crates/buzz-relay/src/api/sms.rs` wired at `POST /hooks/sms/inbound`. Verified: unit tests pass, `cargo clippy -D warnings` clean. **Stops at "allowed → 200 OK acknowledged"** — does not yet synthesize a `KIND_STREAM_MESSAGE_V2` event; that lands with slice 8. -8. **Operator persona — fast path (default_project set)** → verify: seed an `sms_identities` row with `default_project` = bidcraft or construct-pro, post a synthetic inbound-SMS event into the SMS-inbox channel, confirm the persona dispatches via the Part A path (slice 2) into the correct project's `cwd`. Also where `sms.rs`'s handler grows the actual `KIND_STREAM_MESSAGE_V2` event synthesis it currently stops short of. +7. **Twilio signature validation + inbound webhook route** ✅ done — `crates/buzz-relay/src/twilio_auth.rs` (HMAC-SHA1 validation, test vectors independently cross-checked via `openssl` and Python outside this codebase), `crates/buzz-relay/src/api/sms.rs` wired at `POST /hooks/sms/inbound`. Verified: unit tests pass, `cargo clippy -D warnings` clean. +7b. **Event synthesis** ✅ done — an allowed message now produces a real `KIND_STREAM_MESSAGE` (kind 9, **not** `KIND_STREAM_MESSAGE_V2` as first guessed — corrected after checking that buzz-acp's default Mentions-mode subscribe filter, `resolve_channel_filters` in `buzz-acp/src/config.rs`, only wakes agents on kind 9; a V2-only event would never trigger the persona meant to read it) tagged `h`/`sms_from`/`sms_sid`/`p`/`project`, signed by the relay's own keypair, mirroring `workflow_sink.rs`'s proven pattern. New `twilio_sms_inbox_channel` config (single global inbox channel, v1 simplification). Verified: `build_tags()` is a pure, unit-tested function (6 tests). **Not verified:** the actual DB insert (`insert_event_with_thread_metadata`) against a live Postgres — no DB infra available this session. +8. **Operator persona — fast path (default_project set)** → **re-scoped**: the event now carries a `project` tag when the sender's `default_project` is known, so the remaining work is (a) a real `sms-operator` **persona pack** — a `.persona.md` file with YAML frontmatter per `crates/buzz-persona/PERSONA_PACK_SPEC.md`, **not a `.toml`** as first guessed — subscribed to the SMS-inbox channel, and (b) new buzz-acp-side wiring: today's `resolve_effective_cwd`/`build_channel_cwd_map` only support a **static** per-channel project binding (`--channel-projects`), but the SMS-inbox is **one shared channel across all senders/projects** — routing needs to read the *per-message* `project` tag, not just the channel's static binding. This is a real, not-yet-designed extension, not a small follow-up. Verify: seed a row with `default_project` set, post a synthetic inbound-SMS event, confirm the persona dispatches via a per-message-tag-resolved `cwd` into the correct project. 9. **Operator persona — ambiguous path** → verify: seed a row with `default_project = NULL`, post an inbound event, confirm the persona posts a disambiguation reply event instead of dispatching. 10. **Outbound SMS sink** → verify: with real (or Twilio test-credential sandbox) SID/token configured, post a reply event tagged back to an inbound message, confirm `sms_sink.rs` calls Twilio's API and the test phone number (or Twilio's magic test number) receives/logs the outbound message. 11. **End-to-end SMS → project dispatch → reply** → verify: full loop — real inbound SMS from an allow-listed number with a clear `default_project`, agent dispatches and completes, outbound reply SMS arrives back at the sending number. From 403b98b0ce425daa762e3492a56f95c685669b33 Mon Sep 17 00:00:00 2001 From: Michael Feth Date: Thu, 13 Aug 2026 06:44:14 -0400 Subject: [PATCH 08/34] docs(wayfinder): record two blocking findings that invalidate the slice-8 plan Both found by an adversarial research pass, then independently re-verified against real code before recording. 1. Persona Packs are inert at runtime. buzz-acp never loads one: zero buzz_persona references in buzz-acp/src despite a declared Cargo dependency, no --pack/--persona flag, and a persona's subscribe: field is parsed but never mapped to subscribe config. Writing sms-operator.persona.md would validate cleanly and do nothing. Deploying an agent today means desktop personas.json or setting buzz-acp env vars directly. 2. Per-message project routing is blocked by session caching. Reading the project tag IS small (batch is in scope at pool.rs:1684), but pool.rs:1678 short-circuits to a cached session that never reaches the resolver, and cwd is immutable after session/new. A naive implementation routes the first message correctly then silently ignores the tag forever -- and passes a single-message test, the worst failure shape. Neither implemented, deliberately. Landing either half as originally scoped would look green and be wrong. Signed-off-by: Michael Feth --- .../map-external-project-sms-integration.md | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/docs/wayfinder/map-external-project-sms-integration.md b/docs/wayfinder/map-external-project-sms-integration.md index 536f095b833..ffa2d9a416e 100644 --- a/docs/wayfinder/map-external-project-sms-integration.md +++ b/docs/wayfinder/map-external-project-sms-integration.md @@ -118,6 +118,56 @@ treatment for free. 11. **End-to-end SMS → project dispatch → reply** → verify: full loop — real inbound SMS from an allow-listed number with a clear `default_project`, agent dispatches and completes, outbound reply SMS arrives back at the sending number. 12. **Fetch-before-dispatch freshness check (new — from privacy/freshness review)** → verify: point `--project-paths` at a checkout that's behind its remote, dispatch into it, confirm the harness either fast-forwards it first or at minimum logs a clear staleness warning rather than silently working off outdated code. +## ⚠ Two blocking findings (verified 2026-08-13) — these invalidate the original slice-8 plan + +Both were found by an adversarial research pass and then **independently re-verified against the +real code** before being recorded here. They change what "add this as a plugin" can mean today. + +### Finding 1: Persona Packs are inert at runtime — `buzz-acp` never loads one + +The pack *format* is real, well-specified (`PERSONA_PACK_SPEC.md`), and has a working +parser/validator in `buzz-persona` + `buzz pack validate`/`inspect` in the CLI. But **nothing +loads a pack at agent-run time.** + +Verified directly, not taken on report: +- `grep -rn "buzz_persona" crates/buzz-acp/src crates/buzz-acp/tests` → **zero hits**, despite + `crates/buzz-acp/Cargo.toml:22` declaring `buzz-persona = { path = "../buzz-persona" }`. It is + a dead Cargo edge. +- buzz-acp has **no** `--pack` / `--persona` flag and no `BUZZ_ACP_PERSONA_*` env var. +- `Config::persona_env_vars`'s doc comment claims it is "Populated from persona pack resolution" — + **that comment is false**; the only code that pushes into it is the Codex sandbox network var. +- A persona's `subscribe:` field survives parsing into `ResolvedPersona.subscribe`, but its only + consumer in the whole repo is the `buzz pack inspect` printout — the documented mapping to + `Config.subscribe_mode`/`channels_override` **is never performed**. +- Spec §11's distribution surface (`buzz pack ... --output`, `buzz install`, `pack.lock`, + `.buzzpack.sha256`, `~/.buzz/packs/`) does not exist; only `validate` and `inspect` are wired. + +**Consequence:** writing `sms-operator.persona.md` would produce a file that validates cleanly and +does nothing. `buzz pack validate` printing "Valid." means *it parses*, not *it will run*. +**Deploying an agent today** = create it in the desktop app (`personas.json`), or set +`BUZZ_ACP_SYSTEM_PROMPT` / `BUZZ_ACP_MODEL` / env vars on the `buzz-acp` process directly. + +**Consequence for the "plugin" question:** the operator can be *authored* as a pack for future +portability, but a pack is **not a deployment mechanism** today. Making it one is its own project +(wire pack resolution into buzz-acp startup), not a step in this feature. + +### Finding 2: Per-message project routing is blocked by session caching + +Routing on a per-message `["project", …]` tag is *not* just "extend `resolve_effective_cwd`": +- The triggering event and its tags **are** in scope at the call site (`batch` is live and + un-moved at `pool.rs:1684`), so reading the tag is genuinely a small change. That part is fine. +- **But** `pool.rs:1678` short-circuits to a **cached session** and never reaches the resolver, and + a session's `cwd` is immutable after `session/new` (`acp.rs:653`). + +**Consequence:** a naive implementation routes correctly for the *first* message in a channel and +then silently ignores the project tag forever after — and it **passes a single-message test**, +which is the worst possible failure shape. Doing this properly needs a `session_cwd` field on +`SessionState` (`pool.rs:108-132`) plus pre-emptive invalidation when the resolved cwd changes, +touching both invalidation methods (`pool.rs:151-168`) which are load-bearing for core memory, +canvas, delivery state, and turn counters. + +**Status:** deliberately NOT implemented. Landing the tag-read alone would look green and be wrong. + ## Open questions for Michael - Should `linked_pubkey` be required for `allowed = true` rows, or is the shared "SMS relay" service pubkey acceptable for allow-listed-but-unlinked numbers? From 7b53d45a62fcfbad0f7c4fe210f549ca167521d7 Mon Sep 17 00:00:00 2001 From: Michael Feth Date: Thu, 13 Aug 2026 10:54:23 -0400 Subject: [PATCH 09/34] feat(acp): load persona packs at startup, route project per message Two blockers recorded in db2747d are now fixed. 1. Persona packs were inert -- buzz-acp declared buzz-persona as a Cargo dependency and never called it, so a pack validated cleanly and changed nothing. Adds --pack/--persona resolution at startup, mapping a ResolvedPersona onto Config (system prompt, model, title, runtime, instructions, env, subscribe). Explicit CLI/env values win over pack values via clap ArgMatches value_source, so a pack only supplies defaults. Implements the '#'-stripping for subscribe channel names that PERSONA_PACK_SPEC promised and no code did. A bad pack path is a hard startup error, never a silent no-op. 2. Per-message project routing was defeated by session caching: the cached-session short-circuit never reached the cwd resolver and a session's cwd is immutable after session/new, so a naive tag-read would route message 1 and silently ignore the tag forever -- while passing a single-message test. Adds resolve_turn_routing() + batch_project_tag() and session_cwds on SessionState, invalidating pre-emptively only when the resolved cwd actually changes, ordered before the core-memory and canvas/title blocks so a replacement session rebuilds them. Security: a project id now arrives from an event tag, i.e. attacker-influencable. It is only ever a lookup key into the operator-configured --project-paths map and is never joined into a path; unknown ids fall back to the channel binding. Verified by re-running the gates directly rather than trusting the build agents: 29 config::persona_pack_tests pass, 6 pool::tests:: routing_* pass (including routing_hostile_project_ids_cannot_escape_ the_configured_map, which drives 17 hostile ids -- traversal, absolute, UNC, null byte, $HOME/%USERPROFILE% -- and also asserts a legitimate id still resolves so it is not vacuously rejecting everything), and second_message_with_a_different_project_tag_creates_ a_session_in_the_new_cwd passes -- a three-turn test capturing real ACP wire traffic that would fail if routing regressed to first-message-only. cargo fmt --check and cargo clippy -D warnings both clean. Not verified: no live relay, harness, or agent process was run. Signed-off-by: Michael Feth --- crates/buzz-acp/src/config.rs | 1204 ++++++++++++++++++++++++++++++++- crates/buzz-acp/src/lib.rs | 7 + crates/buzz-acp/src/pool.rs | 932 ++++++++++++++++++++++++- 3 files changed, 2117 insertions(+), 26 deletions(-) diff --git a/crates/buzz-acp/src/config.rs b/crates/buzz-acp/src/config.rs index 12d3753ac21..3309c9731f2 100644 --- a/crates/buzz-acp/src/config.rs +++ b/crates/buzz-acp/src/config.rs @@ -4,7 +4,7 @@ //! Config file (TOML) for complex subscription rules. use std::collections::{HashMap, HashSet}; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use clap::Parser; use clap::ValueEnum; @@ -504,6 +504,82 @@ pub struct CliArgs { /// project's cwd instead of the harness's own working directory. #[arg(long, env = "BUZZ_ACP_CHANNEL_PROJECTS", value_delimiter = ',')] pub channel_projects: Option>, + + /// Directory of a persona pack (containing `.plugin/plugin.json`) whose + /// resolved persona supplies harness *defaults*. Every value a pack + /// provides loses to the same value given explicitly on the CLI or through + /// an env var. An unreadable or invalid pack is a hard startup error. + #[arg(long, env = "BUZZ_ACP_PACK")] + pub pack: Option, + + /// Which persona to load from `--pack`. Required when the pack declares + /// more than one persona; optional (and redundant) when it declares one. + #[arg(long, env = "BUZZ_ACP_PERSONA")] + pub persona: Option, +} + +/// Which `CliArgs` values the caller actually supplied, as opposed to values +/// clap filled in from a `default_value`. +/// +/// Four args carry defaults (`agent_command`, `agent_args`, `subscribe`, and +/// the `no_mention_filter` flag), so after parsing they are byte-identical +/// whether the operator set them or not. A persona pack must lose to an +/// operator-supplied value and win over a clap default, and that distinction +/// is only recoverable from [`clap::ArgMatches::value_source`] — hence this +/// struct rides alongside the parsed args. +/// +/// Every other pack-relevant arg is `Option`-typed with no default, so `None` +/// alone already proves "unset" and needs no entry here. +#[derive(Debug, Clone, Copy)] +pub struct ExplicitArgs { + /// `--agent-command` / `BUZZ_ACP_AGENT_COMMAND` was supplied. + pub agent_command: bool, + /// `--agent-args` / `BUZZ_ACP_AGENT_ARGS` was supplied. + pub agent_args: bool, + /// `--subscribe` / `BUZZ_ACP_SUBSCRIBE` was supplied. + pub subscribe: bool, + /// `--no-mention-filter` / `BUZZ_ACP_NO_MENTION_FILTER` was supplied. + pub no_mention_filter: bool, +} + +impl ExplicitArgs { + /// Treat every defaulted arg as operator-supplied. + /// + /// This is the fail-safe direction for callers that reach [`Config`] + /// without an [`clap::ArgMatches`] (notably `Config::from_args`): a pack + /// silently losing a value is recoverable, a pack silently overriding an + /// operator's explicit flag is not. + /// + /// Only `Config::from_args` needs it, and that entry point is test-only + /// today — production goes through `Config::from_cli`, which always has + /// real matches to read provenance from. + #[cfg(test)] + pub const ALL_EXPLICIT: Self = Self { + agent_command: true, + agent_args: true, + subscribe: true, + no_mention_filter: true, + }; + + /// Read the true provenance of each defaulted arg out of clap's matches. + /// + /// The ids are clap-derive's verbatim snake_case field names. Passing an + /// id that is not an argument panics in debug builds, so + /// `explicit_args_ids_match_cli_fields` pins them in a test. + fn from_matches(matches: &clap::ArgMatches) -> Self { + let explicit = |id: &str| { + !matches!( + matches.value_source(id), + None | Some(clap::parser::ValueSource::DefaultValue) + ) + }; + Self { + agent_command: explicit("agent_command"), + agent_args: explicit("agent_args"), + subscribe: explicit("subscribe"), + no_mention_filter: explicit("no_mention_filter"), + } + } } /// Merged NIP-01 subscription filter for a single channel. @@ -567,7 +643,15 @@ pub struct Config { /// Allowed `respond_to` modes. Empty = all modes allowed. pub allowed_respond_to: Vec, /// Per-persona env vars to inject at agent spawn time (e.g., GOOSE_PROVIDER, GOOSE_MODEL, BUZZ_AGENT_MODEL). - /// Populated from persona pack resolution. Empty when no pack is configured. + /// + /// Populated from persona pack resolution when `--pack` is set — these are + /// `ResolvedPersona::runtime_env_vars`, appended *before* the generated + /// `CODEX_CONFIG` entry so `build_codex_config_env` still sees a + /// pack-supplied `CODEX_CONFIG` as its merge base. Without `--pack` the vec + /// holds only that generated entry (or is empty for non-Codex agents). + /// + /// Operator env wins: `AcpClient::spawn` skips any key already present in + /// the harness's own environment. pub persona_env_vars: Vec<(String, String)>, /// Whether `codex_network_env()` successfully injected a `CODEX_CONFIG` entry into /// `persona_env_vars`. When true, `AcpClient::spawn` merges all `CODEX_CONFIG` entries @@ -601,6 +685,15 @@ pub struct Config { /// Resolved through `project_paths` at `PromptContext` build time to /// produce the effective per-channel cwd. pub channel_projects: HashMap, + /// Channel *names* the persona pack asked to subscribe to, verbatim from + /// the pack (leading `#` and all). Empty when no pack is configured, or + /// when `--channels` already pinned the subscription set. + /// + /// These cannot become `channels_override` here: that field is parsed as + /// UUIDs, so names must wait until channel discovery has run. `main()` + /// feeds them to [`resolve_subscribe_channels`] once the relay has + /// answered. See [`normalize_channel_name`] for the `#` rule. + pub pack_subscribe: Vec, } /// Maximum length, in characters, of a session title sent to the adapter. @@ -860,19 +953,155 @@ pub fn propagate_legacy_env_vars() { } } +/// Map a persona pack's ACP runtime id onto the agent binary buzz-acp spawns. +/// +/// Deliberately a closed match. Pack files are data, and falling through to +/// "use the runtime string as the command" would let a pack author name an +/// arbitrary executable that the harness then spawns at startup. +fn runtime_to_agent_command(runtime: &str) -> Result<&'static str, ConfigError> { + match runtime.trim() { + "goose" => Ok("goose"), + "buzz-agent" => Ok("buzz-agent"), + "claude" => Ok("claude-agent-acp"), + "codex" => Ok("codex-acp"), + other => Err(ConfigError::ConfigFile(format!( + "persona pack requests unknown runtime '{other}' \ + (supported: goose, claude, codex, buzz-agent)" + ))), + } +} + +/// Resolve the persona a `--pack` / `--persona` pair selects. +/// +/// Returns `Ok(None)` only when no pack was requested at all. Anything else — +/// missing directory, malformed manifest, unknown persona name, an ambiguous +/// multi-persona pack with no `--persona` — is a hard error. A pack that +/// "loads" into no persona would leave the harness running under a completely +/// different identity than the operator asked for, which is worse than +/// refusing to start. +fn resolve_pack_persona( + pack_dir: Option<&Path>, + persona_name: Option<&str>, +) -> Result, ConfigError> { + let Some(dir) = pack_dir else { + if let Some(name) = persona_name { + tracing::warn!(persona = %name, "--persona given without --pack; ignoring it"); + } + return Ok(None); + }; + + let pack = buzz_persona::resolve::resolve_pack(dir) + .map_err(|e| ConfigError::ConfigFile(format!("persona pack {}: {e}", dir.display())))?; + + let available = || { + pack.personas + .iter() + .map(|p| p.name.as_str()) + .collect::>() + .join(", ") + }; + + let persona = match persona_name { + Some(name) => pack + .personas + .iter() + .find(|p| p.name == name) + .ok_or_else(|| { + ConfigError::ConfigFile(format!( + "persona pack {}: no persona named '{}' (available: {})", + dir.display(), + name, + available() + )) + })? + .clone(), + None if pack.personas.len() == 1 => pack.personas[0].clone(), + None => { + return Err(ConfigError::ConfigFile(format!( + "persona pack {} declares {} personas — pass --pack together with \ + --persona to pick one (available: {})", + dir.display(), + pack.personas.len(), + available() + ))) + } + }; + + tracing::info!( + pack = %pack.id, + pack_version = %pack.version, + persona = %persona.name, + "loaded persona pack" + ); + Ok(Some(persona)) +} + impl Config { pub fn from_cli() -> Result { // Legacy env-var propagation is intentionally NOT done here. // Call `propagate_legacy_env_vars()` before the tokio runtime starts // (in the sync `fn main()` wrapper) — see Rust 2024 edition safety. - let args = CliArgs::parse(); - Self::from_args(args) + // + // Parsed through `ArgMatches` rather than `CliArgs::parse()` so + // `ExplicitArgs` can tell an operator-supplied value from a clap + // default — the distinction persona-pack precedence rests on. + use clap::{CommandFactory, FromArgMatches}; + let matches = CliArgs::command().get_matches(); + // Preserve clap's own usage output and exit code 2 on a bad flag, + // rather than reformatting it as a ConfigError. + let args = CliArgs::from_arg_matches(&matches).unwrap_or_else(|e| e.exit()); + Self::from_args_with_sources(args, ExplicitArgs::from_matches(&matches)) + } + + /// Build a `Config` from an argv vector exactly the way [`Config::from_cli`] + /// builds one from the real process args, including explicit-vs-default + /// tracking. + /// + /// This is the entry point to use when persona-pack precedence matters, + /// since `Config::from_args` cannot recover argument provenance and + /// conservatively assumes everything was explicit. + /// + /// # Errors + /// + /// Returns [`ConfigError::ConfigFile`] with clap's rendered message when + /// argv does not parse, plus every error `Config::from_args` can return. + #[cfg(test)] + pub fn try_from_argv(argv: I) -> Result + where + I: IntoIterator, + T: Into + Clone, + { + use clap::{CommandFactory, FromArgMatches}; + let matches = CliArgs::command() + .try_get_matches_from(argv) + .map_err(|e| ConfigError::ConfigFile(e.to_string()))?; + let args = CliArgs::from_arg_matches(&matches) + .map_err(|e| ConfigError::ConfigFile(e.to_string()))?; + Self::from_args_with_sources(args, ExplicitArgs::from_matches(&matches)) } /// Build a `Config` from already-parsed `CliArgs`. Separated from `from_cli()` so /// tests can construct `CliArgs` via `CliArgs::try_parse_from` and exercise the full /// validation path without going through process args. - pub fn from_args(mut args: CliArgs) -> Result { + /// + /// Argument provenance is unavailable here, so every defaulted arg is + /// treated as explicit (`ExplicitArgs::ALL_EXPLICIT`) and a persona pack + /// can never override one. Use `Config::try_from_argv` when that + /// distinction matters. + #[cfg(test)] + pub fn from_args(args: CliArgs) -> Result { + Self::from_args_with_sources(args, ExplicitArgs::ALL_EXPLICIT) + } + + /// Build a `Config` from parsed args plus the provenance of each defaulted + /// arg. + /// + /// `sources` decides persona-pack precedence: a pack value is applied only + /// where the operator did not speak. + pub fn from_args_with_sources( + mut args: CliArgs, + sources: ExplicitArgs, + ) -> Result { let keys = Keys::parse(&args.private_key)?; // Best-effort zeroize: overwrite the raw private key string to reduce // exposure via core dumps or heap inspection (#41). Without the `zeroize` @@ -881,12 +1110,16 @@ impl Config { .replace_range(.., &"0".repeat(args.private_key.len())); args.private_key.clear(); + // Load the pack once, before anything consults it. A bad pack fails + // startup here rather than degrading into a silently unconfigured run. + let persona = resolve_pack_persona(args.pack.as_deref(), args.persona.as_deref())?; + let system_prompt = if let Some(text) = args.system_prompt { Some(text) } else if let Some(ref path) = args.system_prompt_file { Some(std::fs::read_to_string(path)?) } else { - None + persona.as_ref().map(|p| p.system_prompt.clone()) }; if args.heartbeat_interval > 0 && args.heartbeat_interval < 10 { @@ -937,7 +1170,16 @@ impl Config { } } - let agent_command = args.agent_command; + // A pack's `runtime` supplies the agent binary only when the operator + // left --agent-command at its default. + let pack_runtime_command = match persona.as_ref().and_then(|p| p.runtime.as_deref()) { + Some(runtime) if !sources.agent_command => Some(runtime_to_agent_command(runtime)?), + _ => None, + }; + let agent_command = match pack_runtime_command { + Some(command) => command.to_string(), + None => args.agent_command, + }; if agent_command.trim().is_empty() { return Err(ConfigError::ConfigFile( @@ -945,7 +1187,15 @@ impl Config { )); } - let agent_args = normalize_agent_args(&agent_command, args.agent_args); + // When the pack picked the binary, discard the default args that were + // computed for the *previous* binary so `normalize_agent_args` applies + // the new one's defaults instead. + let raw_agent_args = if pack_runtime_command.is_some() && !sources.agent_args { + Vec::new() + } else { + args.agent_args + }; + let agent_args = normalize_agent_args(&agent_command, raw_agent_args); if let Some(ref channels) = args.channels { for ch in channels { @@ -1078,7 +1328,16 @@ impl Config { // Spawned desktop agents now carry a complete instance snapshot. Team // instructions arrive independently so they can be layered at runtime. let mut persona_env_vars = Vec::new(); - let model = args.model; + // Pack env MUST land before the generated CODEX_CONFIG below: + // `build_codex_config_env` treats the *first* CODEX_CONFIG entry as the + // persona base and deep-merges later ones into it. Appending pack env + // afterwards would silently invert that merge. + if let Some(p) = persona.as_ref() { + persona_env_vars.extend(p.runtime_env_vars.iter().cloned()); + } + let model = args + .model + .or_else(|| persona.as_ref().and_then(|p| p.model.clone())); // Inject CODEX_CONFIG so the @agentclientprotocol/codex-acp adapter (1.x) // opens the Seatbelt network sandbox for buzz-cli (an MCP subprocess). No-op @@ -1096,6 +1355,42 @@ impl Config { let project_paths = parse_project_paths(args.project_paths)?; let channel_projects = parse_channel_projects(args.channel_projects)?; + // Pack triggers → subscribe mode / mention gate. `all_messages` is the + // only trigger shape ACP can express today. + let mut subscribe_mode = args.subscribe; + let mut no_mention_filter = args.no_mention_filter; + if let Some(triggers) = persona.as_ref().map(|p| &p.triggers) { + if triggers.all_messages { + if !sources.subscribe { + subscribe_mode = SubscribeMode::All; + } + if !sources.no_mention_filter { + no_mention_filter = true; + } + } else if !triggers.mentions && !sources.no_mention_filter { + // "respond to neither mentions nor everything" has no ACP + // representation — say so rather than inventing one. + tracing::warn!( + "persona pack sets triggers.mentions=false with all_messages=false; \ + buzz-acp has no such mode — leaving the mention gate unchanged" + ); + } + if !triggers.keywords.is_empty() { + tracing::warn!( + keywords = triggers.keywords.join(","), + "persona pack triggers.keywords are not wired into buzz-acp \ + subscriptions and will be ignored" + ); + } + } + + // Channel names, not UUIDs — resolved after discovery in `main()`. + // Skipped entirely when --channels already pinned the subscription set. + let pack_subscribe = match persona.as_ref() { + Some(p) if args.channels.is_none() => p.subscribe.clone(), + _ => Vec::new(), + }; + let config = Config { keys, relay_url: args.relay_url, @@ -1114,15 +1409,16 @@ impl Config { .as_deref() .map(str::trim) .filter(|value| !value.is_empty()) - .map(str::to_string), + .map(str::to_string) + .or_else(|| persona.as_ref().and_then(|p| p.pack_instructions.clone())), initial_message: args.initial_message, - subscribe_mode: args.subscribe, + subscribe_mode, dedup_mode: args.dedup, multiple_event_handling: args.multiple_event_handling, ignore_self: !args.no_ignore_self, kinds_override: args.kinds, channels_override: args.channels, - no_mention_filter: args.no_mention_filter, + no_mention_filter, config_path: args.config, context_message_limit: args.context_message_limit, max_turns_per_session: args.max_turns_per_session, @@ -1133,6 +1429,7 @@ impl Config { session_title: args .session_title .as_deref() + .or_else(|| persona.as_ref().map(|p| p.display_name.as_str())) .and_then(sanitize_session_title), permission_mode: args.permission_mode, respond_to: args.respond_to, @@ -1149,6 +1446,7 @@ impl Config { base_prompt_content, project_paths, channel_projects, + pack_subscribe, }; Ok(config) @@ -1312,6 +1610,109 @@ pub fn resolve_project_path(config: &Config, project_id: &str) -> Option &str { + let trimmed = raw.trim(); + trimmed.strip_prefix('#').unwrap_or(trimmed).trim() +} + +/// Map persona-pack `subscribe` channel *names* onto discovered channel UUIDs. +/// +/// Both sides are put through [`normalize_channel_name`] and compared +/// case-insensitively, so `"#general"`, `"general"`, `"General"` and +/// `" #general "` all match a discovered channel named `general`. +/// +/// Returns `(uuid_strings, unmatched_names)`. The UUID strings are the shape +/// [`Config::channels_override`] is parsed back out of. Entries that normalize +/// to empty are dropped into `unmatched_names` so the caller can warn instead +/// of matching everything or nothing. +/// +/// Callers must not assign an empty result to `channels_override`: an empty +/// override resolves to an empty subscription set, which mutes the agent +/// silently. +pub fn resolve_subscribe_channels( + names: &[String], + discovered: &HashMap, +) -> (Vec, Vec) { + let mut ids = Vec::new(); + let mut unmatched = Vec::new(); + for raw in names { + let wanted = normalize_channel_name(raw); + if wanted.is_empty() { + unmatched.push(raw.clone()); + continue; + } + let hit = discovered + .iter() + .find(|(_, info)| normalize_channel_name(&info.name).eq_ignore_ascii_case(wanted)); + match hit { + Some((id, _)) => { + let id = id.to_string(); + if !ids.contains(&id) { + ids.push(id); + } + } + None => unmatched.push(raw.clone()), + } + } + (ids, unmatched) +} + +/// Apply a persona pack's `subscribe:` channel names to `config`, once channel +/// discovery has produced real UUIDs to match them against. +/// +/// Precedence: `channels_override.is_some()` means `--channels` / +/// `BUZZ_ACP_CHANNELS` already pinned the subscription set, so the pack is +/// skipped entirely. +/// +/// Refuses to narrow the subscription to nothing. If no pack name matches a +/// discovered channel, `channels_override` is left alone and the harness keeps +/// its default scope — assigning `Some(vec![])` would make +/// [`resolve_channel_filters`] produce an empty target set and +/// [`resolve_dynamic_channel_filter`] reject every future channel, silently +/// deafening the agent with no error anywhere. +pub fn apply_pack_subscribe( + config: &mut Config, + discovered: &HashMap, +) { + if config.channels_override.is_some() || config.pack_subscribe.is_empty() { + return; + } + + let (ids, unmatched) = resolve_subscribe_channels(&config.pack_subscribe, discovered); + for name in &unmatched { + tracing::warn!( + channel = %name, + "persona pack subscribe: no discovered channel with this name" + ); + } + if ids.is_empty() { + tracing::warn!( + "persona pack subscribe matched no discovered channels — \ + keeping the harness's default channel scope rather than \ + subscribing to nothing" + ); + return; + } + tracing::info!( + matched = ids.len(), + "persona pack subscribe resolved to discovered channels" + ); + config.channels_override = Some(ids); +} + /// Parse `--channel-projects` entries (`channel_uuid=project_id` pairs). /// /// Rejects a malformed entry, a non-UUID channel id, or a duplicate channel @@ -1371,21 +1772,64 @@ pub fn build_channel_cwd_map(config: &Config) -> HashMap { ); continue; }; - if !path.is_absolute() || !path.exists() { + let Some(cwd) = validated_project_cwd(project_id, &path) else { tracing::warn!( %channel_id, project_id, - path = %path.display(), - "resolved project path is not an absolute, existing directory — \ + "channel's bound project path is unusable as a cwd — \ dispatches in this channel will use the harness's default cwd" ); continue; - } - result.insert(*channel_id, path.to_string_lossy().into_owned()); + }; + result.insert(*channel_id, cwd); } result } +/// Validate a resolved project path for use as an agent subprocess cwd. +/// +/// Returns the path as a string, or `None` (with a warn) when it is not an +/// absolute, existing directory. Shared by [`build_channel_cwd_map`] and +/// [`build_project_cwd_map`] so both entry points enforce the identical +/// check — a project path is either usable for every routing mechanism or +/// for none of them. +fn validated_project_cwd(project_id: &str, path: &Path) -> Option { + if !path.is_absolute() || !path.exists() { + tracing::warn!( + project_id, + path = %path.display(), + "project path is not an absolute, existing directory — ignoring it as a dispatch cwd" + ); + return None; + } + Some(path.to_string_lossy().into_owned()) +} + +/// Precompute the effective cwd for every *project* id, once at harness +/// startup, keyed by the project's NIP-MP `d`-tag id. +/// +/// This is the allow-list a per-message `["project", ]` event tag is +/// looked up in (see `pool::resolve_effective_cwd`). Unlike +/// [`build_channel_cwd_map`] it iterates `--project-paths` directly: a +/// project reachable by tag need not be bound to any channel. +/// +/// # Security +/// +/// The returned map is the *only* way a project id from an event can reach a +/// filesystem path. Every value here is operator-supplied and validated; +/// nothing derives a path from the id itself. A caller must therefore treat +/// this strictly as `get(id)` — never join, canonicalize, or otherwise build a +/// path out of an untrusted project id. +pub fn build_project_cwd_map(config: &Config) -> HashMap { + config + .project_paths + .iter() + .filter_map(|(project_id, path)| { + validated_project_cwd(project_id, path).map(|cwd| (project_id.clone(), cwd)) + }) + .collect() +} + /// Resolve per-channel NIP-01 filters from config + discovered channels. pub fn resolve_channel_filters( config: &Config, @@ -1633,6 +2077,7 @@ mod tests { base_prompt_content: None, project_paths: HashMap::new(), channel_projects: HashMap::new(), + pack_subscribe: Vec::new(), } } @@ -3264,3 +3709,728 @@ channels = "ALL" assert!(!map.contains_key(&channel_id)); } } + +/// Persona-pack wiring: does a pack on disk actually change the resulting +/// `Config`, and does an explicit CLI value still beat it? +#[cfg(test)] +mod persona_pack_tests { + use super::*; + use crate::relay::ChannelInfo; + + const TEST_PRIVATE_KEY: &str = + "nsec1vl029mgpspedva04g90vltkh6fvh240zqtv9k0t9af8935ke9laqsnlfe5"; + + /// A temp directory that deletes itself on drop, so a failing assertion + /// can't leave pack fixtures behind. + struct PackDir(PathBuf); + + impl Drop for PackDir { + fn drop(&mut self) { + std::fs::remove_dir_all(&self.0).ok(); + } + } + + impl PackDir { + fn path(&self) -> &Path { + &self.0 + } + + fn as_arg(&self) -> String { + self.0.to_string_lossy().into_owned() + } + } + + /// Write a pack directory: `.plugin/plugin.json` plus one `.persona.md` + /// per entry. `persona` entries are `(name, frontmatter_yaml, body)`. + fn write_pack(personas: &[(&str, &str, &str)], instructions: Option<&str>) -> PackDir { + let root = + std::env::temp_dir().join(format!("buzz-acp-pack-test-{}", uuid::Uuid::new_v4())); + std::fs::create_dir_all(root.join(".plugin")).expect("create .plugin"); + std::fs::create_dir_all(root.join("agents")).expect("create agents"); + + let persona_paths: Vec = personas + .iter() + .map(|(name, _, _)| format!("agents/{name}.persona.md")) + .collect(); + let manifest = serde_json::json!({ + "id": "test-pack", + "name": "Test Pack", + "version": "1.2.3", + "personas": persona_paths, + }); + std::fs::write( + root.join(".plugin").join("plugin.json"), + serde_json::to_string(&manifest).expect("serialize manifest"), + ) + .expect("write manifest"); + + for (name, frontmatter, body) in personas { + std::fs::write( + root.join("agents").join(format!("{name}.persona.md")), + format!("---\nname: {name}\n{frontmatter}---\n\n{body}\n"), + ) + .expect("write persona"); + } + + if let Some(text) = instructions { + std::fs::write(root.join("instructions.md"), text).expect("write instructions"); + } + + PackDir(root) + } + + /// A single-persona pack exercising the fields ACP actually maps. + fn write_standard_pack() -> PackDir { + write_pack( + &[( + "scout", + "display_name: \"Scout\"\n\ + description: \"Pack-supplied scout persona.\"\n\ + model: \"anthropic:claude-sonnet-4-20250514\"\n\ + temperature: 0.25\n\ + subscribe:\n - \"#engineering\"\n - general\n", + "You are Scout, defined entirely by the pack.", + )], + Some("Pack-wide team instructions."), + ) + } + + fn config_from(argv: &[&str]) -> Config { + let mut full = vec!["buzz-acp", "--private-key", TEST_PRIVATE_KEY]; + full.extend_from_slice(argv); + Config::try_from_argv(full).expect("config should build") + } + + fn error_from(argv: &[&str]) -> String { + let mut full = vec!["buzz-acp", "--private-key", TEST_PRIVATE_KEY]; + full.extend_from_slice(argv); + Config::try_from_argv(full) + .expect_err("config should fail") + .to_string() + } + + fn channel(name: &str) -> ChannelInfo { + ChannelInfo { + name: name.to_string(), + channel_type: "public".to_string(), + description: None, + } + } + + // ── The pack actually changes Config ───────────────────────────────── + + #[test] + fn pack_supplies_system_prompt_model_title_and_instructions() { + let pack = write_standard_pack(); + let config = config_from(&["--pack", &pack.as_arg()]); + + // buzz-persona hands the markdown body through verbatim (surrounding + // newlines included) — pinned exactly so a future trim is a visible change. + assert_eq!( + config.system_prompt.as_deref(), + Some("\nYou are Scout, defined entirely by the pack.\n"), + "system_prompt must come from the pack's persona body" + ); + assert_eq!( + config.model.as_deref(), + Some("claude-sonnet-4-20250514"), + "model must be the post-split plain id, not 'anthropic:...'" + ); + assert_eq!( + config.session_title.as_deref(), + Some("Scout"), + "session_title must come from the persona display_name" + ); + assert_eq!( + config.team_instructions.as_deref(), + Some("Pack-wide team instructions."), + "team_instructions must come from the pack instructions.md" + ); + } + + #[test] + fn pack_projects_runtime_env_vars_into_persona_env_vars() { + let pack = write_standard_pack(); + let config = config_from(&["--pack", &pack.as_arg()]); + + let vars: HashMap<&str, &str> = config + .persona_env_vars + .iter() + .map(|(k, v)| (k.as_str(), v.as_str())) + .collect(); + assert_eq!(vars.get("GOOSE_MODEL"), Some(&"claude-sonnet-4-20250514")); + assert_eq!(vars.get("GOOSE_PROVIDER"), Some(&"anthropic")); + assert_eq!(vars.get("GOOSE_TEMPERATURE"), Some(&"0.25")); + } + + #[test] + fn pack_subscribe_names_are_carried_for_post_discovery_resolution() { + let pack = write_standard_pack(); + let config = config_from(&["--pack", &pack.as_arg()]); + + assert_eq!( + config.pack_subscribe, + vec!["#engineering".to_string(), "general".to_string()], + "subscribe names must reach Config verbatim (# intact) for later resolution" + ); + assert!( + config.channels_override.is_none(), + "names must NOT be written into channels_override — it is parsed as UUIDs" + ); + } + + #[test] + fn pack_runtime_selects_agent_command_and_resets_agent_args() { + let pack = write_pack( + &[( + "coder", + "display_name: \"Coder\"\ndescription: \"d\"\nruntime: claude\n", + "body", + )], + None, + ); + let config = config_from(&["--pack", &pack.as_arg()]); + + assert_eq!(config.agent_command, "claude-agent-acp"); + assert!( + config.agent_args.is_empty(), + "the goose default 'acp' arg must not survive onto claude-agent-acp: {:?}", + config.agent_args + ); + } + + #[test] + fn pack_all_messages_trigger_sets_subscribe_all_and_drops_mention_gate() { + let pack = write_pack( + &[( + "watcher", + "display_name: \"Watcher\"\ndescription: \"d\"\n\ + triggers:\n mentions: false\n all_messages: true\n", + "body", + )], + None, + ); + let config = config_from(&["--pack", &pack.as_arg()]); + + assert_eq!(config.subscribe_mode, SubscribeMode::All); + assert!(config.no_mention_filter); + } + + #[test] + fn no_pack_leaves_every_pack_driven_field_at_its_default() { + let config = config_from(&[]); + + assert!(config.system_prompt.is_none()); + assert!(config.model.is_none()); + assert!(config.session_title.is_none()); + assert!(config.team_instructions.is_none()); + assert!(config.pack_subscribe.is_empty()); + assert_eq!(config.agent_command, "goose"); + assert_eq!(config.agent_args, vec!["acp".to_string()]); + assert_eq!(config.subscribe_mode, SubscribeMode::Mentions); + assert!(!config.no_mention_filter); + } + + // ── Explicit CLI/env values beat the pack ──────────────────────────── + + #[test] + fn explicit_system_prompt_and_model_override_pack() { + let pack = write_standard_pack(); + let config = config_from(&[ + "--pack", + &pack.as_arg(), + "--system-prompt", + "operator prompt wins", + "--model", + "operator-model", + ]); + + assert_eq!( + config.system_prompt.as_deref(), + Some("operator prompt wins") + ); + assert_eq!(config.model.as_deref(), Some("operator-model")); + } + + #[test] + fn explicit_agent_command_overrides_pack_runtime() { + let pack = write_pack( + &[( + "coder", + "display_name: \"Coder\"\ndescription: \"d\"\nruntime: claude\n", + "body", + )], + None, + ); + let config = config_from(&["--pack", &pack.as_arg(), "--agent-command", "goose"]); + + assert_eq!( + config.agent_command, "goose", + "an explicitly passed --agent-command must beat the pack's runtime" + ); + } + + #[test] + fn explicit_session_title_and_team_instructions_override_pack() { + let pack = write_standard_pack(); + let config = config_from(&[ + "--pack", + &pack.as_arg(), + "--session-title", + "Operator Title", + "--team-instructions", + "Operator instructions", + ]); + + assert_eq!(config.session_title.as_deref(), Some("Operator Title")); + assert_eq!( + config.team_instructions.as_deref(), + Some("Operator instructions") + ); + } + + #[test] + fn explicit_channels_suppresses_pack_subscribe() { + let pack = write_standard_pack(); + let uuid = Uuid::new_v4().to_string(); + let config = config_from(&["--pack", &pack.as_arg(), "--channels", &uuid]); + + assert!( + config.pack_subscribe.is_empty(), + "--channels means the operator pinned the subscription set" + ); + assert_eq!(config.channels_override, Some(vec![uuid])); + } + + #[test] + fn explicit_subscribe_mode_overrides_pack_all_messages_trigger() { + let pack = write_pack( + &[( + "watcher", + "display_name: \"Watcher\"\ndescription: \"d\"\n\ + triggers:\n mentions: false\n all_messages: true\n", + "body", + )], + None, + ); + let config = config_from(&["--pack", &pack.as_arg(), "--subscribe", "mentions"]); + + assert_eq!( + config.subscribe_mode, + SubscribeMode::Mentions, + "an explicit --subscribe must beat the pack's all_messages trigger" + ); + } + + #[test] + fn from_args_treats_defaulted_args_as_explicit() { + // `from_args` cannot see argument provenance, so a pack must never win + // a defaulted arg through that entry point. + let pack = write_pack( + &[( + "coder", + "display_name: \"Coder\"\ndescription: \"d\"\nruntime: claude\n", + "body", + )], + None, + ); + let args = CliArgs::try_parse_from([ + "buzz-acp", + "--private-key", + TEST_PRIVATE_KEY, + "--pack", + &pack.as_arg(), + ]) + .expect("clap should parse args"); + let config = Config::from_args(args).expect("config should build"); + + assert_eq!( + config.agent_command, "goose", + "from_args is fail-safe: the clap default must hold" + ); + assert_eq!( + config.system_prompt.as_deref(), + Some("\nbody\n"), + "Option-typed args carry no default, so the pack still supplies them" + ); + } + + // ── A bad pack fails startup loudly ────────────────────────────────── + + #[test] + fn missing_pack_directory_is_a_hard_startup_error() { + let missing = + std::env::temp_dir().join(format!("buzz-acp-absent-{}", uuid::Uuid::new_v4())); + let msg = error_from(&["--pack", &missing.to_string_lossy()]); + + assert!( + msg.contains("persona pack"), + "error must name the pack: {msg}" + ); + } + + #[test] + fn malformed_manifest_is_a_hard_startup_error() { + let pack = write_standard_pack(); + std::fs::write(pack.path().join(".plugin").join("plugin.json"), "{not json") + .expect("corrupt manifest"); + let msg = error_from(&["--pack", &pack.as_arg()]); + + assert!( + msg.contains("persona pack"), + "error must name the pack: {msg}" + ); + } + + #[test] + fn unknown_pack_runtime_is_rejected_rather_than_spawned() { + // A pack must not be able to name an arbitrary executable. + let pack = write_pack( + &[( + "evil", + "display_name: \"Evil\"\ndescription: \"d\"\nruntime: /bin/sh\n", + "body", + )], + None, + ); + let msg = error_from(&["--pack", &pack.as_arg()]); + + assert!( + msg.contains("unknown runtime"), + "error should reject the runtime id: {msg}" + ); + } + + #[test] + fn multi_persona_pack_without_persona_flag_is_an_error_listing_choices() { + let pack = write_pack( + &[ + ( + "alpha", + "display_name: \"Alpha\"\ndescription: \"d\"\n", + "a", + ), + ("beta", "display_name: \"Beta\"\ndescription: \"d\"\n", "b"), + ], + None, + ); + let msg = error_from(&["--pack", &pack.as_arg()]); + + assert!( + msg.contains("--persona"), + "error should name the flag: {msg}" + ); + assert!(msg.contains("alpha"), "error should list choices: {msg}"); + assert!(msg.contains("beta"), "error should list choices: {msg}"); + } + + #[test] + fn persona_flag_selects_one_persona_from_a_multi_persona_pack() { + let pack = write_pack( + &[ + ( + "alpha", + "display_name: \"Alpha\"\ndescription: \"d\"\n", + "a", + ), + ("beta", "display_name: \"Beta\"\ndescription: \"d\"\n", "b"), + ], + None, + ); + let config = config_from(&["--pack", &pack.as_arg(), "--persona", "beta"]); + + assert_eq!(config.system_prompt.as_deref(), Some("\nb\n")); + assert_eq!(config.session_title.as_deref(), Some("Beta")); + } + + #[test] + fn unknown_persona_name_is_an_error_listing_choices() { + let pack = write_pack( + &[ + ( + "alpha", + "display_name: \"Alpha\"\ndescription: \"d\"\n", + "a", + ), + ("beta", "display_name: \"Beta\"\ndescription: \"d\"\n", "b"), + ], + None, + ); + let msg = error_from(&["--pack", &pack.as_arg(), "--persona", "gamma"]); + + assert!(msg.contains("gamma"), "error should name the miss: {msg}"); + assert!(msg.contains("alpha"), "error should list choices: {msg}"); + } + + // ── The real shipped example pack ──────────────────────────────────── + + #[test] + fn the_shipped_meadow_core_example_pack_configures_the_harness() { + // Synthetic fixtures can drift from what pack authors actually write. + // This loads the pack committed at examples/meadow-core. + let pack_dir = Path::new(env!("CARGO_MANIFEST_DIR")).join("../../examples/meadow-core"); + assert!( + pack_dir.join(".plugin").join("plugin.json").exists(), + "example pack missing at {}", + pack_dir.display() + ); + let pack_arg = pack_dir.to_string_lossy().into_owned(); + + // Three personas, so an unqualified --pack must refuse to guess. + let msg = error_from(&["--pack", &pack_arg]); + assert!(msg.contains("--persona"), "should demand a persona: {msg}"); + + let config = config_from(&["--pack", &pack_arg, "--persona", "bana"]); + assert_eq!(config.session_title.as_deref(), Some("Bana")); + assert!( + config + .system_prompt + .as_deref() + .is_some_and(|p| p.contains("architecture reviewer")), + "system prompt should be Bana's markdown body" + ); + assert_eq!( + config.pack_subscribe, + vec!["#architecture".to_string()], + "the '#'-prefixed subscribe entry must survive to resolution time" + ); + assert!( + config + .team_instructions + .as_deref() + .is_some_and(|t| !t.trim().is_empty()), + "the pack ships instructions.md, which should land as team_instructions" + ); + + // ...and its '#'-prefixed subscribe resolves against a discovered channel. + let architecture = Uuid::new_v4(); + let discovered = HashMap::from([(architecture, channel("architecture"))]); + let mut config = config; + apply_pack_subscribe(&mut config, &discovered); + assert_eq!( + config.channels_override, + Some(vec![architecture.to_string()]) + ); + } + + // ── Codex merge ordering ───────────────────────────────────────────── + + #[test] + fn pack_env_precedes_the_generated_codex_config_entry() { + // build_codex_config_env treats the FIRST CODEX_CONFIG in the vec as + // the persona base; pack env appended afterwards would invert it. + let pack = write_pack( + &[( + "coder", + "display_name: \"Coder\"\ndescription: \"d\"\n\ + runtime: codex\nmodel: \"openai:gpt-5\"\n", + "body", + )], + None, + ); + let config = config_from(&["--pack", &pack.as_arg()]); + + assert!( + config.has_generated_codex_config, + "codex-acp should have produced a generated CODEX_CONFIG" + ); + let generated = config + .persona_env_vars + .iter() + .position(|(k, _)| k == "CODEX_CONFIG") + .expect("generated CODEX_CONFIG entry"); + let pack_var = config + .persona_env_vars + .iter() + .position(|(k, _)| k == "GOOSE_MODEL") + .expect("pack-projected env var"); + assert!( + pack_var < generated, + "pack env must be appended before the generated CODEX_CONFIG entry" + ); + } + + // ── '#'-stripping channel-name resolution ──────────────────────────── + + #[test] + fn normalize_channel_name_strips_exactly_one_leading_hash() { + assert_eq!(normalize_channel_name("general"), "general"); + assert_eq!(normalize_channel_name("#general"), "general"); + assert_eq!(normalize_channel_name(" #general "), "general"); + assert_eq!(normalize_channel_name("# general"), "general"); + assert_eq!( + normalize_channel_name("##general"), + "#general", + "'#' is a display sigil, not a repeatable one" + ); + assert_eq!( + normalize_channel_name("general#"), + "general#", + "a trailing '#' is part of the name" + ); + assert_eq!(normalize_channel_name("#"), ""); + } + + #[test] + fn resolve_subscribe_channels_matches_hashed_and_bare_names() { + let engineering = Uuid::new_v4(); + let general = Uuid::new_v4(); + let discovered = HashMap::from([ + (engineering, channel("engineering")), + (general, channel("general")), + ]); + + let names = vec![ + "#engineering".to_string(), + "general".to_string(), + " #General ".to_string(), + ]; + let (ids, unmatched) = resolve_subscribe_channels(&names, &discovered); + + assert!(unmatched.is_empty(), "unexpected misses: {unmatched:?}"); + assert_eq!( + ids, + vec![engineering.to_string(), general.to_string()], + "case-insensitive duplicate must not be emitted twice" + ); + } + + #[test] + fn resolve_subscribe_channels_matches_a_relay_name_that_carries_a_hash() { + let id = Uuid::new_v4(); + let discovered = HashMap::from([(id, channel("#ops"))]); + + let (ids, unmatched) = resolve_subscribe_channels(&["ops".to_string()], &discovered); + + assert!(unmatched.is_empty()); + assert_eq!(ids, vec![id.to_string()]); + } + + #[test] + fn resolve_subscribe_channels_reports_misses_instead_of_matching_everything() { + let id = Uuid::new_v4(); + let discovered = HashMap::from([(id, channel("general"))]); + + let names = vec![ + "##general".to_string(), + "nope".to_string(), + "#".to_string(), + " ".to_string(), + ]; + let (ids, unmatched) = resolve_subscribe_channels(&names, &discovered); + + assert!( + ids.is_empty(), + "none of these should match 'general': {ids:?}" + ); + assert_eq!( + unmatched.len(), + 4, + "every miss must be reported: {unmatched:?}" + ); + } + + // ── apply_pack_subscribe: never narrow to nothing ──────────────────── + + #[test] + fn apply_pack_subscribe_sets_channels_override_from_matched_names() { + let engineering = Uuid::new_v4(); + let discovered = HashMap::from([ + (engineering, channel("engineering")), + (Uuid::new_v4(), channel("random")), + ]); + let mut config = config_from(&[]); + config.pack_subscribe = vec!["#engineering".to_string()]; + + apply_pack_subscribe(&mut config, &discovered); + + assert_eq!( + config.channels_override, + Some(vec![engineering.to_string()]) + ); + } + + #[test] + fn apply_pack_subscribe_never_mutes_the_agent_when_nothing_matches() { + // The highest-severity failure mode: assigning Some(vec![]) here makes + // resolve_channel_filters produce an empty target set, and the agent + // goes deaf with no error anywhere. + let discovered = HashMap::from([(Uuid::new_v4(), channel("general"))]); + let mut config = config_from(&[]); + config.pack_subscribe = vec!["#nonexistent".to_string()]; + + apply_pack_subscribe(&mut config, &discovered); + + assert_eq!( + config.channels_override, None, + "an all-miss pack subscribe must leave the default scope intact" + ); + } + + #[test] + fn apply_pack_subscribe_defers_to_an_operator_channels_override() { + let engineering = Uuid::new_v4(); + let discovered = HashMap::from([(engineering, channel("engineering"))]); + let operator_pick = Uuid::new_v4().to_string(); + let mut config = config_from(&[]); + config.pack_subscribe = vec!["#engineering".to_string()]; + config.channels_override = Some(vec![operator_pick.clone()]); + + apply_pack_subscribe(&mut config, &discovered); + + assert_eq!( + config.channels_override, + Some(vec![operator_pick]), + "--channels must win over the pack's subscribe list" + ); + } + + #[test] + fn apply_pack_subscribe_is_a_noop_without_a_pack() { + let discovered = HashMap::from([(Uuid::new_v4(), channel("general"))]); + let mut config = config_from(&[]); + + apply_pack_subscribe(&mut config, &discovered); + + assert_eq!(config.channels_override, None); + } + + // ── value_source id guard ──────────────────────────────────────────── + + #[test] + fn explicit_args_ids_match_cli_fields() { + // `ArgMatches::value_source` panics in debug builds on an unknown id, + // so a renamed CliArgs field must fail here rather than at startup. + use clap::{CommandFactory, FromArgMatches}; + let matches = CliArgs::command() + .try_get_matches_from(["buzz-acp", "--private-key", TEST_PRIVATE_KEY]) + .expect("clap should parse args"); + CliArgs::from_arg_matches(&matches).expect("args should build"); + + let sources = ExplicitArgs::from_matches(&matches); + assert!(!sources.agent_command); + assert!(!sources.agent_args); + assert!(!sources.subscribe); + assert!(!sources.no_mention_filter); + + let matches = CliArgs::command() + .try_get_matches_from([ + "buzz-acp", + "--private-key", + TEST_PRIVATE_KEY, + "--agent-command", + "codex", + "--agent-args", + "acp", + "--subscribe", + "all", + "--no-mention-filter", + ]) + .expect("clap should parse args"); + let sources = ExplicitArgs::from_matches(&matches); + assert!(sources.agent_command); + assert!(sources.agent_args); + assert!(sources.subscribe); + assert!(sources.no_mention_filter); + } +} diff --git a/crates/buzz-acp/src/lib.rs b/crates/buzz-acp/src/lib.rs index 1093ad824c1..9b2a489e860 100644 --- a/crates/buzz-acp/src/lib.rs +++ b/crates/buzz-acp/src/lib.rs @@ -2069,6 +2069,10 @@ async fn tokio_main() -> Result<()> { tracing::info!("discovered {} channel(s)", channel_info_map.len()); let channel_ids: Vec = channel_info_map.keys().copied().collect(); + // Persona-pack `subscribe:` holds channel NAMES, so it can only be applied + // now that discovery has answered. + config::apply_pack_subscribe(&mut config, &channel_info_map); + let rules: Vec = match config.subscribe_mode { SubscribeMode::Mentions => { vec![SubscriptionRule { @@ -2183,6 +2187,7 @@ async fn tokio_main() -> Result<()> { .to_string_lossy() .to_string(), channel_cwd: crate::config::build_channel_cwd_map(&config), + project_cwd: crate::config::build_project_cwd_map(&config), rest_client: relay.rest_client(), channel_info: pool::ChannelInfoResolver::new(channel_info_map, relay.rest_client()), context_message_limit: config.context_message_limit, @@ -6768,6 +6773,7 @@ mod build_mcp_servers_tests { base_prompt_content: None, project_paths: std::collections::HashMap::new(), channel_projects: std::collections::HashMap::new(), + pack_subscribe: Vec::new(), } } @@ -6993,6 +6999,7 @@ mod error_outcome_emission_tests { base_prompt_content: None, project_paths: std::collections::HashMap::new(), channel_projects: std::collections::HashMap::new(), + pack_subscribe: Vec::new(), } } diff --git a/crates/buzz-acp/src/pool.rs b/crates/buzz-acp/src/pool.rs index 6da8402507a..2526135a3ed 100644 --- a/crates/buzz-acp/src/pool.rs +++ b/crates/buzz-acp/src/pool.rs @@ -129,6 +129,16 @@ pub struct SessionState { /// Per-channel successful-delivery state. Created with the ACP session and /// cleared atomically with every invalidation path. pub deliveries: HashMap, + /// channel_id → the cwd the live session's `session/new` was called with. + /// + /// ACP has no way to move a live session to another directory (`cwd` is set + /// once, in `session/new`), so this is what a later turn compares against to + /// decide whether the cached session is still in the right project. Written + /// only by [`SessionState::register_channel_session`] and removed by every + /// invalidation path, so it cannot outlive the `sessions` entry it describes. + /// + /// Heartbeats are deliberately absent: they always run at `ctx.cwd`. + pub session_cwds: HashMap, } impl SessionState { @@ -153,9 +163,28 @@ impl SessionState { self.core_sections.remove(channel_id); self.canvas_sections.remove(channel_id); self.deliveries.remove(channel_id); + self.session_cwds.remove(channel_id); self.sessions.remove(channel_id).is_some() } + /// The single way a channel's ACP session is recorded. + /// + /// Session id, the cwd it was created with, and a fresh delivery ledger are + /// written together so `session_cwds` structurally cannot drift out of sync + /// with `sessions` — a drift that would silently reroute (or fail to + /// reroute) later turns. + pub(crate) fn register_channel_session( + &mut self, + channel_id: Uuid, + session_id: String, + cwd: String, + ) { + self.sessions.insert(channel_id, session_id); + self.session_cwds.insert(channel_id, cwd); + self.deliveries + .insert(channel_id, ChannelDeliveryState::default()); + } + /// Invalidate all sessions and turn counters (e.g. after agent exit). pub fn invalidate_all(&mut self) { self.sessions.clear(); @@ -166,6 +195,7 @@ impl SessionState { self.core_sections.clear(); self.canvas_sections.clear(); self.deliveries.clear(); + self.session_cwds.clear(); } pub(crate) fn mark_channel_delivery_success( @@ -186,6 +216,7 @@ impl SessionState { || self.core_sections.contains_key(channel_id) || self.canvas_sections.contains_key(channel_id) || self.deliveries.contains_key(channel_id) + || self.session_cwds.contains_key(channel_id) } } @@ -582,6 +613,14 @@ pub struct PromptContext { /// by [`crate::config::build_channel_cwd_map`]. A channel absent from /// this map dispatches with `cwd` (the harness's own default). pub channel_cwd: HashMap, + /// Project id → validated cwd, precomputed once at startup by + /// [`crate::config::build_project_cwd_map`] from `--project-paths`. + /// + /// This is the allow-list that a per-message `["project", ]` event tag + /// is looked up in. A project id absent from this map is ignored — it is + /// **never** treated as, or joined into, a filesystem path. See + /// [`resolve_turn_routing`] for the full precedence and threat model. + pub project_cwd: HashMap, /// REST client for pre-prompt context fetches (thread/DM history). pub rest_client: RestClient, /// Shared channel metadata for startup-known and dynamically joined channels. @@ -967,12 +1006,99 @@ fn resolve_effective_cwd(ctx: &PromptContext, channel_id: Option) -> &str .unwrap_or(&ctx.cwd) } -/// Create a new ACP session via `session_new_full()`, populate model capabilities -/// on the agent (first session only), and apply `desired_model` if set. +/// The project id carried by the newest `["project", ]` tag in a batch's +/// **live** events, if any. /// -/// On error from `session_new_full()`, returns the `AcpError` — caller handles -/// error reporting. Model-switch failures are logged and gracefully ignored -/// (the agent proceeds with its default model). +/// Reverse iteration gives last-writer-wins within a single batch: when several +/// messages flush together, the most recent one decides where the turn runs. +/// +/// `cancelled_events` are deliberately ignored — they belong to the turn being +/// superseded, so a merged re-prompt follows the newest request, not the one it +/// replaced. +/// +/// The returned string is an opaque lookup key, never a path. See +/// [`resolve_turn_routing`]. +fn batch_project_tag(batch: Option<&FlushBatch>) -> Option<&str> { + batch?.events.iter().rev().find_map(|be| { + be.event.tags.iter().find_map(|tag| { + let slice = tag.as_slice(); + match slice { + [key, value, ..] if key == "project" && !value.is_empty() => Some(value.as_str()), + _ => None, + } + }) + }) +} + +/// Where this turn's session must run, and whether a live session created +/// somewhere else has to be torn down first. +#[derive(Debug, PartialEq, Eq)] +struct CwdRouting { + /// The cwd `session/new` must be called with for this turn. + pub cwd: String, + /// True when a live session exists for this channel but was created in a + /// different cwd. The caller must `invalidate_channel` *before* any code + /// that reads `state.sessions` to decide whether it is creating a session. + pub must_invalidate: bool, +} + +/// Decide this turn's cwd, and whether the cached session is still usable. +/// +/// Precedence: per-message `["project", ]` tag → channel binding → +/// harness default. +/// +/// # Security +/// +/// `project_tag` originates in an **event tag**, so it is attacker-influencable +/// — unlike the operator-configured channel binding. It is therefore consulted +/// strictly as `project_cwd.get(id)`: a key lookup into the operator-supplied +/// `--project-paths` allow-list. It is never joined onto a base directory, +/// canonicalized, or otherwise turned into a path. An unknown id — including a +/// traversal attempt, an absolute path, a UNC path, or one carrying a null byte +/// — misses the map and falls back to the pre-existing channel/default +/// behavior, so a hostile tag can only ever select a directory the operator +/// already configured. +fn resolve_turn_routing( + default_cwd: &str, + channel_cwd: &HashMap, + project_cwd: &HashMap, + channel_id: Uuid, + project_tag: Option<&str>, + live_session_cwd: Option<&str>, +) -> CwdRouting { + // The ONLY bridge from an event-supplied id to a filesystem path. + let tagged = project_tag.and_then(|id| project_cwd.get(id).map(String::as_str)); + if let (Some(id), None) = (project_tag, tagged) { + tracing::warn!( + target: "pool::session", + %channel_id, + project_id = id, + "message carries a project id that is not in --project-paths — \ + ignoring it and using the channel/default cwd" + ); + } + + let cwd = tagged + .or_else(|| channel_cwd.get(&channel_id).map(String::as_str)) + .unwrap_or(default_cwd) + .to_owned(); + + // Only a live session whose cwd differs forces a rotation. `None` covers + // both "no live session" (the creation path below resolves the right cwd + // anyway) and the unreachable-in-production case of a `sessions` entry with + // no recorded cwd, which stays on the pre-feature behavior rather than + // rotating the session on every single turn. + let must_invalidate = match live_session_cwd { + Some(live) => live != cwd, + None => false, + }; + + CwdRouting { + cwd, + must_invalidate, + } +} + struct NewSessionChannelContext<'a> { huddle_instructions: Option<&'a str>, canvas: Option<&'a str>, @@ -981,9 +1107,23 @@ struct NewSessionChannelContext<'a> { channel_type: Option<&'a str>, } +/// Create a new ACP session via `session_new_full()`, populate model capabilities +/// on the agent (first session only), and apply `desired_model` if set. +/// +/// On error from `session_new_full()`, returns the `AcpError` — caller handles +/// error reporting. Model-switch failures are logged and gracefully ignored +/// (the agent proceeds with its default model). +/// +/// `effective_cwd` is decided by the caller (see [`resolve_turn_routing`]) and +/// used verbatim, so the cwd that was *compared* against the live session and +/// the cwd the replacement session is *created* with can never disagree. +// Two call sites (channel + heartbeat); every parameter is an independent +// per-session input, so bundling them would only move the argument list. +#[allow(clippy::too_many_arguments)] async fn create_session_and_apply_model( agent: &mut OwnedAgent, ctx: &PromptContext, + effective_cwd: &str, agent_core: Option<&str>, channel: NewSessionChannelContext<'_>, ) -> Result { @@ -994,7 +1134,6 @@ async fn create_session_and_apply_model( // its own `[Agent Memory — core]` header, and canvas carries its own // `[Channel Canvas]` header; both are appended with a blank-line separator. let is_goose = agent.agent_name == "goose"; - let effective_cwd = resolve_effective_cwd(ctx, channel.id); let combined_system_prompt = with_canvas( with_huddle_instructions( with_core( @@ -1578,6 +1717,61 @@ pub async fn run_prompt_task( .unwrap_or_default(); let _reaction_guard = ReactionGuard::new(ctx.rest_client.clone(), reaction_ids.clone()); + // ── Per-message project routing ────────────────────────────────────────── + // + // A live ACP session's cwd is fixed at `session/new`, so routing this turn + // into a different project means rotating the session. The decision MUST + // run before the core-memory block below and the canvas/title block after + // it: both gate on `!agent.state.sessions.contains_key(cid)`, and if the + // invalidation landed after those reads they would each conclude "not a new + // session" for a session about to be destroyed — leaving the replacement + // session with no core memory, no canvas, an unqualified title, and no git + // origin env. A reroute *is* a rotation, so the replacement has to rebuild + // all of that for the project it is actually landing in. + // + // Both prerequisites hold here: `source` is bound above, the observer + // context is installed (so `observe` reaches the frame), and `batch` is + // still un-moved. + let turn_cwd: Option = match &source { + PromptSource::Channel(cid) => { + let routing = resolve_turn_routing( + &ctx.cwd, + &ctx.channel_cwd, + &ctx.project_cwd, + *cid, + batch_project_tag(batch.as_ref()), + agent.state.session_cwds.get(cid).map(String::as_str), + ); + if routing.must_invalidate { + let previous_session = agent.state.sessions.get(cid).cloned(); + let from_cwd = agent.state.session_cwds.get(cid).cloned(); + tracing::info!( + target: "pool::session", + channel = %cid, + ?from_cwd, + to_cwd = %routing.cwd, + "message routes to a different project — rotating the session" + ); + agent.acp.observe( + "session_rerouted", + serde_json::json!({ + "channelId": cid.to_string(), + "fromCwd": from_cwd, + "toCwd": routing.cwd, + "previousSessionId": previous_session, + }), + ); + // Single channel only — never `invalidate_all`, which would take + // out every other channel's session and the heartbeat with it. + agent.state.invalidate_channel(cid); + } + Some(routing.cwd) + } + // Heartbeats always run at the harness default; they carry no batch and + // so have no project tag to route on. + PromptSource::Heartbeat => None, + }; + // // Core memory is delivered inside the system prompt the harness already // builds (system role for protocol >= 2, the `[System]` user-message @@ -1710,9 +1904,15 @@ pub async fn run_prompt_task( // agent in several channels doesn't produce identical session // rows; `title_channel` comes from the single resolve above and // is `None` for DM, unresolved, and unnamed channels. + // Decided above, before core/canvas/title were resolved, so the + // session is created in exactly the cwd that was compared. + let effective_cwd = turn_cwd + .clone() + .unwrap_or_else(|| resolve_effective_cwd(&ctx, Some(*cid)).to_string()); match create_session_and_apply_model( &mut agent, &ctx, + &effective_cwd, agent_core.as_deref(), NewSessionChannelContext { huddle_instructions: huddle_instructions.as_deref(), @@ -1729,11 +1929,11 @@ pub async fn run_prompt_task( target: "pool::session", "created session {sid} for channel {cid}" ); - agent.state.sessions.insert(*cid, sid.clone()); + // Session id, its cwd, and a fresh delivery ledger land + // together — see `register_channel_session`. agent .state - .deliveries - .insert(*cid, ChannelDeliveryState::default()); + .register_channel_session(*cid, sid.clone(), effective_cwd); // Seed a zero usage baseline: buzz-acp spawned this session // so prior usage is zero by definition — first turn is reliable. agent.acp.notify_session_spawned(&sid); @@ -1775,9 +1975,11 @@ pub async fn run_prompt_task( if let Some(sid) = &agent.state.heartbeat_session { (sid.clone(), false) } else { + // Heartbeats always run at the harness's own cwd. match create_session_and_apply_model( &mut agent, &ctx, + &ctx.cwd, None, NewSessionChannelContext { huddle_instructions: None, @@ -7594,6 +7796,7 @@ printf '%s\n' '{{"jsonrpc":"2.0","id":0,"result":{{"stopReason":"end_turn"}}}}'" base_prompt: None, cwd: ".".to_string(), channel_cwd: HashMap::new(), + project_cwd: HashMap::new(), rest_client: RestClient { http: reqwest::Client::new(), base_url: "http://127.0.0.1:0".to_string(), @@ -7702,6 +7905,717 @@ printf '%s\n' '{{"jsonrpc":"2.0","id":0,"result":{{"stopReason":"end_turn"}}}}'" assert_eq!(resolve_effective_cwd(&ctx, Some(Uuid::new_v4())), ctx.cwd); } + // ── batch_project_tag ──────────────────────────────────────────────────── + + fn tagged_event(content: &str, tags: Vec) -> crate::queue::BatchEvent { + let event = EventBuilder::new(Kind::Custom(9), content) + .tags(tags) + .sign_with_keys(&Keys::generate()) + .unwrap(); + crate::queue::BatchEvent { + event, + prompt_tag: "test".into(), + received_at: std::time::Instant::now(), + } + } + + fn batch_with( + events: Vec, + cancelled: Vec, + ) -> FlushBatch { + FlushBatch { + channel_id: Uuid::new_v4(), + events, + cancelled_events: cancelled, + cancel_reason: None, + } + } + + #[test] + fn batch_project_tag_is_none_without_a_batch_or_tag() { + assert_eq!(batch_project_tag(None), None); + let batch = batch_with(vec![tagged_event("plain", vec![])], vec![]); + assert_eq!(batch_project_tag(Some(&batch)), None); + } + + #[test] + fn batch_project_tag_takes_the_newest_tagged_event() { + let batch = batch_with( + vec![ + tagged_event("first", vec![Tag::parse(["project", "alpha"]).unwrap()]), + tagged_event("second", vec![Tag::parse(["project", "beta"]).unwrap()]), + ], + vec![], + ); + assert_eq!(batch_project_tag(Some(&batch)), Some("beta")); + } + + #[test] + fn batch_project_tag_ignores_cancelled_events() { + // The cancelled turn's project must not win over the live request that + // superseded it. + let batch = batch_with( + vec![tagged_event("live", vec![])], + vec![tagged_event( + "cancelled", + vec![Tag::parse(["project", "alpha"]).unwrap()], + )], + ); + assert_eq!(batch_project_tag(Some(&batch)), None); + } + + #[test] + fn batch_project_tag_ignores_malformed_and_empty_project_tags() { + // A bare ["project"] tag has no value, and ["project", ""] has an empty + // one; neither may be treated as a project id. + let batch = batch_with( + vec![tagged_event( + "malformed", + vec![ + Tag::parse(["project"]).unwrap(), + Tag::parse(["project", ""]).unwrap(), + ], + )], + vec![], + ); + assert_eq!(batch_project_tag(Some(&batch)), None); + } + + // ── resolve_turn_routing ───────────────────────────────────────────────── + + fn project_map(pairs: &[(&str, &str)]) -> HashMap { + pairs + .iter() + .map(|(k, v)| ((*k).to_string(), (*v).to_string())) + .collect() + } + + #[test] + fn routing_project_tag_beats_the_channel_binding() { + let channel_id = Uuid::new_v4(); + let mut channel_cwd = HashMap::new(); + channel_cwd.insert(channel_id, "/bound/channel".to_string()); + + let routing = resolve_turn_routing( + "/harness/default", + &channel_cwd, + &project_map(&[("alpha", "/projects/alpha")]), + channel_id, + Some("alpha"), + None, + ); + assert_eq!(routing.cwd, "/projects/alpha"); + } + + #[test] + fn routing_falls_back_to_channel_binding_then_default() { + let channel_id = Uuid::new_v4(); + let mut channel_cwd = HashMap::new(); + channel_cwd.insert(channel_id, "/bound/channel".to_string()); + let projects = project_map(&[("alpha", "/projects/alpha")]); + + // No tag → channel binding. + assert_eq!( + resolve_turn_routing( + "/harness/default", + &channel_cwd, + &projects, + channel_id, + None, + None + ) + .cwd, + "/bound/channel" + ); + // No tag and no binding → harness default. + assert_eq!( + resolve_turn_routing( + "/harness/default", + &channel_cwd, + &projects, + Uuid::new_v4(), + None, + None + ) + .cwd, + "/harness/default" + ); + } + + #[test] + fn routing_invalidates_only_when_the_live_session_cwd_differs() { + let channel_id = Uuid::new_v4(); + let projects = project_map(&[("alpha", "/projects/alpha"), ("beta", "/projects/beta")]); + let empty = HashMap::new(); + let route = |tag, live| { + resolve_turn_routing( + "/harness/default", + &empty, + &projects, + channel_id, + Some(tag), + live, + ) + }; + + // Same project as the live session → reuse it. This is the guard against + // rotating (and so re-fetching core + canvas) on every single message. + assert!(!route("alpha", Some("/projects/alpha")).must_invalidate); + // Different project → rotate. + assert!(route("beta", Some("/projects/alpha")).must_invalidate); + // No live session → nothing to invalidate; creation resolves the cwd. + assert!(!route("beta", None).must_invalidate); + } + + #[test] + fn routing_does_not_churn_the_session_when_nothing_is_tagged() { + // The overwhelmingly common case: untagged messages on an unbound + // channel must never rotate a live session. + let channel_id = Uuid::new_v4(); + let no_bindings = HashMap::new(); + let no_projects = HashMap::new(); + let routing = resolve_turn_routing( + "/harness/default", + &no_bindings, + &no_projects, + channel_id, + None, + Some("/harness/default"), + ); + assert_eq!(routing.cwd, "/harness/default"); + assert!(!routing.must_invalidate); + } + + /// A project id arrives in an **event tag**, so it is attacker-controlled. + /// It may only ever select a value already present in the operator's + /// `--project-paths` map; it must never become a path in its own right. + #[test] + fn routing_hostile_project_ids_cannot_escape_the_configured_map() { + let channel_id = Uuid::new_v4(); + let mut channel_cwd = HashMap::new(); + channel_cwd.insert(channel_id, "/bound/channel".to_string()); + let projects = project_map(&[("alpha", "/projects/alpha")]); + + let hostile = [ + "../../etc", + "../../../../../../etc/passwd", + "/etc/passwd", + "C:\\Windows\\System32", + "\\\\attacker\\share", + "//attacker/share", + "alpha/../../../etc", + "alpha\0extra", + "./alpha", + "ALPHA", + "alpha ", + " alpha", + "~", + "~/.ssh", + "$HOME", + "%USERPROFILE%", + "", + ]; + + for id in hostile { + let routing = resolve_turn_routing( + "/harness/default", + &channel_cwd, + &projects, + channel_id, + Some(id), + None, + ); + // Falls back to the operator-configured channel binding — never to + // anything derived from the hostile id. + assert_eq!( + routing.cwd, "/bound/channel", + "hostile project id {id:?} must fall back to the channel binding" + ); + // And the id itself never leaks into the resolved path. + if !id.is_empty() { + assert!( + !routing.cwd.contains(id), + "hostile project id {id:?} leaked into the resolved cwd" + ); + } + } + + // Sanity: the one *configured* id still resolves, so the assertions + // above are rejecting hostile input rather than rejecting everything. + assert_eq!( + resolve_turn_routing( + "/harness/default", + &channel_cwd, + &projects, + channel_id, + Some("alpha"), + None, + ) + .cwd, + "/projects/alpha" + ); + } + + #[test] + fn routing_unknown_project_id_never_invalidates_a_live_session() { + // An unknown id must be inert: it resolves to the same cwd the session + // already has, so a stream of bogus tags cannot force session churn. + let channel_id = Uuid::new_v4(); + let no_bindings = HashMap::new(); + let no_projects = HashMap::new(); + let routing = resolve_turn_routing( + "/harness/default", + &no_bindings, + &no_projects, + channel_id, + Some("does-not-exist"), + Some("/harness/default"), + ); + assert_eq!(routing.cwd, "/harness/default"); + assert!(!routing.must_invalidate); + } + + // ── session_cwds lifecycle ─────────────────────────────────────────────── + + #[test] + fn register_channel_session_records_session_cwd_and_delivery_ledger() { + let mut s = SessionState::default(); + let ch = Uuid::new_v4(); + s.register_channel_session(ch, "sess-1".into(), "/projects/alpha".into()); + + assert_eq!(s.sessions[&ch], "sess-1"); + assert_eq!(s.session_cwds[&ch], "/projects/alpha"); + assert!(s.deliveries.contains_key(&ch)); + } + + #[test] + fn invalidate_channel_clears_session_cwd_but_leaves_other_channels() { + let mut s = SessionState::default(); + let ch_a = Uuid::new_v4(); + let ch_b = Uuid::new_v4(); + s.register_channel_session(ch_a, "sess-a".into(), "/projects/alpha".into()); + s.register_channel_session(ch_b, "sess-b".into(), "/projects/beta".into()); + + s.invalidate_channel(&ch_a); + + assert!(!s.session_cwds.contains_key(&ch_a)); + assert_eq!(s.session_cwds[&ch_b], "/projects/beta"); + } + + #[test] + fn invalidate_all_clears_session_cwds() { + let mut s = SessionState::default(); + s.register_channel_session(Uuid::new_v4(), "sess".into(), "/projects/alpha".into()); + s.invalidate_all(); + assert!(s.session_cwds.is_empty()); + } + + #[test] + fn has_channel_state_true_when_only_session_cwd_present() { + // Guards against a future edit dropping `session_cwds` from + // `invalidate_channel` and leaving orphaned per-channel state. + let mut s = SessionState::default(); + let ch = Uuid::new_v4(); + s.session_cwds.insert(ch, "/projects/alpha".into()); + assert!(s.has_channel_state(&ch)); + } + + // ── per-message project routing, end to end ────────────────────────────── + + /// Scratch dir for subprocess-backed tests. + /// + /// Deliberately under the workspace `target/` rather than + /// `std::env::temp_dir()`: the fake agent is a shell script that has to + /// reference this path from inside a single-quoted redirect, and a + /// backslashed Windows temp path is mangled there (it lands in the CWD under + /// a transliterated name instead). A forward-slash path under `target/` is + /// handled correctly by every shell and is already git-ignored. + /// Not canonicalized: on Windows `canonicalize` returns a `\\?\`-prefixed + /// verbatim path, which the shell cannot resolve. The joined path (with + /// `..` segments intact) is understood by every shell as-is. + fn reroute_scratch_dir() -> std::path::PathBuf { + let dir = + std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("../../target/acp-reroute-tests"); + std::fs::create_dir_all(&dir).expect("create scratch dir"); + dir + } + + /// Spawn a fake ACP agent that answers `session/new` with a distinct session + /// id and every other request with `end_turn`, appending each request it + /// receives to `capture`. + /// + /// Two deliberate choices, both about surviving Windows: + /// + /// * The script is written to a **file** and run as `sh ` rather than + /// passed via `-c "