diff --git a/.agents/global-rules b/.agents/global-rules index 9adbefb36..bbed3ba9e 160000 --- a/.agents/global-rules +++ b/.agents/global-rules @@ -1 +1 @@ -Subproject commit 9adbefb3660632b692f3a711ffa86764fac481f9 +Subproject commit bbed3ba9edbb48ed05ea388569321dde1a02b48a diff --git a/.env.advanced.example b/.env.advanced.example index 910a5f795..6c061abc2 100644 --- a/.env.advanced.example +++ b/.env.advanced.example @@ -200,9 +200,9 @@ UNITY_ASYNCIO_DEBUG=0 # Orchestra persistence when publishing is on: # all — write every event to Events/* (default, legacy behavior) -# allowlist — write only ManagerMethod/ToolLoop for listed tools +# allowlist — write only ManagerMethod/ToolLoop for listed action/tool names # EVENTBUS_ORCHESTRA_PERSIST_MODE=all -# EVENTBUS_ORCHESTRA_PERSIST_TOOLS=execute_code,execute_function +# EVENTBUS_ORCHESTRA_PERSIST_TOOLS=act,execute_code,execute_function # Stream ManagerMethod/ToolLoop to Pub/Sub for Console Live Actions (independent # of the Orchestra allowlist; stream_filters apply here only). diff --git a/.secrets.baseline b/.secrets.baseline index 7e580d3ac..6b4e0e02e 100644 --- a/.secrets.baseline +++ b/.secrets.baseline @@ -259,7 +259,7 @@ "filename": "tests/conversation_manager/core/test_event_handlers.py", "hashed_secret": "00942f4668670f34c5943cf52c7ef3139fe2b8d6", "is_verified": false, - "line_number": 3555 + "line_number": 3874 } ], "tests/conversation_manager/core/test_event_logging.py": [ @@ -367,14 +367,14 @@ "filename": "tests/gateway/common/test_livekit.py", "hashed_secret": "07cc235c65c465c09ab85e3daa9a71f89b3892fd", "is_verified": false, - "line_number": 38 + "line_number": 39 }, { "type": "Secret Keyword", "filename": "tests/gateway/common/test_livekit.py", "hashed_secret": "72cb70dbbafe97e5ea13ad88acd65d08389439b0", "is_verified": false, - "line_number": 39 + "line_number": 40 } ], "tests/secret_manager/test_dotenv.py": [ @@ -484,21 +484,21 @@ "filename": "unify/gateway/channels/phone/views.py", "hashed_secret": "e54919ec32579c37fe783d99d1df750fabeff18f", "is_verified": false, - "line_number": 103 + "line_number": 200 }, { "type": "Hex High Entropy String", "filename": "unify/gateway/channels/phone/views.py", "hashed_secret": "bf0206d3f49e3b278d6ababbc5c93d560a662cb9", "is_verified": false, - "line_number": 107 + "line_number": 204 }, { "type": "Hex High Entropy String", "filename": "unify/gateway/channels/phone/views.py", "hashed_secret": "273a878f5d1b6386edf30435c933904f36bbe713", "is_verified": false, - "line_number": 111 + "line_number": 208 } ], "unify/gateway/channels/whatsapp/views.py": [ @@ -534,5 +534,5 @@ } ] }, - "generated_at": "2026-07-24T02:37:51Z" + "generated_at": "2026-07-28T04:20:55Z" } diff --git a/AGENTS.md b/AGENTS.md index e4a15b5ae..d189845bc 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1264,46 +1264,6 @@ The alternative workflow creates significant noise: ## Exception If the user **explicitly asks** for a feature branch or PR workflow, follow their instructions. But **never default to this behavior** in worktree mode. -# Git History for Context - -## Context -This rule applies when you are trying to understand the *rationale* behind specific code blocks, the evolution of a module, or when deciding whether "weird" looking code is essential or legacy technical debt. - -## Rules - -### 1. Strategic Git Usage -- **Use as a Second Level of Analysis**: If the code's purpose isn't clear from the current state alone (static analysis), use `git blame` or `git log` to uncover the "why". -- **Not a Mandate**: Do not check git history for every file you touch. This creates noise. Use it selectively when you lack context. - -### 2. Understanding Code Evolution -- **Identify Legacy Code**: If you suspect code is redundant or outdated, check its commit date and message. If it was added months ago for a feature that is no longer relevant, this confirms it can likely be purged. -- **Find the "Why"**: Expressive commit messages often contain the reasoning that comments lack. Use them to understand the author's original intent before refactoring or deleting complex logic. - -### 3. Targeted Queries -- **Be Surgical**: When querying git, look for the history of specific lines or changes (e.g., `git blame -L n,m filename` or `git log -p filename`) rather than dumping the entire history into the context. -- **Synthesize**: Use the information to form a narrative about the code's lifecycle (e.g., "This was added in commit X to fix bug Y, but since we rewrote the bug Y subsystem, this is now dead code"). - -### 4. Investigating Regressions with Git Diff - -When debugging test failures or regressions, git history can pinpoint exactly what changed. - -**When the user proactively provides context:** -If the user says something like "the test was passing at commit ``, and the relevant changes are in ``", use this optimally: -- Run `git log --oneline ..HEAD -- ` to see which commits touched the area -- Run `git diff ..HEAD -- ` to get the **aggregate diff** (not serial diffs commit-by-commit) -- Cross-reference the diff with commit messages to understand developer intent -- The overall diff is mathematically equivalent to composing serial diffs, but far more token-efficient and cognitively cleaner - -**When debugging hits a roadblock:** -If direct code analysis and debug logging (`CURSOR_DEBUG_LOG`) aren't yielding answers, *then* ask the user: -- "Do you know when this test was last passing? If you have a commit hash and know which files/folders are likely involved, that would help narrow down what changed." -- Don't front-load this question—often the user doesn't know the answer. Try direct debugging first. - -**Avoid wasteful patterns:** -- Don't ask the user to provide diffs—ask for the commit hash and run git commands yourself -- Don't read diffs commit-by-commit and mentally compose them; use the aggregate diff -- Don't dump entire file histories; scope queries to the relevant path(s) - # Python Formatting & Pre-commit Every first-party Python repo (`orchestra`, `unify`, `unisdk`, `unillm`, `unify-deploy`) enforces formatting with **black** (plus `isort`/`autoflake`), and CI rejects unformatted code. A missing local hook or a drifting/duplicated black version is the single most common avoidable CI failure. This rule keeps local and CI identical so it stops blocking us. @@ -1352,46 +1312,6 @@ pre-commit run black --all-files # or: uv run black . / poetry run bl A fresh clone has no git hook until `pre-commit install` runs, so without it the first place formatting is ever checked is CI — which then blocks the PR. Pinning the formatters once in the locked `lint` group, and running that exact locked tool from both the hook and CI, removes every variant of the failure: "hook never ran", "version drift", and "duplicated pin disagreed". -This rule standardizes how we add temporary debug logging during failing tests and how we clean it up afterwards. ALWAYS use this process in agent sessions WHENEVER A TEST FAILS. This is the ONLY permitted way to address failing tests. - -1) Always start with hardcoded, unconditional debug logs -- Add logs immediately, without flags or guards. Do not gate behind environment variables or configuration. -- Use **only** the `CURSOR_DEBUG_LOG` function. No other logging method is permitted. -- **Finding the function**: Search for it with `rg "CURSOR_DEBUG_LOG"` to locate the utility in your project, then import and use it. - - **Python**: `from import CURSOR_DEBUG_LOG` then `CURSOR_DEBUG_LOG("message", variable)` - - **JavaScript/TypeScript**: `import { CURSOR_DEBUG_LOG } from ""` then `CURSOR_DEBUG_LOG("message", { variable })` -- Behavior: Prints an entry to stderr/stdout, making it easy to correlate with test runs. - -2) Python-specific import discipline -- **Self-contained imports**: Each debug snippet must include ALL its own imports inline (e.g., `import json as _json; import os as _os;`). Never rely on the file's existing imports. -- **Prefixed names**: Use underscore-prefixed aliases (`_json`, `_os`, `_pid`) to avoid shadowing. -- **Region markers**: Wrap in `# #region agent log` / `# #endregion` for easy identification and removal. -- This prevents `NameError` crashes when debug snippets reference modules that aren't imported at that location. - -3) Investigation workflow -- Step A: Add targeted debug calls around suspected code paths. -- Step B: Re-run the failing test(s) and inspect the new logs. -- Step C: If you are not 100% certain of the root cause, add more debug entries and repeat. -- Step D: Only when you are 100% confident of the cause, implement a direct fix (with or without keeping some logs briefly for confirmation). -- Step E: The user may repeatedly re-run tests and paste logs; continue iterating until the issue is definitively fixed. - -4) Cleanup policy -- After the fix is confirmed, remove all temporary logging. -- Grep to find every occurrence: - - ripgrep: `rg -n "CURSOR_DEBUG_LOG" -S` - - grep: `grep -Rin "CURSOR_DEBUG_LOG" .` -- Delete each call site (and any now-unused imports) before finalizing the fix. - -5) Alignment with workspace rules -- No fast paths or heuristics: Logging should not add conditional shortcuts; it merely reports state unconditionally. -- No exception-handling shields: Do not add defensive exception handling (try/catch, try/except) around the logs. Keep failures visible. -- No test details in production prompts: Temporary logs must not leak test-specific information into production prompts or docstrings. -- Rapid evolution: This logging is temporary by design; remove it once the issue is resolved—do not preserve backward compatibility. - -6) Intent and scope -- This logging exists solely for interactive debugging in agent sessions. -- The function name `CURSOR_DEBUG_LOG` is intentionally unique and grep-friendly to ensure quick cleanup on request. - # Full Local Stack First This is internal agent/developer guidance. It intentionally differs from the @@ -1450,191 +1370,72 @@ Prefer the durable full stack above. When isolated Orchestra is required **shared** instance for every agent on the machine. Do not stop/restart Orchestra from one agent session while another’s tests are using it. -# Local Stack Logs: Where To Look First - -When the user says something like *"we just did a local deployment and X happened, -check the logs to investigate"*, the logs almost always already exist on disk. Do -**not** re-explore the filesystem from scratch — go straight to the locations below. - -## 1. Central source of truth: `$UNIFY_REPO_PATH/logs/` +# Shared agent conversation archive -Default `~/unify/logs/`. Stack scripts may still export the legacy alias -`UNITY_REPO_PATH`; both refer to the same checkout. A local deployment aggregates -**every** repo's logs here — including Orchestra, which runs as a separate process. -The exact paths are set in `unify-deploy/selfhost/self_host_env.sh` (search -`*_LOG_DIR`); confirm the live values with `stack.sh status`. +Unify keeps a private repo of **raw** agent transcripts at **`~/shared_context`** +(GitHub: `unifyai/shared_context`), keyed by **GitHub login** (e.g. `djl11`). -| Dir | Env var | Contents | -|---|---|---| -| `logs/unillm/` | `UNILLM_LOG_DIR` | Raw LLM request/response, one `.txt` per call — system/user prompts, tool args, `reasoning_content`, model. This is **"what the model actually produced"**. | -| `logs/unisdk/` | `UNISDK_LOG_DIR` | UniSDK ↔ Orchestra HTTP traces (JSON per request). | -| `logs/orchestra/` | `ORCHESTRA_LOG_DIR` | Orchestra server-side per-request traces. | -| `logs/unify/` | `UNITY_LOG_DIR` | Unify runtime file logs (env var name is legacy). | -| `logs/all/` | `*_OTEL_LOG_DIR` | **Combined cross-repo OTel traces** — one `{trace_id}.jsonl` per request, with unify + unisdk + unillm (+ orchestra) spans stacked together. Use this for the end-to-end story of a single request. | -| `logs/pytest/`, `logs/ci/` | — | Test runs / downloaded-CI logs. | +## Design (important) -Deep reference (formats, env vars, examples): `/logs/README.md` (also present -in `unisdk/logs/README.md` and `unillm/logs/README.md`). +- **Adjacent clone, not a submodule.** `shared_context` sits next to product + checkouts (`~/unify`, `~/orchestra`, `~/brain`, …). It is **not** nested under + any public or private product repo. +- **Why:** `unify` (and other open repos) stay public; transcript data stays + private. Public cloners never need or see this tree. One clone serves agents + in **every** eng repo that pulls `unifyai/global-agent-rules`. +- **Applies everywhere** this rule is loaded: `unify`, `orchestra`, `unisdk`, + `unillm`, `unify-deploy`, `console`, `brain`, `docs`, `landing-page`, and any + other repo that includes these global rules. -## 2. CRITICAL: these dirs are gitignored AND cursorignored +## When to load this -`unify/.gitignore` has `logs/*`; `unify/.cursorignore` has `logs/` and `logs/**`. -Consequence — this is the usual reason an agent "can't find the logs": +Use before answering questions about past investigations or decisions across the +team — e.g. "did we set up X?", "who changed Y?", "why did we do Z?" — when the +answer might live in someone else's Cursor / Claude Code / Codex session, not +only the current chat. -- The built-in **Read / Grep / Glob tools return nothing**, "permission denied", or - "filtered out by .cursorignore" for anything under `logs/`. -- Plain `rg` / `grep` also **skip** these dirs (they respect `.gitignore`). +## How to search -Always inspect log dirs via the **Shell tool with ignore-bypass**, and read -individual files through the shell (not the Read tool): +Prefer ripgrep over reading whole files. Search **tracked** login trees only — +**do not** search `yours/` unless the user explicitly asks about their local / +unexported chats: ```bash -rg -uu -n "pattern" ~/unify/logs/unillm # -uu = --no-ignore --hidden -rg -uu -n . ~/unify/logs/all/.jsonl +rg -n -i "keyword" ~/shared_context/derived/index.jsonl +rg -n -i "keyword" ~/shared_context -g '!yours/**' -g '!tools/**' -g '!.git/**' ``` -## 3. Operational logs that live OUTSIDE the `logs/` tree - -- `~/.unity/service.log` — the self-host **stack supervisor**: startup, Orchestra - boot, gateway restarts, and the CM's own log path. Location is printed by - `stack.sh status`. (Note: may contain a one-off DB dump near a reset.) -- `/tmp/unity-local.log` — the **ConversationManager event "story"**: notifications, - guide/speak decisions, tool calls — the human-readable narrative of a live - conversation. Best first read for "what happened in this chat/call". -- `~/.unity/comms-bridge.log` — inbound email / SMS / WhatsApp polling. -- `~/.unity/call-tunnel.log` — cloudflared tunnel used for local phone/WhatsApp - call webhooks. LiveKit media itself is in LiveKit Cloud for source-stack runs. - -## 4. Ground truth that is NOT a file: Orchestra `Transcripts` context - -What was actually **spoken / sent / received** on calls and channels lives in -Orchestra's Postgres, not in a file log. When a file log shows the *intended* text -but you need the *downstream reality* (e.g. the TTS rendering vs. the LLM text), -query the `Transcripts` context (and `Contacts`, `Tasks`, …) via the UniSDK logs API -or Console: +`derived/index.jsonl` is rebuilt locally by `tools/sync.sh` / `tools/export.py` +after pull (gitignored). If it is missing, search tracked trees directly or run: ```bash -curl -s --get "http://127.0.0.1:8000/v0/logs" \ - --data-urlencode "project_name=Assistants" \ - --data-urlencode "context=//Transcripts" \ - --data-urlencode "limit=200" \ - -H "Authorization: Bearer $KEY" +python3 ~/shared_context/tools/export.py --index-only ``` -Local keys: the coordinator/owner API key is in `~/.unity/coordinator-runtime.json`; -`userId` is in `~/.unity/self-host-owner.json`. `unify_meet` rows are call -utterances (`sender_id` identifies the speaker). - -## Quick start - -1. `bash ~/unify-deploy/selfhost/stack.sh status` — running services + log paths. -2. `rg -uu` into `~/unify/logs/{all,unillm,unisdk,orchestra,unify}` for the request. -3. For one end-to-end request, open the matching `logs/all/{trace_id}.jsonl`. -4. For the conversation narrative, read `/tmp/unity-local.log`. -5. For what was truly spoken/received, query the Orchestra `Transcripts` context. - -# Deployed System Topology (shared context) - -This is broad orientation so agents in **any** repo know the shape of the deployed -system without re-exploring it every time. The **authoritative, exhaustive source of -truth** (every resource name, secret, CI trigger, and rename loose-end) is the -**`unify-deploy` repo root `README.md`**. Read that before deep infra work; do not -rediscover it from scratch. - -## Repos - -`unify` (runtime/brain, public), `unify-deploy` (private: hosted comms app + adapters, -assistant VM/tunnel infra, self-host stack, client overlay, prod CI/CD), `orchestra` -(backend API + Postgres, hosted), `console` (Next.js UI, hosted), `unisdk` (Python SDK, -public), `unillm` (LLM layer, public). Dependency `magnitude` is consumed at branch -`unity-modifications`. All repos: `main` = prod, `staging` = dev; promote `staging`→`main`. - -## GCP projects (4) - -| Project ID | Role | -|---|---| -| `responsive-city-458413-a2` (display "Unity LiveKit") | Main runtime: GKE cluster `unity`, Cloud Run `droid-comms-app`/`droid-adapters` (+`-staging`), Pub/Sub fleet, most buckets, tunnel servers, Artifact Registry | -| `unity-assistant-vms` | Assistant desktop VM pool: pool images/families, pool VMs, static IPs, per-assistant archives | -| `saas-368716` ("SaaS") | **Orchestra + Console + landing page** (Cloud Run) and **Cloud SQL** Postgres (`prod-ssd-usc1`/`staging-ssd-usc1`, us-central1) | -| `unify-dns-server` | Public DNS zone `unifyai` → `unify.ai` (incl. `vm.unify.ai`, `tunnel.unify.ai`) | - -## gcloud regions (don't trust the defaults) - -Almost every resource is **regional/zonal**, and several `gcloud` surfaces **default to the wrong location and return stale/empty output without erroring** — misreading that ("the build never ran", "service doesn't exist") is a recurring mistake. **Pass the location flag explicitly; suspect a wrong/`global` location before suspecting a wrong project.** - -- **Cloud Build is the #1 trap — it's regional and the `global` default lies.** All triggers + builds for **both** `responsive-city-458413-a2` and `saas-368716` are in **`us-central1`**. With no `--region`, `gcloud builds …` hits `global`, where `responsive-city-458413-a2` is **empty/months-stale** and `saas-368716` shows **only `landing-page`** (hiding all `orchestra`/`console` builds). Always: `gcloud builds {list,describe,log,triggers list,triggers run} --region=us-central1`. -- **GKE cluster `unity`**, Cloud Run comms/adapters, tunnel/pool VMs → **`us-central1`** (VM zones `-a`/`-f`). -- **saas Cloud Run** `orchestra`/`landing-page`/Console (prod + staging) → **`us-central1`**. **Cloud SQL** `prod-ssd-usc1`/`staging-ssd-usc1` → **`us-central1`**. (Consolidated from europe-west1/west3 in July 2026; the entire estate is now `us-central1`.) -- **Secret Manager** + **Cloud DNS** → global (no region flag; `--project` only). -- Org GitHub var `GCP_LOCATION=us-central1` is the saas Cloud Run default; the whole estate now shares `us-central1`. - -Full per-surface table + exact service names: `unify-deploy` README §3 "gcloud region/zone cheat-sheet". - -## Where things run - -- **Assistant runtime** = `unity` container as on-demand GKE Jobs (label `app=droid`) on cluster `unity`. Idle→live via Pub/Sub `droid-startup[-staging]` / `droid-{assistant_id}[-staging]`. 7-min inactivity timeout; jobs retained for logs; `job-watcher` (kopf) does crash-safe cleanup. -- **Hosted comms**: adapters (inbound webhooks) + comms app (outbound + infra control plane `/infra/*`) on Cloud Run; the comms-app image also runs the GKE `assistant-session-controller`/`-pool-controller`. -- **Assistant desktops**: pooled Ubuntu/Windows VMs in `unity-assistant-vms`; runtime syncs `~/Unity/Local` over rclone SFTP (user `unityuser`, port 2222); cross-session home persisted to `gs://droid-assistant-archives/{id}.tar.gz`; optional rathole tunnel relay (`unity-tunnel-server`) for user machines. -- **Backend/UI/DB** all in `saas-368716`. Orchestra at `https://api.unify.ai/v0`. -- **Fleet audit** (`AssistantJobs`): Orchestra `is_system` project; writes via - comms `/infra/assistant-jobs/*` (pod `UNIFY_KEY`) or Console hosted reads with - `ORCHESTRA_ADMIN_KEY` as `__system__`. Never a Workspace `User` API key. - -## Legacy Resource Naming Reality (critical) - -The platform has gone through additive renames rather than a single in-place resource cutover. -Consequence: some live GCP/GitHub resources still intentionally use legacy `droid-*` or -`unity-*` names. A name mismatch -fails at *runtime* (404/401/empty config), not at build — this has caused silent prod outages. - -**Immutable / permanently `unity` (do NOT try to "fix" in code — code targets these on purpose):** -project IDs `unity-assistant-vms` & `responsive-city-458413-a2`, all service-account emails -(`pool-vm-sa@unity-assistant-vms`, `comm-sa@responsive-city-458413-a2`), and GKE cluster `unity` -(`DROID_GKE_CLUSTER_NAME` default `unity`). - -**Canonical names that commonly confuse (use these, verify before assuming):** -- GKE cluster: `unity`; VM/image project: `unity-assistant-vms`. -- Tunnel: VM `unity-tunnel-server`, bucket `unity-tunnel-config` (there is **no** `droid-tunnel-config`). -- Desktop pool: Ubuntu migrated to `droid-pool-ubuntu-*` (image family `droid-pool-ubuntu-vm`); **Windows still `unity-pool-windows-*`**. -- Archive bucket: `droid-assistant-archives` (live); `unity-assistant-archives` is legacy rollback. -- Data buckets (recordings/logs/artifacts) and ~84% of Pub/Sub topics/subs are still `unity-*`. -- CI: GitHub org secrets are still `UNITY_ADAPTERS_URL`/`UNITY_COMMS_URL` while workflows read `DROID_*` (so they can resolve empty). -- Deliberate legacy-named identifiers (not typos): `UnitySystemEvent` gateway envelope (unity↔console wire contract), `UnityTests` default test project, `unity-user-filesync` SSH key comment, `WaitingForUnity` state labels, `magnitude@unity-modifications`. - -When something infra-related "doesn't exist" or 404s/401s, suspect a legacy resource-name mismatch -and confirm the real resource name against the `unify-deploy` README (or `gcloud`/`gh`) rather -than trusting the code constant. - -# Orchestra Admin vs User API Access +Sessions live at +`/{cursor|codex|claude-code}///{meta.json,transcript.jsonl}`. -Orchestra (`api.unify.ai/v0` prod, `api.staging.internal.saas.unify.ai/v0` staging) has **two distinct auth paths** (`orchestra/web/api/dependencies.py`). Confusing them wastes hours. +`yours/{cursor,codex,claude-code}` are local symlinks to personal stores and are +gitignored. -## The two dependencies +If `~/shared_context` is missing, say so and suggest: -- **`auth_api_key`** — looks the Bearer token up as a **user API key** and sets `request.state.user_id`. All **data** endpoints use it (`/logs` get/update/`atomic_field_update`, contexts, dashboards, etc.). Results are **scoped to that key's owner**. There is **no admin bypass** here. -- **`auth_admin_key`** — matches the Bearer against the server's `ORCHESTRA_ADMIN_KEY` (`secrets.compare_digest`), a Cloud Scheduler OIDC token, or an `AdminUser`'s key. Only the **`/admin/*`** routers (registered with `ADMIN_AUTH`) use it. It **gates operations; it does not grant a data scope.** +```bash +git clone git@github.com:unifyai/shared_context.git ~/shared_context +``` -### Consequences (do not relearn these the hard way) -- `ORCHESTRA_ADMIN_KEY` on a data endpoint → **`401 {"detail":"Invalid API key"}`** (because `/logs` only does user-key lookup). This is expected, not a broken key. -- Even an **admin user's own** `UNIFY_KEY` is data-scoped: it returns `0` for another tenant's contexts. Admin status does not widen `/logs` results. -- The live `ORCHESTRA_ADMIN_KEY` is the **GCP secret** in `saas-368716` (prod) — repo `.env` copies may be stale. Staging uses a different value. Fetch with `gcloud secrets versions access latest --secret=ORCHESTRA_ADMIN_KEY --project=saas-368716`. +Do **not** suggest `git submodule add` / nesting it under a product repo. -## How to read/write a specific tenant's data smoothly +## Citing -Use the admin API to fetch the target's **own** API key, then use that key on the normal data endpoints: +Cite **user**, **tool**, **date**, and **path** so a human can open the same session. -1. **Enumerate + get keys** (admin key): `GET /admin/assistant` returns every assistant including its `api_key`, `agent_id`, `user_id` (also `GET /admin/assistant/{id}`, `/admin/assistant/user/{user_id}`). - ```bash - curl -s --get "$ORCHESTRA_URL/admin/assistant" -H "Authorization: Bearer $ORCHESTRA_ADMIN_KEY" - ``` -2. **Use the assistant's `api_key`** as the Bearer on the data API — now scoped to that tenant: - ```python - import unify - logs = unify.get_logs(project="Assistants", filter="'x' in content", api_key=assistant_key) - unify.update_logs(logs=log_id, entries={"content": new}, context=ctx, api_key=assistant_key) - ``` +## Do not -A cross-tenant migration = loop assistants from `/admin/assistant`, then operate per-assistant with each key (no superuser data key exists). `gcloud` Cloud SQL (`prod-ssd`, `saas-368716`) is the direct-DB fallback for bulk passes. +- Do not confuse this with `brain` (curated company memory). +- Do not scrub or rewrite historical transcripts. +- Do not push/sync unless the user asked you to. +- Do not grep `yours/` unless the user asked for local-only context. # Orchestra / DataManager: Server-Side Queries First @@ -1734,73 +1535,102 @@ keyword and must not be renamed. - Playbook with copy-paste recipes: `brain/docs/operations/orchestra-data-access.md` -# TaskScheduler surgery (Tasks rows) +# Infra command safety -Agents frequently break recurring jobs by hand-editing `Teams/*/Tasks` -(or assistant-scoped `…/Tasks`) via DataManager / UniSDK. Follow this rule for -**any** TaskScheduler ops across brain, unify, and orchestra. +Two traps here fail *silently* — wrong output rather than an error — so they +cannot be discovered by trying. -## Identity model +**`gcloud` is regional and the default lies.** Most resources are regional or +zonal, and several `gcloud` surfaces default to `global` and return stale or +empty output **without erroring**. Cloud Build is the worst: with no +`--region`, `gcloud builds …` hits `global`, where the main project looks +empty and the saas project hides every `orchestra`/`console` build. Reading +that as "the build never ran" or "the service doesn't exist" is a recurring +mistake. Pass the location flag explicitly — the estate is `us-central1` — +and suspect a wrong location before a wrong project. -- **`Tasks`** is definition-only: **one row per `task_id`**, the whole series. - `unique_keys={"task_id": "int"}`, `auto_counting={"task_id": None}` - (`unify/task_scheduler/task_scheduler.py`). -- **`Tasks/Executions`** holds the runs: one row per wake/attempt, keyed by - `run_key` (the idempotency key). Occurrence and attempt are the same row. - Recurrence creates the *next* Execution when the current one **starts** — it - does **not** clone the Tasks row. -- **`instance_id` is vestigial.** It is a legacy occurrence counter kept only so - pre-migration rows still read back. It is not unique, not auto-counted, and - not part of identity; new rows get `0`. Treat any non-zero `instance_id` as a - pre-migration artefact, not as a thing to allocate, increment, or reason about. -- Concurrency is normal now: several Executions can be in flight against one - definition, so a definition sitting in `active` is not a zombie by itself. +**Some live resources keep legacy `droid-*` / `unity-*` names on purpose.** +The platform was renamed additively, so code targets those names deliberately. +A name mismatch fails at *runtime* (404/401/empty config), not at build, and +has caused silent production outages. When something infra-related 404s or +"doesn't exist", suspect a legacy name before changing the code constant. -## Hard refuse +Full topology — projects, regions, where things run, and the exhaustive +legacy-name list — is in +[`.agents/global-rules/situational/deployed-system-topology.md`](.agents/global-rules/situational/deployed-system-topology.md). +Read it before any non-trivial infra work. -- Do **not** set or change `task_id` on an existing row (Orchestra rejects - writes to auto-counted unique identity fields). -- Do **not** resurrect `cancelled` / `failed` / `completed` rows by flipping - them back to `scheduled` (or rewriting their `schedule`). Same for terminal - Executions. -- Do **not** invent a Tasks row by hand with an explicit `task_id` — go through - TaskScheduler APIs and let Orchestra allocate it. -- Do **not** write `instance_id` at all. It buys nothing on the current model, - and a non-zero value pushes reads down the legacy compat path (see below). +# Deployed System Topology (shared context) -## Allowed ops +This is broad orientation so agents in **any** repo know the shape of the deployed +system without re-exploring it every time. The **authoritative, exhaustive source of +truth** (every resource name, secret, CI trigger, and rename loose-end) is the +**`unify-deploy` repo root `README.md`**. Read that before deep infra work; do not +rediscover it from scratch. -| Goal | How | +## Repos + +`unify` (runtime/brain, public), `unify-deploy` (private: hosted comms app + adapters, +assistant VM/tunnel infra, self-host stack, client overlay, prod CI/CD), `orchestra` +(backend API + Postgres, hosted), `console` (Next.js UI, hosted), `unisdk` (Python SDK, +public), `unillm` (LLM layer, public). Dependency `magnitude` is consumed at branch +`unity-modifications`. All repos: `main` = prod, `staging` = dev; promote `staging`→`main`. + +## GCP projects (4) + +| Project ID | Role | |---|---| -| Arm a planted custom task | Set **`enabled=True`** on the definition row (the single `task_id` row, `custom_key` set). TaskScheduler schedules the next Execution. | -| Pause | `enabled=False` on the definition row; optionally cancel open Executions. | -| One-off catch-up / run now | `POST /v0/tasks/{task_id}/trigger` (`trigger_task(task_id=…)` in `typed_tasks_client`). It takes no `instance_id`. | -| Change cadence | Edit `tasks.jsonl` + deploy reconcile, or TaskScheduler APIs that own the schedule — not ad-hoc DM patches. | -| Stuck `active` zombie | `POST /admin/task-source/release-active` with the source task log id. | +| `responsive-city-458413-a2` (display "Unity LiveKit") | Main runtime: GKE cluster `unity`, Cloud Run `droid-comms-app`/`droid-adapters` (+`-staging`), Pub/Sub fleet, most buckets, tunnel servers, Artifact Registry | +| `unity-assistant-vms` | Assistant desktop VM pool: pool images/families, pool VMs, static IPs, per-assistant archives | +| `saas-368716` ("SaaS") | **Orchestra + Console + landing page** (Cloud Run) and **Cloud SQL** Postgres (`prod-ssd-usc1`/`staging-ssd-usc1`, us-central1) | +| `unify-dns-server` | Public DNS zone `unifyai` → `unify.ai` (incl. `vm.unify.ai`, `tunnel.unify.ai`) | -## Legacy compat path +## gcloud regions (don't trust the defaults) -`_get_task_row(task_id, instance_id)` addresses by `task_id` alone when -`instance_id == 0`, and only falls back to the old -`task_id AND instance_id` filter when it is non-zero -(`unify/task_scheduler/task_scheduler.py`, the `if instance_id != 0:` branch). -That fallback exists to read surviving pre-migration rows. Do not lean on it -for new work, and do not pass a non-zero `instance_id` to make a lookup -"more specific" — on a post-migration task it just fails to match. +Almost every resource is **regional/zonal**, and several `gcloud` surfaces **default to the wrong location and return stale/empty output without erroring** — misreading that ("the build never ran", "service doesn't exist") is a recurring mistake. **Pass the location flag explicitly; suspect a wrong/`global` location before suspecting a wrong project.** -If a `task_id` genuinely resolves to more than one Tasks row, that is a -pre-migration remnant (or a bad hand-write), not a counter desync. Delete the -stale duplicate, or leave it terminal and do not re-trigger until the health -check is clean. +- **Cloud Build is the #1 trap — it's regional and the `global` default lies.** All triggers + builds for **both** `responsive-city-458413-a2` and `saas-368716` are in **`us-central1`**. With no `--region`, `gcloud builds …` hits `global`, where `responsive-city-458413-a2` is **empty/months-stale** and `saas-368716` shows **only `landing-page`** (hiding all `orchestra`/`console` builds). Always: `gcloud builds {list,describe,log,triggers list,triggers run} --region=us-central1`. +- **GKE cluster `unity`**, Cloud Run comms/adapters, tunnel/pool VMs → **`us-central1`** (VM zones `-a`/`-f`). +- **saas Cloud Run** `orchestra`/`landing-page`/Console (prod + staging) → **`us-central1`**. **Cloud SQL** `prod-ssd-usc1`/`staging-ssd-usc1` → **`us-central1`**. (Consolidated from europe-west1/west3 in July 2026; the entire estate is now `us-central1`.) +- **Secret Manager** + **Cloud DNS** → global (no region flag; `--project` only). +- Org GitHub var `GCP_LOCATION=us-central1` is the saas Cloud Run default; the whole estate now shares `us-central1`. -## Break-glass +Full per-surface table + exact service names: `unify-deploy` README §3 "gcloud region/zone cheat-sheet". -Only with an explicit operator rationale: fail an `active` zombie via -`POST /admin/task-source/release-active`, then clean up **extra** open -Executions so one next wake remains. +## Where things run -Ops detail: brain `docs/operations/scheduled-jobs.md` (re-arm / disable / -catch-up). Health check: `python3 -m scripts.tasks_health_check`. +- **Assistant runtime** = `unity` container as on-demand GKE Jobs (label `app=droid`) on cluster `unity`. Idle→live via Pub/Sub `droid-startup[-staging]` / `droid-{assistant_id}[-staging]`. 7-min inactivity timeout; jobs retained for logs; `job-watcher` (kopf) does crash-safe cleanup. +- **Hosted comms**: adapters (inbound webhooks) + comms app (outbound + infra control plane `/infra/*`) on Cloud Run; the comms-app image also runs the GKE `assistant-session-controller`/`-pool-controller`. +- **Assistant desktops**: pooled Ubuntu/Windows VMs in `unity-assistant-vms`; runtime syncs `~/Unity/Local` over rclone SFTP (user `unityuser`, port 2222); cross-session home persisted to `gs://droid-assistant-archives/{id}.tar.gz`; optional rathole tunnel relay (`unity-tunnel-server`) for user machines. +- **Backend/UI/DB** all in `saas-368716`. Orchestra at `https://api.unify.ai/v0`. +- **Fleet audit** (`AssistantJobs`): Orchestra `is_system` project; writes via + comms `/infra/assistant-jobs/*` (pod `UNIFY_KEY`) or Console hosted reads with + `ORCHESTRA_ADMIN_KEY` as `__system__`. Never a Workspace `User` API key. + +## Legacy Resource Naming Reality (critical) + +The platform has gone through additive renames rather than a single in-place resource cutover. +Consequence: some live GCP/GitHub resources still intentionally use legacy `droid-*` or +`unity-*` names. A name mismatch +fails at *runtime* (404/401/empty config), not at build — this has caused silent prod outages. + +**Immutable / permanently `unity` (do NOT try to "fix" in code — code targets these on purpose):** +project IDs `unity-assistant-vms` & `responsive-city-458413-a2`, all service-account emails +(`pool-vm-sa@unity-assistant-vms`, `comm-sa@responsive-city-458413-a2`), and GKE cluster `unity` +(`DROID_GKE_CLUSTER_NAME` default `unity`). + +**Canonical names that commonly confuse (use these, verify before assuming):** +- GKE cluster: `unity`; VM/image project: `unity-assistant-vms`. +- Tunnel: VM `unity-tunnel-server`, bucket `unity-tunnel-config` (there is **no** `droid-tunnel-config`). +- Desktop pool: Ubuntu migrated to `droid-pool-ubuntu-*` (image family `droid-pool-ubuntu-vm`); **Windows still `unity-pool-windows-*`**. +- Archive bucket: `droid-assistant-archives` (live); `unity-assistant-archives` is legacy rollback. +- Data buckets (recordings/logs/artifacts) and ~84% of Pub/Sub topics/subs are still `unity-*`. +- CI: GitHub org secrets are still `UNITY_ADAPTERS_URL`/`UNITY_COMMS_URL` while workflows read `DROID_*` (so they can resolve empty). +- Deliberate legacy-named identifiers (not typos): `UnitySystemEvent` gateway envelope (unity↔console wire contract), `UnityTests` default test project, `unity-user-filesync` SSH key comment, `WaitingForUnity` state labels, `magnitude@unity-modifications`. + +When something infra-related "doesn't exist" or 404s/401s, suspect a legacy resource-name mismatch +and confirm the real resource name against the `unify-deploy` README (or `gcloud`/`gh`) rather +than trusting the code constant. # Fleet audit auth (AssistantJobs) @@ -1829,6 +1659,160 @@ discovery) is authenticated as Orchestra **`__system__`** via `deploy/scripts/dev/verify_assistant_jobs_system.py` after Orchestra or secret changes. +# Git History for Context + +## Context +This rule applies when you are trying to understand the *rationale* behind specific code blocks, the evolution of a module, or when deciding whether "weird" looking code is essential or legacy technical debt. + +## Rules + +### 1. Strategic Git Usage +- **Use as a Second Level of Analysis**: If the code's purpose isn't clear from the current state alone (static analysis), use `git blame` or `git log` to uncover the "why". +- **Not a Mandate**: Do not check git history for every file you touch. This creates noise. Use it selectively when you lack context. + +### 2. Understanding Code Evolution +- **Identify Legacy Code**: If you suspect code is redundant or outdated, check its commit date and message. If it was added months ago for a feature that is no longer relevant, this confirms it can likely be purged. +- **Find the "Why"**: Expressive commit messages often contain the reasoning that comments lack. Use them to understand the author's original intent before refactoring or deleting complex logic. + +### 3. Targeted Queries +- **Be Surgical**: When querying git, look for the history of specific lines or changes (e.g., `git blame -L n,m filename` or `git log -p filename`) rather than dumping the entire history into the context. +- **Synthesize**: Use the information to form a narrative about the code's lifecycle (e.g., "This was added in commit X to fix bug Y, but since we rewrote the bug Y subsystem, this is now dead code"). + +### 4. Investigating Regressions with Git Diff + +When debugging test failures or regressions, git history can pinpoint exactly what changed. + +**When the user proactively provides context:** +If the user says something like "the test was passing at commit ``, and the relevant changes are in ``", use this optimally: +- Run `git log --oneline ..HEAD -- ` to see which commits touched the area +- Run `git diff ..HEAD -- ` to get the **aggregate diff** (not serial diffs commit-by-commit) +- Cross-reference the diff with commit messages to understand developer intent +- The overall diff is mathematically equivalent to composing serial diffs, but far more token-efficient and cognitively cleaner + +**When debugging hits a roadblock:** +If direct code analysis and debug logging (`CURSOR_DEBUG_LOG`) aren't yielding answers, *then* ask the user: +- "Do you know when this test was last passing? If you have a commit hash and know which files/folders are likely involved, that would help narrow down what changed." +- Don't front-load this question—often the user doesn't know the answer. Try direct debugging first. + +**Avoid wasteful patterns:** +- Don't ask the user to provide diffs—ask for the commit hash and run git commands yourself +- Don't read diffs commit-by-commit and mentally compose them; use the aggregate diff +- Don't dump entire file histories; scope queries to the relevant path(s) + +# Local Stack Logs: Where To Look First + +When the user says something like *"we just did a local deployment and X happened, +check the logs to investigate"*, the logs almost always already exist on disk. Do +**not** re-explore the filesystem from scratch — go straight to the locations below. + +## 1. Central source of truth: `$UNIFY_REPO_PATH/logs/` + +Default `~/unify/logs/`. Stack scripts may still export the legacy alias +`UNITY_REPO_PATH`; both refer to the same checkout. A local deployment aggregates +**every** repo's logs here — including Orchestra, which runs as a separate process. +The exact paths are set in `unify-deploy/selfhost/self_host_env.sh` (search +`*_LOG_DIR`); confirm the live values with `stack.sh status`. + +| Dir | Env var | Contents | +|---|---|---| +| `logs/unillm/` | `UNILLM_LOG_DIR` | Raw LLM request/response, one `.txt` per call — system/user prompts, tool args, `reasoning_content`, model. This is **"what the model actually produced"**. | +| `logs/unisdk/` | `UNISDK_LOG_DIR` | UniSDK ↔ Orchestra HTTP traces (JSON per request). | +| `logs/orchestra/` | `ORCHESTRA_LOG_DIR` | Orchestra server-side per-request traces. | +| `logs/unify/` | `UNITY_LOG_DIR` | Unify runtime file logs (env var name is legacy). | +| `logs/all/` | `*_OTEL_LOG_DIR` | **Combined cross-repo OTel traces** — one `{trace_id}.jsonl` per request, with unify + unisdk + unillm (+ orchestra) spans stacked together. Use this for the end-to-end story of a single request. | +| `logs/pytest/`, `logs/ci/` | — | Test runs / downloaded-CI logs. | + +Deep reference (formats, env vars, examples): `/logs/README.md` (also present +in `unisdk/logs/README.md` and `unillm/logs/README.md`). + +## 2. CRITICAL: these dirs are gitignored AND cursorignored + +`unify/.gitignore` has `logs/*`; `unify/.cursorignore` has `logs/` and `logs/**`. +Consequence — this is the usual reason an agent "can't find the logs": + +- The built-in **Read / Grep / Glob tools return nothing**, "permission denied", or + "filtered out by .cursorignore" for anything under `logs/`. +- Plain `rg` / `grep` also **skip** these dirs (they respect `.gitignore`). + +Always inspect log dirs via the **Shell tool with ignore-bypass**, and read +individual files through the shell (not the Read tool): + +```bash +rg -uu -n "pattern" ~/unify/logs/unillm # -uu = --no-ignore --hidden +rg -uu -n . ~/unify/logs/all/.jsonl +``` + +## 3. Operational logs that live OUTSIDE the `logs/` tree + +- `~/.unity/service.log` — the self-host **stack supervisor**: startup, Orchestra + boot, gateway restarts, and the CM's own log path. Location is printed by + `stack.sh status`. (Note: may contain a one-off DB dump near a reset.) +- `/tmp/unity-local.log` — the **ConversationManager event "story"**: notifications, + guide/speak decisions, tool calls — the human-readable narrative of a live + conversation. Best first read for "what happened in this chat/call". +- `~/.unity/comms-bridge.log` — inbound email / SMS / WhatsApp polling. +- `~/.unity/call-tunnel.log` — cloudflared tunnel used for local phone/WhatsApp + call webhooks. LiveKit media itself is in LiveKit Cloud for source-stack runs. + +## 4. Ground truth that is NOT a file: Orchestra `Transcripts` context + +What was actually **spoken / sent / received** on calls and channels lives in +Orchestra's Postgres, not in a file log. When a file log shows the *intended* text +but you need the *downstream reality* (e.g. the TTS rendering vs. the LLM text), +query the `Transcripts` context (and `Contacts`, `Tasks`, …) via the UniSDK logs API +or Console: + +```bash +curl -s --get "http://127.0.0.1:8000/v0/logs" \ + --data-urlencode "project_name=Assistants" \ + --data-urlencode "context=//Transcripts" \ + --data-urlencode "limit=200" \ + -H "Authorization: Bearer $KEY" +``` + +Local keys: the coordinator/owner API key is in `~/.unity/coordinator-runtime.json`; +`userId` is in `~/.unity/self-host-owner.json`. `unify_meet` rows are call +utterances (`sender_id` identifies the speaker). + +## Quick start + +1. `bash ~/unify-deploy/selfhost/stack.sh status` — running services + log paths. +2. `rg -uu` into `~/unify/logs/{all,unillm,unisdk,orchestra,unify}` for the request. +3. For one end-to-end request, open the matching `logs/all/{trace_id}.jsonl`. +4. For the conversation narrative, read `/tmp/unity-local.log`. +5. For what was truly spoken/received, query the Orchestra `Transcripts` context. + +# Orchestra Admin vs User API Access + +Orchestra (`api.unify.ai/v0` prod, `api.staging.internal.saas.unify.ai/v0` staging) has **two distinct auth paths** (`orchestra/web/api/dependencies.py`). Confusing them wastes hours. + +## The two dependencies + +- **`auth_api_key`** — looks the Bearer token up as a **user API key** and sets `request.state.user_id`. All **data** endpoints use it (`/logs` get/update/`atomic_field_update`, contexts, dashboards, etc.). Results are **scoped to that key's owner**. There is **no admin bypass** here. +- **`auth_admin_key`** — matches the Bearer against the server's `ORCHESTRA_ADMIN_KEY` (`secrets.compare_digest`), a Cloud Scheduler OIDC token, or an `AdminUser`'s key. Only the **`/admin/*`** routers (registered with `ADMIN_AUTH`) use it. It **gates operations; it does not grant a data scope.** + +### Consequences (do not relearn these the hard way) +- `ORCHESTRA_ADMIN_KEY` on a data endpoint → **`401 {"detail":"Invalid API key"}`** (because `/logs` only does user-key lookup). This is expected, not a broken key. +- Even an **admin user's own** `UNIFY_KEY` is data-scoped: it returns `0` for another tenant's contexts. Admin status does not widen `/logs` results. +- The live `ORCHESTRA_ADMIN_KEY` is the **GCP secret** in `saas-368716` (prod) — repo `.env` copies may be stale. Staging uses a different value. Fetch with `gcloud secrets versions access latest --secret=ORCHESTRA_ADMIN_KEY --project=saas-368716`. + +## How to read/write a specific tenant's data smoothly + +Use the admin API to fetch the target's **own** API key, then use that key on the normal data endpoints: + +1. **Enumerate + get keys** (admin key): `GET /admin/assistant` returns every assistant including its `api_key`, `agent_id`, `user_id` (also `GET /admin/assistant/{id}`, `/admin/assistant/user/{user_id}`). + ```bash + curl -s --get "$ORCHESTRA_URL/admin/assistant" -H "Authorization: Bearer $ORCHESTRA_ADMIN_KEY" + ``` +2. **Use the assistant's `api_key`** as the Bearer on the data API — now scoped to that tenant: + ```python + import unify + logs = unify.get_logs(project="Assistants", filter="'x' in content", api_key=assistant_key) + unify.update_logs(logs=log_id, entries={"content": new}, context=ctx, api_key=assistant_key) + ``` + +A cross-tenant migration = loop assistants from `/admin/assistant`, then operate per-assistant with each key (no superuser data key exists). `gcloud` Cloud SQL (`prod-ssd`, `saas-368716`) is the direct-DB fallback for bulk passes. + # OAuth Scopes: Mirrored Between Communication and Orchestra ## Context @@ -1868,3 +1852,111 @@ When asked to add or modify a scope in one repo: ### 4. Keep the Cross-Reference Comment Accurate The docstring at the top of each `scopes.py` points at its sibling. If either file moves, update both docstrings. + +# TaskScheduler surgery (Tasks rows) + +Agents frequently break recurring jobs by hand-editing `Teams/*/Tasks` +(or assistant-scoped `…/Tasks`) via DataManager / UniSDK. Follow this rule for +**any** TaskScheduler ops across brain, unify, and orchestra. + +## Identity model + +- **`Tasks`** is definition-only: **one row per `task_id`**, the whole series. + `unique_keys={"task_id": "int"}`, `auto_counting={"task_id": None}` + (`unify/task_scheduler/task_scheduler.py`). +- **`Tasks/Executions`** holds the runs: one row per wake/attempt, keyed by + `run_key` (the idempotency key). Occurrence and attempt are the same row. + Recurrence creates the *next* Execution when the current one **starts** — it + does **not** clone the Tasks row. +- **`instance_id` is vestigial.** It is a legacy occurrence counter kept only so + pre-migration rows still read back. It is not unique, not auto-counted, and + not part of identity; new rows get `0`. Treat any non-zero `instance_id` as a + pre-migration artefact, not as a thing to allocate, increment, or reason about. +- Concurrency is normal now: several Executions can be in flight against one + definition, so a definition sitting in `active` is not a zombie by itself. + +## Hard refuse + +- Do **not** set or change `task_id` on an existing row (Orchestra rejects + writes to auto-counted unique identity fields). +- Do **not** resurrect `cancelled` / `failed` / `completed` rows by flipping + them back to `scheduled` (or rewriting their `schedule`). Same for terminal + Executions. +- Do **not** invent a Tasks row by hand with an explicit `task_id` — go through + TaskScheduler APIs and let Orchestra allocate it. +- Do **not** write `instance_id` at all. It buys nothing on the current model, + and a non-zero value pushes reads down the legacy compat path (see below). + +## Allowed ops + +| Goal | How | +|---|---| +| Arm a planted custom task | Set **`enabled=True`** on the definition row (the single `task_id` row, `custom_key` set). TaskScheduler schedules the next Execution. | +| Pause | `enabled=False` on the definition row; optionally cancel open Executions. | +| One-off catch-up / run now | `POST /v0/tasks/{task_id}/trigger` (`trigger_task(task_id=…)` in `typed_tasks_client`). It takes no `instance_id`. | +| Change cadence | Edit `tasks.jsonl` + deploy reconcile, or TaskScheduler APIs that own the schedule — not ad-hoc DM patches. | +| Stuck `active` zombie | `POST /admin/task-source/release-active` with the source task log id. | + +## Legacy compat path + +`_get_task_row(task_id, instance_id)` addresses by `task_id` alone when +`instance_id == 0`, and only falls back to the old +`task_id AND instance_id` filter when it is non-zero +(`unify/task_scheduler/task_scheduler.py`, the `if instance_id != 0:` branch). +That fallback exists to read surviving pre-migration rows. Do not lean on it +for new work, and do not pass a non-zero `instance_id` to make a lookup +"more specific" — on a post-migration task it just fails to match. + +If a `task_id` genuinely resolves to more than one Tasks row, that is a +pre-migration remnant (or a bad hand-write), not a counter desync. Delete the +stale duplicate, or leave it terminal and do not re-trigger until the health +check is clean. + +## Break-glass + +Only with an explicit operator rationale: fail an `active` zombie via +`POST /admin/task-source/release-active`, then clean up **extra** open +Executions so one next wake remains. + +Ops detail: brain `docs/operations/scheduled-jobs.md` (re-arm / disable / +catch-up). Health check: `python3 -m scripts.tasks_health_check`. + +This rule standardizes how we add temporary debug logging during failing tests and how we clean it up afterwards. ALWAYS use this process in agent sessions WHENEVER A TEST FAILS. This is the ONLY permitted way to address failing tests. + +1) Always start with hardcoded, unconditional debug logs +- Add logs immediately, without flags or guards. Do not gate behind environment variables or configuration. +- Use **only** the `CURSOR_DEBUG_LOG` function. No other logging method is permitted. +- **Finding the function**: Search for it with `rg "CURSOR_DEBUG_LOG"` to locate the utility in your project, then import and use it. + - **Python**: `from import CURSOR_DEBUG_LOG` then `CURSOR_DEBUG_LOG("message", variable)` + - **JavaScript/TypeScript**: `import { CURSOR_DEBUG_LOG } from ""` then `CURSOR_DEBUG_LOG("message", { variable })` +- Behavior: Prints an entry to stderr/stdout, making it easy to correlate with test runs. + +2) Python-specific import discipline +- **Self-contained imports**: Each debug snippet must include ALL its own imports inline (e.g., `import json as _json; import os as _os;`). Never rely on the file's existing imports. +- **Prefixed names**: Use underscore-prefixed aliases (`_json`, `_os`, `_pid`) to avoid shadowing. +- **Region markers**: Wrap in `# #region agent log` / `# #endregion` for easy identification and removal. +- This prevents `NameError` crashes when debug snippets reference modules that aren't imported at that location. + +3) Investigation workflow +- Step A: Add targeted debug calls around suspected code paths. +- Step B: Re-run the failing test(s) and inspect the new logs. +- Step C: If you are not 100% certain of the root cause, add more debug entries and repeat. +- Step D: Only when you are 100% confident of the cause, implement a direct fix (with or without keeping some logs briefly for confirmation). +- Step E: The user may repeatedly re-run tests and paste logs; continue iterating until the issue is definitively fixed. + +4) Cleanup policy +- After the fix is confirmed, remove all temporary logging. +- Grep to find every occurrence: + - ripgrep: `rg -n "CURSOR_DEBUG_LOG" -S` + - grep: `grep -Rin "CURSOR_DEBUG_LOG" .` +- Delete each call site (and any now-unused imports) before finalizing the fix. + +5) Alignment with workspace rules +- No fast paths or heuristics: Logging should not add conditional shortcuts; it merely reports state unconditionally. +- No exception-handling shields: Do not add defensive exception handling (try/catch, try/except) around the logs. Keep failures visible. +- No test details in production prompts: Temporary logs must not leak test-specific information into production prompts or docstrings. +- Rapid evolution: This logging is temporary by design; remove it once the issue is resolved—do not preserve backward compatibility. + +6) Intent and scope +- This logging exists solely for interactive debugging in agent sessions. +- The function name `CURSOR_DEBUG_LOG` is intentionally unique and grep-friendly to ensure quick cleanup on request. diff --git a/agent-service/src/index.ts b/agent-service/src/index.ts index 10077d20e..cfa7ad550 100644 --- a/agent-service/src/index.ts +++ b/agent-service/src/index.ts @@ -843,6 +843,25 @@ const startBrowserOnVm = async ( } // --- Google Meet browser launcher --- + +// Fixed page geometry for the Meet browser. +// +// The width is capped by the vision budget, not by taste: the planner receives +// a screenshot scaled only by 1/devicePixelRatio (magnitude-core's WebHarness) +// and answers in CSS pixels, so the image must reach the model unresized or its +// coordinates land short of their target. ``modelObservationMaxEdge`` in +// ``unify/common/observation_scaling_policy.json`` puts that ceiling at 1568px +// for the Claude models this service runs on, and 1280x800 is one of the +// declared ``defaultAspectTargets`` underneath it — wide enough for Meet's +// desktop layout, small enough to survive the trip intact. +const MEET_VIEWPORT = { width: 1280, height: 800 }; + +// The OS window has to outsize the page it hosts, or the tab renders into +// something smaller than the viewport it was promised and the screenshot no +// longer matches what a click at those coordinates will hit. The slack covers +// the tab strip and omnibox. +const MEET_WINDOW_HEIGHT = MEET_VIEWPORT.height + 120; + const startGoogleMeetBrowser = async ( meetUrl: string, storageStateName?: string, @@ -862,6 +881,8 @@ const startGoogleMeetBrowser = async ( "--disable-features=IsolateOrigins,site-per-process", '--auto-select-desktop-capture-source="Entire screen"', '--auto-select-tab-capture-source-by-title=Desktop', + `--window-size=${MEET_VIEWPORT.width},${MEET_WINDOW_HEIGHT}`, + "--window-position=0,0", ], env: { ...process.env, @@ -872,10 +893,23 @@ const startGoogleMeetBrowser = async ( tracesDir: defaultBrowserPaths.tracesDir || undefined, }, contextOptions: { - viewport: null, + // Pinned rather than inherited from the window (``viewport: null``): + // every click the planner emits is an absolute coordinate read off a + // screenshot, so the join is only as reliable as the layout is + // reproducible. An inherited viewport is whatever Chromium negotiates + // with the window manager — a 1920x1080 desktop yielded a 937px-wide + // page, which is Meet's narrow layout and a different control map from + // the one the same prompts hit elsewhere. See MEET_VIEWPORT for the + // width choice. + viewport: { ...MEET_VIEWPORT }, ignoreHTTPSErrors: true, permissions: ['camera', 'microphone'], }, + // Anti-automation hardening (magnitude-core web/stealth.ts). Google flags + // an un-hardened automated Chromium and forces the persistent session + // into a reauth challenge, degrading the signed-in join to an anonymous + // guest that Meet turns away. Keep it on for the Meet browser. + stealth: true, }; if (storageStateName) { browser.storageStateName = storageStateName; @@ -1143,6 +1177,23 @@ async function googleMeetJoinFlow(agent: BrowserAgent): Promise false); + if (onAccountsDomain || reauthVisible) { + const reauthMsg = reauthVisible + ? await page.locator(reauthSelector).first().textContent().catch(() => 'reauth') + : 'accounts.google.com'; + console.warn(`[googlemeet/join] signed-out/reauth detected: "${reauthMsg}" (url=${pageUrl})`); + return { status: 'error', reason: `meet_signed_out: "${reauthMsg}" (url=${pageUrl})` }; + } + // Phase 1: pre-join prep (popups, camera off). The browser joins signed in // via persistent storage state, so there is no name field to fill. console.log('[googlemeet/join] Phase 1: prepare...'); @@ -1353,7 +1404,11 @@ app.post('/browser-states/:name/save', async (req: Request, res: Response) => { message: 'A valid browser state name and sessionId are required', }); } - const session = activeSessions.get(sessionId); + // Google Meet joins live in their own session store, and Meet is the only + // flow that runs a signed-in browser, so its sessions must be reachable here + // for cookie write-back after a join. Teams joins are anonymous and have no + // state to persist. + const session = activeSessions.get(sessionId) ?? googleMeetSessions.get(sessionId); if (!session) { return res.status(404).json({ error: 'session_not_found', message: 'Browser session does not exist' }); } diff --git a/sandboxes/conversation_manager/README.md b/sandboxes/conversation_manager/README.md index 26ce6ef6e..0ff785624 100644 --- a/sandboxes/conversation_manager/README.md +++ b/sandboxes/conversation_manager/README.md @@ -92,15 +92,15 @@ To keep interactive EventBus traffic sparse in Orchestra while still retaining a ```bash export EVENTBUS_PUBLISHING_ENABLED=true export EVENTBUS_ORCHESTRA_PERSIST_MODE=allowlist -export EVENTBUS_ORCHESTRA_PERSIST_TOOLS=execute_code,execute_function +export EVENTBUS_ORCHESTRA_PERSIST_TOOLS=act,execute_code,execute_function # optional Live Actions stream (unchanged by the allowlist): # export EVENTBUS_PUBSUB_STREAMING=true ``` In ``allowlist`` mode: -- **Outside** a task run: only allowlisted tools (default ``execute_code`` / - ``execute_function``) are written to ``Events/*``. +- **Outside** a task run: only allowlisted action/tool names (default ``act``, + ``execute_code``, and ``execute_function``) are written to ``Events/*``. - **Inside** an ``ActiveTask`` (``CURRENT_TASK_RUN_LINEAGE`` / payload ``run_key`` + ``task_id``/``instance_id``): the full ManagerMethod + ToolLoop tree is persisted and stamped for join from ``Tasks/Executions``. diff --git a/tests/conftest.py b/tests/conftest.py index 15ea780d5..5cbb0e4ad 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -636,6 +636,8 @@ def pytest_unconfigure(config): os.environ["HOME"] = _original_home if _hf_home_set_by_us: os.environ.pop("HF_HOME", None) + if _speaker_model_set_by_us: + os.environ.pop("UNIFY_SPEAKER_MODEL_PATH", None) def pytest_terminal_summary(terminalreporter, exitstatus, config): @@ -661,6 +663,7 @@ def pytest_terminal_summary(terminalreporter, exitstatus, config): _original_home: str | None = None _hf_home_set_by_us: bool = False +_speaker_model_set_by_us: bool = False def pytest_configure(config): @@ -703,6 +706,22 @@ def pytest_configure(config): os.environ["HF_HOME"] = original_hf _hf_home_set_by_us = True + # Same problem, same fix, for the speaker-embedding model: it is cached + # under ~/.cache/unify/speaker_id/ and the HOME override hides it, so + # every real-model speaker-identification test silently skips instead of + # running. Globbing (rather than importing speaker_id for the filename) + # keeps pytest_configure free of heavy imports. + global _speaker_model_set_by_us + if "UNIFY_SPEAKER_MODEL_PATH" not in os.environ and _original_home: + import glob as _glob + + cached_models = _glob.glob( + os.path.join(_original_home, ".cache", "unify", "speaker_id", "*.onnx"), + ) + if len(cached_models) == 1: + os.environ["UNIFY_SPEAKER_MODEL_PATH"] = cached_models[0] + _speaker_model_set_by_us = True + config.addinivalue_line( "markers", "requires_real_unify: mark test as requiring the real unify implementation", diff --git a/tests/conversation_manager/core/test_call_manager_rooms.py b/tests/conversation_manager/core/test_call_manager_rooms.py index faee2bff0..05437e630 100644 --- a/tests/conversation_manager/core/test_call_manager_rooms.py +++ b/tests/conversation_manager/core/test_call_manager_rooms.py @@ -166,3 +166,197 @@ async def test_recording_ready_prefers_call_session_then_room_then_conference( assert metadata["recording_url"].endswith("/call.mp3") assert metadata["recording_call_session_id"] == "CA111" assert metadata["recording_room_name"] == "unity_wa_room_123_CA111" + + +@pytest.mark.asyncio +async def test_recording_ready_recovers_exchange_from_stored_metadata(): + """A recycled container has no in-memory map, so fall back to the store. + + Egress finalises minutes after the room closes, by which time the pod that + ran the call is usually gone. Without this the file exists in GCS but is + never linked to its transcript. + """ + transcript_manager = MagicMock() + transcript_manager.resolve_exchange_id_by_metadata = MagicMock( + side_effect=lambda key, value: 77 if key == "provider_call_sid" else None, + ) + cm = MagicMock() + cm._recording_exchange_ids = {} + cm.transcript_manager = transcript_manager + cm._session_logger = MagicMock() + + await EventHandler.handle_event( + RecordingReady( + conference_name="legacy_conf", + recording_url="https://storage.googleapis.com/bucket/call.mp3", + call_session_id="", + provider_call_sid="CA111", + room_name="unity_wa_room_123_CA111", + ), + cm, + ) + + transcript_manager.update_exchange_metadata.assert_called_once() + exchange_id, metadata = transcript_manager.update_exchange_metadata.call_args.args + assert exchange_id == 77 + assert metadata["recording_url"].endswith("/call.mp3") + + +@pytest.mark.asyncio +async def test_recording_ready_gives_up_when_no_identifier_resolves(): + transcript_manager = MagicMock() + transcript_manager.resolve_exchange_id_by_metadata = MagicMock(return_value=None) + cm = MagicMock() + cm._recording_exchange_ids = {} + cm.transcript_manager = transcript_manager + cm._session_logger = MagicMock() + + await EventHandler.handle_event( + RecordingReady( + conference_name="unknown_conf", + recording_url="https://storage.googleapis.com/bucket/call.mp3", + ), + cm, + ) + + transcript_manager.update_exchange_metadata.assert_not_called() + + +# --------------------------------------------------------------------------- +# _start_session_recording -- recording is requested at call start +# --------------------------------------------------------------------------- + + +def _recording_cm(**call_manager_attrs) -> MagicMock: + cm = MagicMock() + cm._session_logger = MagicMock() + defaults = { + "room_name": "unity_42_phone", + "assistant_id": "42", + "user_id": "7", + "call_session_id": "", + "provider_call_sid": "CA111", + "conference_name": "Unity_conf_1", + "unify_meet_call_session_id": "", + } + defaults.update(call_manager_attrs) + for key, value in defaults.items(): + setattr(cm.call_manager, key, value) + return cm + + +@pytest.mark.asyncio +async def test_phone_call_started_requests_recording_with_linkage_ids(monkeypatch): + """The call-started path is where the room is first known to carry audio.""" + from unify.conversation_manager.domains import event_handlers + from unify.conversation_manager import utils as cm_utils + from unify.conversation_manager.events import PhoneCallStarted + + start = MagicMock(return_value=True) + monkeypatch.setattr(cm_utils, "start_call_recording", start) + + cm = _recording_cm() + await event_handlers._start_session_recording( + PhoneCallStarted(contact={"contact_id": 2}), + cm, + ) + + start.assert_called_once() + args, kwargs = start.call_args + assert args[0] == "unity_42_phone" + assert args[1] == "42" + assert kwargs["provider_call_sid"] == "CA111" + assert kwargs["conference_name"] == "Unity_conf_1" + + +@pytest.mark.asyncio +async def test_unify_meet_started_requests_recording_with_session_id(monkeypatch): + from unify.conversation_manager.domains import event_handlers + from unify.conversation_manager import utils as cm_utils + from unify.conversation_manager.events import UnifyMeetStarted + + start = MagicMock(return_value=True) + monkeypatch.setattr(cm_utils, "start_call_recording", start) + + cm = _recording_cm( + room_name="unity_call_CS_9", + unify_meet_call_session_id="CS_9", + provider_call_sid="", + conference_name="", + ) + await event_handlers._start_session_recording( + UnifyMeetStarted(contact={"contact_id": 1}, call_session_id="CS_9"), + cm, + ) + + start.assert_called_once() + args, kwargs = start.call_args + assert args[0] == "unity_call_CS_9" + assert kwargs["call_session_id"] == "CS_9" + # A meet has no telephony leg, so these stay empty rather than leaking + # phone-call state from a previous session on the same call manager. + assert kwargs["provider_call_sid"] == "" + assert kwargs["conference_name"] == "" + + +@pytest.mark.asyncio +@pytest.mark.parametrize("event_name", ["GoogleMeetStarted", "TeamsMeetStarted"]) +async def test_browser_meets_never_request_recording(monkeypatch, event_name): + """Browser-meet audio never reaches the LiveKit room, so egress cannot work. + + Requesting it anyway is what left compositors waiting on rooms for up to an + hour before aborting with no file. + """ + from unify.conversation_manager.domains import event_handlers + from unify.conversation_manager import utils as cm_utils + from unify.conversation_manager import events + + start = MagicMock(return_value=True) + monkeypatch.setattr(cm_utils, "start_call_recording", start) + + event_cls = getattr(events, event_name) + cm = _recording_cm(room_name="unity_42_gmeet") + await event_handlers._start_session_recording( + event_cls(contact={"contact_id": 2}), + cm, + ) + + start.assert_not_called() + + +@pytest.mark.asyncio +async def test_session_recording_skipped_without_a_room(monkeypatch): + from unify.conversation_manager.domains import event_handlers + from unify.conversation_manager import utils as cm_utils + from unify.conversation_manager.events import PhoneCallStarted + + start = MagicMock(return_value=True) + monkeypatch.setattr(cm_utils, "start_call_recording", start) + + cm = _recording_cm(room_name="") + await event_handlers._start_session_recording( + PhoneCallStarted(contact={"contact_id": 2}), + cm, + ) + + start.assert_not_called() + + +@pytest.mark.asyncio +async def test_session_recording_failure_never_escapes(monkeypatch): + """A recording problem must not disturb a live call.""" + from unify.conversation_manager.domains import event_handlers + from unify.conversation_manager import utils as cm_utils + from unify.conversation_manager.events import PhoneCallStarted + + monkeypatch.setattr( + cm_utils, + "start_call_recording", + MagicMock(side_effect=RuntimeError("comms down")), + ) + + cm = _recording_cm() + await event_handlers._start_session_recording( + PhoneCallStarted(contact={"contact_id": 2}), + cm, + ) diff --git a/tests/conversation_manager/core/test_event_handlers.py b/tests/conversation_manager/core/test_event_handlers.py index 5abb504fa..e0a8cebb4 100644 --- a/tests/conversation_manager/core/test_event_handlers.py +++ b/tests/conversation_manager/core/test_event_handlers.py @@ -2648,6 +2648,7 @@ async def test_task_due_start_executes_scheduler_with_scheduled_reason( source_task_log_id=555, revision="rev-1", task_name="Morning briefing", + task_description="Deliver the overnight briefing summary unprompted.", ) mock_cm.actor = MagicMock() captured: dict[str, object] = {} @@ -2688,6 +2689,10 @@ async def _noop(*args, **kwargs): assert captured["task_id"] == 101 assert captured["_activated_by"] == ActivatedBy.schedule assert captured["delegate"] is not None + assert ( + mock_cm.in_flight_actions[handle_id]["task_description"] + == "Deliver the overnight briefing summary unprompted." + ) @pytest.mark.asyncio @_handle_project @@ -2759,6 +2764,7 @@ async def _spy_act(*args, **kwargs): source_task_log_id=source_task_log_id, revision="rev-1", task_name="Scheduled integration report", + task_description="Prepare the scheduled report.", ) mock_cm.actor = actor @@ -2819,6 +2825,10 @@ async def _noop(*args, **kwargs): assert calls assert calls[0]["guidelines"] is not None assert calls[0]["persist"] is False + assert ( + mock_cm.in_flight_actions[handle_id]["task_description"] + == "Prepare the scheduled report." + ) rows = scheduler._filter_tasks(filter=f"task_id == {task_id}") assert len(rows) == 1 @@ -3185,6 +3195,145 @@ async def test_rest_task_trigger_starts_task_with_explicit_provenance( assert provenance.source_ref == "req-abc" mock_cm.request_llm_run.assert_not_called() + @pytest.mark.asyncio + async def test_rest_task_trigger_start_passes_task_description_through( + self, + mock_cm, + ): + """REST-triggered task registration must carry the authored task + description through to completed-actions rendering, so the brain + sees delivery intent (e.g. "deliver unprompted") next to the result. + """ + + from types import SimpleNamespace + + from unify.conversation_manager.domains.renderer import Renderer + from unify.conversation_manager.domains.task_execution import ( + _start_live_task_trigger_execution, + ) + + mock_cm.actor = MagicMock() + fake_task = SimpleNamespace( + description=( + "Deliver this summary unprompted to Yusha via task " + "completion delivery." + ), + ) + fake_scheduler = MagicMock() + fake_scheduler.execute = AsyncMock(return_value=MagicMock()) + fake_scheduler._get_task_or_raise = MagicMock(return_value=fake_task) + + event = TaskTriggerRequested( + task_id=301, + source_task_log_id=9001, + source_ref="req-abc", + task_label="Review report", + task_summary="Review the weekly report.", + ) + + async def _noop(*args, **kwargs): + return None + + with ( + patch( + "unify.conversation_manager.domains.task_execution.ManagerRegistry.get_task_scheduler", + return_value=fake_scheduler, + ), + patch( + "unify.conversation_manager.domains.task_execution.managers_utils.actor_watch_result", + new=_noop, + ), + patch( + "unify.conversation_manager.domains.task_execution.managers_utils.actor_watch_notifications", + new=_noop, + ), + patch( + "unify.conversation_manager.domains.task_execution.managers_utils.actor_watch_clarifications", + new=_noop, + ), + ): + handle_id = await _start_live_task_trigger_execution(event, mock_cm) + + fake_scheduler._get_task_or_raise.assert_called_once_with(301) + assert ( + mock_cm.in_flight_actions[handle_id]["task_description"] + == "Deliver this summary unprompted to Yusha via task completion delivery." + ) + + # Simulate the handle reaching completion and confirm the rendered + # block carries both tags the brain relies on. + completed_actions = { + handle_id: { + **mock_cm.in_flight_actions[handle_id], + "handle_actions": [ + { + "action_name": "act_completed", + "query": "Report reviewed.", + "success": True, + "result": "Report reviewed.", + }, + ], + }, + } + rendered = Renderer().render_completed_actions(completed_actions) + assert "" in rendered + assert ( + "Deliver this summary unprompted to Yusha via " + "task completion delivery." in rendered + ) + + @pytest.mark.asyncio + async def test_rest_task_trigger_start_tolerates_missing_task_lookup( + self, + mock_cm, + ): + """A failed description lookup must not block task-trigger startup.""" + + from unify.conversation_manager.domains.task_execution import ( + _start_live_task_trigger_execution, + ) + + mock_cm.actor = MagicMock() + fake_scheduler = MagicMock() + fake_scheduler.execute = AsyncMock(return_value=MagicMock()) + fake_scheduler._get_task_or_raise = MagicMock( + side_effect=ValueError("No task found with id=301"), + ) + + event = TaskTriggerRequested( + task_id=301, + source_task_log_id=9001, + source_ref="req-abc", + task_label="Review report", + task_summary="Review the weekly report.", + ) + + async def _noop(*args, **kwargs): + return None + + with ( + patch( + "unify.conversation_manager.domains.task_execution.ManagerRegistry.get_task_scheduler", + return_value=fake_scheduler, + ), + patch( + "unify.conversation_manager.domains.task_execution.managers_utils.actor_watch_result", + new=_noop, + ), + patch( + "unify.conversation_manager.domains.task_execution.managers_utils.actor_watch_notifications", + new=_noop, + ), + patch( + "unify.conversation_manager.domains.task_execution.managers_utils.actor_watch_clarifications", + new=_noop, + ), + ): + handle_id = await _start_live_task_trigger_execution(event, mock_cm) + + assert handle_id in mock_cm.in_flight_actions + assert "task_description" not in mock_cm.in_flight_actions[handle_id] + @pytest.mark.asyncio async def test_inbound_message_surfaces_trigger_candidates(self, mock_cm): """Inbound user messages should surface only live matching trigger tasks.""" @@ -3411,6 +3560,176 @@ async def test_inbound_call_queues_fast_brain_trigger_context(self, mock_cm): assert "Do not mention the task unless it naturally helps" in guidance.message +class TestProviderEventDispatchNotifications: + """Tests for the live provider-event dispatch call site. + + This is the exact path behind the incident where a live task completed + correctly but the reactive brain never saw the task's own authored + description (only a generic dispatch string) next to the result. + """ + + @pytest.mark.asyncio + async def test_provider_event_dispatch_registers_task_description( + self, + mock_cm, + ): + """A successful live start must carry outcome.description through to + the registered handle so completed-actions rendering surfaces it. + """ + + from datetime import datetime, timezone + + from unify.conversation_manager.domains.renderer import Renderer + from unify.conversation_manager.domains.task_execution import ( + _handle_provider_event_dispatch_requested_event, + ) + from unify.conversation_manager.events import ProviderEventDispatchRequested + from unify.task_scheduler.provider_event_dispatch import ( + LiveProviderEventDispatchOutcome, + ) + + mock_cm.actor = MagicMock() + fake_handle = MagicMock() + outcome = LiveProviderEventDispatchOutcome( + operation_id="op-1", + run_id=4242, + run_key="run-key-1", + captured_task_revision=3, + status="started", + fencing_token=7, + adopted_only=False, + description=( + "Deliver this summary unprompted to Yusha via task " + "completion delivery." + ), + ) + + event = ProviderEventDispatchRequested( + operation_id="op-1", + run_id=4242, + run_key="run-key-1", + assistant_id="assistant-123", + task_id=101, + binding_id="binding-1", + receipt_id="receipt-1", + accepted_revision="rev-123", + event_context_ref="blob://binding-1/receipt-1", + issued_at=datetime.now(timezone.utc).isoformat(), + ) + + async def _noop(*args, **kwargs): + return None + + with ( + patch( + "unify.task_scheduler.provider_event_execution.handle_provider_event_live_dispatch", + new=AsyncMock(return_value=(outcome, fake_handle)), + ), + patch( + "unify.conversation_manager.domains.task_execution.managers_utils.actor_watch_result", + new=_noop, + ), + patch( + "unify.conversation_manager.domains.task_execution.managers_utils.actor_watch_notifications", + new=_noop, + ), + patch( + "unify.conversation_manager.domains.task_execution.managers_utils.actor_watch_clarifications", + new=_noop, + ), + ): + should_request_llm = await _handle_provider_event_dispatch_requested_event( + event, + mock_cm, + ) + + assert should_request_llm is False + assert len(mock_cm.in_flight_actions) == 1 + handle_id = next(iter(mock_cm.in_flight_actions)) + assert ( + mock_cm.in_flight_actions[handle_id]["task_description"] + == "Deliver this summary unprompted to Yusha via task completion delivery." + ) + + # Simulate the handle reaching completion and confirm the rendered + # block carries both tags the brain relies on. + completed_actions = { + handle_id: { + **mock_cm.in_flight_actions[handle_id], + "handle_actions": [ + { + "action_name": "act_completed", + "query": "Provider event completed.", + "success": True, + "result": "Provider event completed.", + }, + ], + }, + } + rendered = Renderer().render_completed_actions(completed_actions) + assert "" in rendered + assert ( + "Deliver this summary unprompted to Yusha via " + "task completion delivery." in rendered + ) + + @pytest.mark.asyncio + async def test_provider_event_dispatch_adopted_only_skips_handle_registration( + self, + mock_cm, + ): + """Adopt-only / already-terminal claims return handle=None and must + never reach _register_live_task_handle, regardless of description. + """ + + from datetime import datetime, timezone + + from unify.conversation_manager.domains.task_execution import ( + _handle_provider_event_dispatch_requested_event, + ) + from unify.conversation_manager.events import ProviderEventDispatchRequested + from unify.task_scheduler.provider_event_dispatch import ( + LiveProviderEventDispatchOutcome, + ) + + mock_cm.actor = MagicMock() + outcome = LiveProviderEventDispatchOutcome( + operation_id="op-2", + run_id=4243, + run_key="run-key-2", + captured_task_revision=3, + status="adopted", + fencing_token=7, + adopted_only=True, + description="Some other task's authored description.", + ) + + event = ProviderEventDispatchRequested( + operation_id="op-2", + run_id=4243, + run_key="run-key-2", + assistant_id="assistant-123", + task_id=102, + binding_id="binding-2", + receipt_id="receipt-2", + accepted_revision="rev-124", + event_context_ref="blob://binding-2/receipt-2", + issued_at=datetime.now(timezone.utc).isoformat(), + ) + + with patch( + "unify.task_scheduler.provider_event_execution.handle_provider_event_live_dispatch", + new=AsyncMock(return_value=(outcome, None)), + ): + should_request_llm = await _handle_provider_event_dispatch_requested_event( + event, + mock_cm, + ) + + assert should_request_llm is False + assert mock_cm.in_flight_actions == {} + + # ============================================================================= # 11. SyncContacts Event Handler Tests # ============================================================================= diff --git a/tests/conversation_manager/core/test_managers_utils.py b/tests/conversation_manager/core/test_managers_utils.py index 5e6e6ae0a..4851bc8f3 100644 --- a/tests/conversation_manager/core/test_managers_utils.py +++ b/tests/conversation_manager/core/test_managers_utils.py @@ -724,3 +724,106 @@ async def test_log_message_slack_channel_without_ids_creates_fresh_exchange(): assert ( _exchange_id_of_last_message(cm) != first_exchange ), "Slack channel messages without ids must not group under a blank key" + + +# --------------------------------------------------------------------------- +# Call-utterance offsets +# --------------------------------------------------------------------------- + + +class TestCallUtteranceStamp: + """The ``MM.SS`` offset written onto each call utterance's metadata.""" + + def test_measures_from_the_utterance_not_the_logging_clock(self): + """Regression: the stamp used to read the clock at write time. + + ``log_message`` runs on the transcript worker, so a stamp read there + charges the utterance for however long its write queued behind exchange + creation and context provisioning. In staging that inflated a Unify + Meet's first utterance by 18s while later ones were accurate, stretching + early offsets past their position in the audio. + """ + from unify.conversation_manager.domains.managers_utils import ( + call_utterance_stamp, + ) + + call_start = datetime(2026, 7, 27, 12, 48, 23) + spoken_at = call_start + timedelta(seconds=5) + + # Even if the write lands 18s later, the stamp reflects when it was said. + assert call_utterance_stamp(call_start, spoken_at) == "00.05" + + def test_is_monotonic_in_speech_order(self): + """No TTS fudge, so ordering by stamp matches ordering by speech. + + A +2s allowance used to be added to assistant turns, which inverted + adjacent utterances: a reply could be stamped before the question. + """ + from unify.conversation_manager.domains.managers_utils import ( + call_utterance_stamp, + ) + + call_start = datetime(2026, 7, 27, 12, 48, 23) + stamps = [ + call_utterance_stamp(call_start, call_start + timedelta(seconds=offset)) + for offset in (5, 9, 15, 20) + ] + assert stamps == ["00.05", "00.09", "00.15", "00.20"] + assert stamps == sorted(stamps) + + def test_minutes_are_not_truncated_past_an_hour(self): + from unify.conversation_manager.domains.managers_utils import ( + call_utterance_stamp, + ) + + call_start = datetime(2026, 7, 27, 12, 0, 0) + assert ( + call_utterance_stamp(call_start, call_start + timedelta(seconds=4325)) + == "72.05" + ) + + def test_blank_outside_a_call(self): + from unify.conversation_manager.domains.managers_utils import ( + call_utterance_stamp, + ) + + assert call_utterance_stamp(None, datetime(2026, 7, 27)) == "" + + def test_clamps_an_utterance_stamped_before_the_call_start(self): + from unify.conversation_manager.domains.managers_utils import ( + call_utterance_stamp, + ) + + call_start = datetime(2026, 7, 27, 12, 48, 23) + assert ( + call_utterance_stamp(call_start, call_start - timedelta(seconds=9)) + == "00.00" + ) + + +class TestCallStartForMedium: + def test_each_voice_medium_reads_its_own_session_clock(self): + from unify.conversation_manager.domains.managers_utils import ( + call_start_for_medium, + ) + from unify.conversation_manager.cm_types import Medium + + call_manager = MagicMock() + call_manager.call_start_timestamp = "phone" + call_manager.unify_meet_start_timestamp = "meet" + call_manager.google_meet_start_timestamp = "gmeet" + call_manager.teams_meet_start_timestamp = "teams" + + assert call_start_for_medium(call_manager, Medium.PHONE_CALL) == "phone" + assert call_start_for_medium(call_manager, Medium.WHATSAPP_CALL) == "phone" + assert call_start_for_medium(call_manager, Medium.UNIFY_MEET) == "meet" + assert call_start_for_medium(call_manager, Medium.GOOGLE_MEET) == "gmeet" + assert call_start_for_medium(call_manager, Medium.TEAMS_MEET) == "teams" + + def test_text_mediums_have_no_session_clock(self): + from unify.conversation_manager.domains.managers_utils import ( + call_start_for_medium, + ) + from unify.conversation_manager.cm_types import Medium + + assert call_start_for_medium(MagicMock(), Medium.EMAIL) is None diff --git a/tests/conversation_manager/core/test_renderer.py b/tests/conversation_manager/core/test_renderer.py index 08caacb56..dace79be3 100644 --- a/tests/conversation_manager/core/test_renderer.py +++ b/tests/conversation_manager/core/test_renderer.py @@ -1097,6 +1097,47 @@ def test_single_completed_action(self, renderer): assert "pause_" not in result assert "resume_" not in result assert "interject_" not in result + # No task_description was set, so the tag must not appear at all. + assert "" not in result + + def test_completed_action_with_task_description_renders_both_tags( + self, + renderer, + ): + """A live task handle carrying task_description renders it alongside + original_request, so the brain sees the task's own authored + instructions (e.g. delivery intent) next to the result. + """ + completed_actions = { + 0: { + "handle": MagicMock(), + "query": "Provider event started task 101 (operation op-1).", + "action_type": "task", + "task_description": ( + "Deliver this summary unprompted to Yusha via task " + "completion delivery." + ), + "handle_actions": [ + { + "action_name": "act_completed", + "query": "Done.", + "success": True, + "result": "Done.", + }, + ], + }, + } + + result = renderer.render_completed_actions(completed_actions) + + assert ( + "Provider event started task 101 " + "(operation op-1)." in result + ) + assert ( + "Deliver this summary unprompted to Yusha via " + "task completion delivery." in result + ) def test_multiple_completed_actions(self, renderer): """Multiple completed actions render correctly.""" diff --git a/tests/conversation_manager/core/test_utils.py b/tests/conversation_manager/core/test_utils.py index 9df146d42..7463a1086 100644 --- a/tests/conversation_manager/core/test_utils.py +++ b/tests/conversation_manager/core/test_utils.py @@ -1254,34 +1254,73 @@ def test_numeric_suffix_does_not_confuse_parser(self): # ============================================================================= -# dispatch_livekit_agent code quality tests +# comms control-message code quality tests # ============================================================================= -class TestDispatchLivekitAgentCodeQuality: - """Regression tests for dispatch_livekit_agent implementation. +class TestCommsPostCodeQuality: + """Regression tests for the runtime's fire-and-forget calls into comms. These tests inspect the source code to prevent accidental regressions. """ def test_uses_requests_post_not_http_post(self): - """dispatch_livekit_agent must use requests.post directly, not http.post. + """The comms POST helper must use requests.post directly, not http.post. - The http module from unisdk.utils has retry logic baked in. For this - fire-and-forget dispatch, we intentionally want NO retries - the timeout - is expected and we should move on immediately. Using http.post would - cause multiple retry attempts with backoff delays, defeating the purpose. + The http module from unisdk.utils has retry logic baked in. For these + fire-and-forget control messages we intentionally want NO retries - the + timeout is expected and we should move on immediately. Using http.post + would retry with backoff, which for agent dispatch means dispatching + several agents into one room. """ import inspect - from unify.conversation_manager.utils import dispatch_livekit_agent + from unify.conversation_manager.utils import _post_to_comms - source = inspect.getsource(dispatch_livekit_agent) + source = inspect.getsource(_post_to_comms) - # Must use requests.post directly - check = "requests.post(" in source and "http.post(" not in source assert "requests.post(" in source, ( - "dispatch_livekit_agent must use requests.post() directly, " - "not http.post(). The http module has retry logic that would " - "dispatch multiple agents due to the expected timeout - " - "we want fire-and-forget behavior." + "_post_to_comms must use requests.post() directly, not http.post(). " + "The http module has retry logic that would dispatch multiple " + "agents due to the expected timeout - we want fire-and-forget." + ) + assert "http.post(" not in source + + def test_public_helpers_route_through_the_shared_post(self): + """Both comms calls must share one transport, so retry policy is one place.""" + import inspect + from unify.conversation_manager.utils import ( + dispatch_livekit_agent, + start_call_recording, ) + + for func in (dispatch_livekit_agent, start_call_recording): + source = inspect.getsource(func) + assert "_post_to_comms(" in source, ( + f"{func.__name__} must POST via _post_to_comms so it inherits " + "the no-retry, failure-swallowing transport." + ) + assert "requests.post(" not in source + + def test_dispatch_does_not_request_recording(self): + """Recording must not ride on agent dispatch. + + Starting egress at dispatch time races the participant join, and LiveKit + aborts such a job without writing a file. Recording is requested from the + call-started path instead. + """ + import inspect + from unify.conversation_manager.utils import dispatch_livekit_agent + + source = inspect.getsource(dispatch_livekit_agent) + # No record flag in the request payload, and no egress endpoint reached. + assert '"record"' not in source + assert "record=" not in source + assert "start-recording" not in source + assert "/phone/dispatch-livekit-agent" in source + + def test_start_call_recording_targets_the_recording_endpoint(self): + import inspect + from unify.conversation_manager.utils import start_call_recording + + source = inspect.getsource(start_call_recording) + assert "/phone/start-recording" in source diff --git a/tests/conversation_manager/voice/speaker_corpus.py b/tests/conversation_manager/voice/speaker_corpus.py new file mode 100644 index 000000000..61a4c0164 --- /dev/null +++ b/tests/conversation_manager/voice/speaker_corpus.py @@ -0,0 +1,206 @@ +"""Multi-speaker audio corpus for real-model speaker-identification tests. + +The stub embedder in ``test_speaker_id.py`` maps audio to orthogonal 2-d unit +vectors, so every similarity threshold trivially passes and segment duration +cannot matter. That harness proves the tracker's *wiring*; it cannot prove its +*tuning*. This module supplies the audio needed to measure the tuning against +the real CAM++ extractor. + +Two corpus sources, in priority order: + +1. ``$UNIFY_SPEAKER_TEST_CORPUS`` — a directory of ``{speaker}_{passage}.wav`` + files (16-bit PCM WAV). **Real human recordings belong here.** Threshold + calibration requires them; see the caveat below. +2. A synthetic corpus generated with macOS ``say`` and cached under + ``~/.cache/unify/speaker_id/test_corpus/``. Generated once, then reused. + +Caveat on the synthetic corpus: ``say`` voices come from one synthesiser and +share its artifacts, so different-speaker similarity is *inflated* relative to +real humans. That makes it sound for tests asserting a threshold is set too +low (a real corpus would only widen the gap) but unsound for calibrating where +a threshold should sit. Tests needing the latter gate on a real corpus. + +``say`` also silently substitutes the default voice for any voice that is not +installed, which yields byte-identical files for several requested names, so +the builder de-duplicates on content hash. +""" + +from __future__ import annotations + +import hashlib +import os +import shutil +import subprocess +import sys +from pathlib import Path + +import numpy as np + +from unify.conversation_manager.speaker_id import wav_bytes_to_pcm + +CORPUS_ENV = "UNIFY_SPEAKER_TEST_CORPUS" +SAMPLE_RATE = 16000 + +# Two passages so the same speaker can be scored against *different words*, +# which is the real-world same-speaker case (never the identical utterance). +PASSAGES = { + "a": ( + "I have seen things you people would not believe. Attack ships on fire " + "off the shoulder of Orion. I watched C beams glitter in the dark near " + "the Tannhauser Gate. All those moments will be lost in time, like " + "tears in rain." + ), + "b": ( + "The quick brown fox jumps over the lazy dog while the sun sets over " + "the quiet harbour and the boats return home for the evening tide. " + "Nothing else stirred along the whole length of the empty road." + ), +} + +# Requested generously; whatever this machine actually installs survives the +# de-duplication pass below. +_SAY_VOICES = ( + "Samantha", + "Daniel", + "Karen", + "Moira", + "Rishi", + "Alex", + "Fiona", + "Tessa", + "Veena", + "Nicky", +) + +_MIN_SPEAKERS = 4 + + +def _cache_dir() -> Path: + # ``UNITY_REAL_HOME`` is the pre-isolation home that tests/conftest.py + # records before pointing HOME at a temp dir. Preferring it keeps the + # generated corpus in one place instead of re-rendering it into a fresh + # temp home on every pytest session. + real_home = os.environ.get("UNITY_REAL_HOME") + if real_home: + root = Path(real_home) / ".cache" + else: + root = Path(os.environ.get("XDG_CACHE_HOME", Path.home() / ".cache")) + return root / "unify" / "speaker_id" / "test_corpus" + + +def _generate_with_say(dest: Path) -> None: + """Render every passage in every available ``say`` voice into ``dest``.""" + dest.mkdir(parents=True, exist_ok=True) + for voice in _SAY_VOICES: + for passage_key, text in PASSAGES.items(): + out = dest / f"{voice}_{passage_key}.wav" + if out.exists(): + continue + try: + subprocess.run( + [ + "say", + "-v", + voice, + "--data-format=LEI16@16000", + "-o", + str(out), + text, + ], + check=True, + capture_output=True, + timeout=60, + ) + except (subprocess.CalledProcessError, subprocess.TimeoutExpired): + # Voice unavailable on this machine; the others still suffice. + out.unlink(missing_ok=True) + + +def _read_pcm(path: Path) -> np.ndarray: + pcm, rate = wav_bytes_to_pcm(path.read_bytes()) + if rate != SAMPLE_RATE: + raise ValueError(f"{path.name}: expected {SAMPLE_RATE} Hz, got {rate}") + return pcm + + +def _source_dir() -> Path | None: + """The directory holding the corpus, generating it first if it can.""" + override = os.environ.get(CORPUS_ENV, "") + if override: + return Path(override) + cache = _cache_dir() + if sys.platform == "darwin" and shutil.which("say"): + _generate_with_say(cache) + return cache if cache.exists() else None + + +def _speaker_files() -> dict[str, dict[str, Path]]: + """``{speaker: {passage: path}}`` for complete, distinct voices. + + De-duplication hashes the file bytes rather than decoded audio so + availability can be checked without decoding the whole corpus. ``say`` + substitutes the default voice for any name that is not installed, which + yields byte-identical renders under several different speaker names. + """ + source = _source_dir() + if source is None: + return {} + by_speaker: dict[str, dict[str, Path]] = {} + for wav in sorted(source.glob("*.wav")): + if "_" not in wav.stem: + continue + speaker, passage = wav.stem.rsplit("_", 1) + by_speaker.setdefault(speaker, {})[passage] = wav + + complete = { + name: passages + for name, passages in by_speaker.items() + if set(passages) >= set(PASSAGES) + } + seen: set[str] = set() + unique: dict[str, dict[str, Path]] = {} + for name in sorted(complete): + digest = hashlib.sha256(complete[name]["a"].read_bytes()).hexdigest() + if digest in seen: + continue + seen.add(digest) + unique[name] = complete[name] + return unique + + +def load_corpus() -> dict[str, dict[str, np.ndarray]]: + """Return ``{speaker: {passage: int16 mono PCM @ 16 kHz}}``.""" + return { + name: {passage: _read_pcm(path) for passage, path in passages.items()} + for name, passages in _speaker_files().items() + } + + +def is_real_corpus() -> bool: + """Whether the corpus is human recordings rather than TTS renders. + + Only a real corpus can say where a threshold *should* sit; the synthetic + one can only say when a threshold is set too low. See the module docstring. + """ + return bool(os.environ.get(CORPUS_ENV, "")) + + +def unavailable_reason() -> str | None: + """``None`` when the corpus is usable, else a human-readable skip reason. + + Deliberately avoids decoding the audio: this runs at import time to drive + the module-level ``skipif``. + """ + corpus = _speaker_files() + if not corpus: + return ( + f"no speaker corpus: set ${CORPUS_ENV} to a directory of " + "{speaker}_{passage}.wav files, or run on macOS so it can be " + "generated with `say`" + ) + if len(corpus) < _MIN_SPEAKERS: + return ( + f"speaker corpus has {len(corpus)} distinct voice(s), " + f"need >= {_MIN_SPEAKERS}" + ) + return None diff --git a/tests/conversation_manager/voice/test_speaker_id.py b/tests/conversation_manager/voice/test_speaker_id.py index 424ecc648..8e81092d2 100644 --- a/tests/conversation_manager/voice/test_speaker_id.py +++ b/tests/conversation_manager/voice/test_speaker_id.py @@ -400,10 +400,74 @@ async def test_no_suggestion_when_contact_enrolled(self): async def test_short_segments_ignored(self): tracker = _make_tracker(enrolled={5: VOICE_A}) clock = _Clock() - # Below SEGMENT_MIN_S: no embedding scheduled. + # Below SEGMENT_MIN_S and never topped up: buffered, then dropped at + # finalize rather than embedded, so it contributes no cluster. _feed_segment(tracker, clock, "S0", amplitude=1000, seconds=0.2) await tracker.finalize() assert tracker.resolve("S0") is None + assert tracker.diagnostics()["segments_dropped"] == 1 + + async def test_short_segments_accumulate_until_they_can_be_embedded(self): + """Backchannels are buffered per id, not discarded. + + Several sub-threshold finals from one speaker are concatenated and + embedded once together, so the speaker is still attributed instead of + the turns vanishing. + """ + tracker = _make_tracker(enrolled={5: VOICE_A}) + clock = _Clock() + for _ in range(3): + _feed_segment(tracker, clock, "S0", amplitude=1000, seconds=0.9) + await tracker.finalize() + + resolution = tracker.resolve("S0") + assert resolution is not None + assert resolution.contact_id == 5 + stats = tracker.diagnostics() + assert stats["segments_observed"] == 3 + assert stats["segments_buffered"] == 2 # first two held, third flushed + assert stats["segments_embedded"] == 1 # one embedding for all three + assert stats["clusters"] == 1 + + async def test_buffered_audio_is_prepended_to_the_next_full_segment(self): + """A short turn followed by a long one yields a single merged segment. + + The buffered audio must not be stranded: it belongs to the same voice, + so it is carried into the next embedding rather than dropped. + """ + tracker = _make_tracker(enrolled={5: VOICE_A}) + clock = _Clock() + _feed_segment(tracker, clock, "S0", amplitude=1000, seconds=0.9) + _feed_segment(tracker, clock, "S0", amplitude=1000, seconds=3.0) + await tracker.finalize() + + stats = tracker.diagnostics() + assert stats["segments_buffered"] == 1 + assert stats["segments_embedded"] == 1 + assert stats["segments_dropped"] == 0 + assert tracker._speakers["S0"].pending_duration_s == 0.0 + # Both turns' audio is behind the one cluster. + cluster = tracker._speakers["S0"].clusters[0] + assert cluster.accumulator.total_duration_s == pytest.approx(3.9, abs=0.05) + + async def test_short_segments_buffer_per_diarization_id(self): + """One speaker's backchannels never top up another speaker's buffer.""" + tracker = _make_tracker(enrolled={5: VOICE_A}) + clock = _Clock() + _feed_segment(tracker, clock, "S0", amplitude=1000, seconds=1.2) + _feed_segment(tracker, clock, "S1", amplitude=9000, seconds=1.2) + await tracker.await_pending() + + # Neither id has reached SEGMENT_MIN_S on its own. + assert tracker.diagnostics()["segments_embedded"] == 0 + assert tracker._speakers["S0"].pending_duration_s == pytest.approx( + 1.2, + abs=0.05, + ) + assert tracker._speakers["S1"].pending_duration_s == pytest.approx( + 1.2, + abs=0.05, + ) async def test_co_located_voices_split_into_clusters(self): # A single diarization id (S0) that actually carries two physically @@ -429,13 +493,15 @@ async def test_co_located_voices_split_into_clusters(self): assert second.provisional is True # The enrolled voice returns: attribution swings back to its pinned - # cluster (verified), though the id stays provisional (multi-voice). + # cluster, so the contact is still named for routing — but the id now + # carries two voices, so this utterance cannot be certified as theirs + # and `verified` is withheld. _feed_segment(tracker, clock, "S0", amplitude=1000, seconds=3.0) await tracker.await_pending() third = tracker.resolve("S0") assert third.contact_id == 5 - assert third.verified is True assert third.provisional is True + assert third.verified is False async def test_co_located_second_voice_blocks_enrollment_and_suggests(self): # Two voices under one diarization id count as two speakers: the @@ -567,6 +633,50 @@ def test_non_permanent_contact_can_be_disengaged(self): assert not engaged.is_engaged_contact(7) +# ───────────────────────────────────────────────────────────────────────────── +# Mid-call profile refresh (late joiners) +# ───────────────────────────────────────────────────────────────────────────── + + +@pytest.mark.asyncio +class TestMidCallProfileRefresh: + async def test_late_profile_pins_a_voice_already_heard(self): + """Someone who joined and spoke before their profile arrived is pinned. + + Cluster centroids are re-scored on every segment, so the late joiner is + picked up on their next utterance without replaying the call. + """ + tracker = _make_tracker(enrolled={}) + clock = _Clock() + _feed_segment(tracker, clock, "S0", amplitude=9000, seconds=3.0) + await tracker.await_pending() + assert tracker.resolve("S0") is None # nobody enrolled yet + + assert tracker.add_enrolled_profiles({9: VOICE_B}) == 1 + + _feed_segment(tracker, clock, "S0", amplitude=9000, seconds=3.0) + await tracker.await_pending() + resolution = tracker.resolve("S0") + assert resolution is not None + assert resolution.contact_id == 9 + + async def test_refresh_does_not_disturb_an_existing_profile(self): + """A repeated roster push must not overwrite pins already in effect.""" + tracker = _make_tracker(enrolled={5: VOICE_A}) + assert tracker.add_enrolled_profiles({5: VOICE_B}) == 0 + + clock = _Clock() + _feed_segment(tracker, clock, "S0", amplitude=1000, seconds=3.0) + await tracker.await_pending() + assert tracker.resolve("S0").contact_id == 5 + + async def test_refresh_ignores_malformed_entries(self): + tracker = _make_tracker(enrolled={}) + assert tracker.add_enrolled_profiles({"not-an-id": VOICE_A}) == 0 + assert tracker.add_enrolled_profiles({}) == 0 + assert tracker.add_enrolled_profiles({7: VOICE_A}) == 1 + + # ───────────────────────────────────────────────────────────────────────────── # Profiles partition (scorer input) # ───────────────────────────────────────────────────────────────────────────── @@ -752,7 +862,7 @@ def _vad_event(ev_type, *, speaking=False, speech_duration=0.0): ) -def _make_gate(scorer): +def _make_gate(scorer, *, feed_scorer: bool = True): from livekit.agents import vad as agents_vad from unify.conversation_manager.engaged_vad import EngagedGateVAD @@ -778,11 +888,25 @@ def stream(self) -> ScriptedVADStream: return self.last_stream inner = ScriptedVAD() - gate = EngagedGateVAD(inner=inner, scorer=scorer) + gate = EngagedGateVAD(inner=inner, scorer=scorer, feed_scorer=feed_scorer) stream = gate.stream() return inner, gate, stream +def _push_audio_frame(stream) -> None: + """Push one frame into the gate's input, as the AgentSession would.""" + from livekit import rtc + + stream.push_frame( + rtc.AudioFrame( + data=_tone(1000, 0.02).tobytes(), + sample_rate=SR, + num_channels=1, + samples_per_channel=int(0.02 * SR), + ), + ) + + async def _next_event(stream, timeout: float = 1.0): return await asyncio.wait_for(stream.__anext__(), timeout=timeout) @@ -798,6 +922,35 @@ async def _expect_no_event(stream, wait: float = 0.15) -> None: pass +@pytest.mark.asyncio +class TestEngagedGateVADScorerFeed: + """The scorer must have exactly one audio source. + + It keeps a single rolling window, so feeding it from both the LiveKit VAD + stream and the browser-meet PortAudio bridge would interleave unrelated + audio into the same window. Browser meets feed it from the bridge and + construct the gate with ``feed_scorer=False``. + """ + + async def test_feeds_the_scorer_by_default(self): + scorer = _FakeScorer() + _inner, _gate, stream = _make_gate(scorer) + await asyncio.sleep(0) + _push_audio_frame(stream) + await asyncio.sleep(0.05) + assert scorer.audio_calls == 1 + await stream.aclose() + + async def test_does_not_feed_the_scorer_when_disabled(self): + scorer = _FakeScorer() + _inner, _gate, stream = _make_gate(scorer, feed_scorer=False) + await asyncio.sleep(0) + _push_audio_frame(stream) + await asyncio.sleep(0.05) + assert scorer.audio_calls == 0 + await stream.aclose() + + @pytest.mark.asyncio class TestEngagedGateVAD: async def test_forwards_speech_events_when_not_confident(self): diff --git a/tests/conversation_manager/voice/test_speaker_id_real_model.py b/tests/conversation_manager/voice/test_speaker_id_real_model.py new file mode 100644 index 000000000..a126a0e30 --- /dev/null +++ b/tests/conversation_manager/voice/test_speaker_id_real_model.py @@ -0,0 +1,641 @@ +""" +tests/conversation_manager/voice/test_speaker_id_real_model.py +============================================================== + +Tuning tests for speaker identification, run against the real CAM++ extractor. + +``test_speaker_id.py`` drives a stub embedder that returns orthogonal 2-d unit +vectors keyed on mean amplitude. That proves the tracker's *wiring* — ring +buffer, clustering, pinning, enrollment gating — but it cannot prove its +*tuning*: under the stub every similarity threshold trivially separates the +two "voices", and embeddings do not depend on segment length at all. This +module covers what the stub cannot. + +Two classes of test, with very different corpus requirements: + +*Self-comparison* — how much audio the model needs before an embedding of a +speaker resembles another embedding of that same speaker (``SEGMENT_MIN_S``, +``REALTIME_WINDOW_S``). Synthesiser artifacts are constant within one voice, +so these are sound on any corpus, including the generated one. + +*Cross-speaker* — whether each threshold separates different people +(``SPEAKER_MATCH_THRESHOLD``, ``CLUSTER_JOIN_SIM``, ``CROSS_ID_MERGE_SIM``) +and whether the tracker keeps two speakers apart. These are only meaningful on +a corpus the model can actually separate, so they sit behind the measured +``separable_corpus`` gate rather than an assumption. Screening the macOS +``say`` voices found 16 of 21 ranking some *other* voice above themselves — +CAM++ largely measures the synthesiser there, not the speaker — so on a +generated corpus these skip. Supply human recordings via +``$UNIFY_SPEAKER_TEST_CORPUS`` to run them. + +The remaining ``xfail(strict=True)`` markers are all cross-speaker, covering +defects that cannot be confirmed or fixed without a separable corpus. They +record the gap rather than hiding it; strict means a fix turns them into +failures until the marker is removed, which is the intended signal. The +duration defects they used to sit alongside are fixed, so those tests now +assert the corrected behaviour directly. + +Marked ``slow``: real inference, tens of embeddings. +""" + +from __future__ import annotations + +import logging +from dataclasses import dataclass + +import numpy as np +import pytest + +from unify.conversation_manager import speaker_id +from unify.conversation_manager.speaker_id import ( + CLUSTER_JOIN_SIM, + CROSS_ID_MERGE_SIM, + REALTIME_MATCH_THRESHOLD, + REALTIME_WINDOW_S, + SEGMENT_MIN_S, + SPEAKER_MATCH_THRESHOLD, + CentroidAccumulator, + SpeakerEmbedder, + SpeakerTracker, + cosine_similarity, +) + +from .speaker_corpus import ( + CORPUS_ENV, + SAMPLE_RATE, + is_real_corpus, + load_corpus, + unavailable_reason, +) + +_log = logging.getLogger(__name__) + +_MODEL_PATH = speaker_id.ensure_speaker_model(download=False) + +# Only look for (and, on macOS, generate) the corpus once the model is known to +# be present — otherwise collection pays for ~20 `say` renders it cannot use. +_CORPUS_REASON = unavailable_reason() if _MODEL_PATH is not None else None + +pytestmark = [ + pytest.mark.slow, + pytest.mark.skipif( + _MODEL_PATH is None, + reason="speaker embedding model not cached locally", + ), + pytest.mark.skipif(_CORPUS_REASON is not None, reason=_CORPUS_REASON or ""), +] + +# A "profile" is one whole passage, standing in for an enrollment; segments are +# sliced out of the *other* passage, so a speaker is never scored against their +# own identical audio. Scoring against different words is the realistic case +# and is markedly harder than re-scoring the same recording. +_PROFILE_PASSAGE = "a" +_SEGMENT_PASSAGE = "b" + +# A segment at the accepted floor must beat a half-length one by at least this +# much for the duration effect to count as real rather than noise. +_MONOTONIC_MARGIN = 0.15 + +# At ``SEGMENT_MIN_S`` most slices of a speaker must match that speaker's own +# profile; at half the floor almost none may. These bracket the regime change +# the floor is placed at — measured across every slice position of every corpus +# voice: 0.8s -> 0% clearing threshold, 1.0s -> 0%, 1.5s -> 12%, 2.0s -> 65%, +# 4.0s -> 83%. Deliberately loose: individual slices vary a lot with which +# words they happen to contain, so only the distribution is meaningful. +_USABLE_FRACTION_AT_FLOOR = 0.5 +_UNUSABLE_FRACTION_BELOW_FLOOR = 0.15 + + +# ───────────────────────────────────────────────────────────────────────────── +# Fixtures +# ───────────────────────────────────────────────────────────────────────────── + + +@pytest.fixture(scope="module") +def embedder() -> SpeakerEmbedder: + return SpeakerEmbedder(_MODEL_PATH) + + +@pytest.fixture(scope="module") +def corpus() -> dict[str, dict[str, np.ndarray]]: + return load_corpus() + + +@pytest.fixture(scope="module") +def profiles(embedder, corpus) -> dict[str, np.ndarray]: + """Full-passage embedding per speaker — the stand-in for an enrollment.""" + return { + name: embedder.embed_sync(passages[_PROFILE_PASSAGE], SAMPLE_RATE) + for name, passages in corpus.items() + } + + +@dataclass(frozen=True) +class _CorpusQuality: + """How well the model separates this corpus's voices. + + ``rank1`` is the fraction of speakers whose own profile is the closest + match to their other passage — the property cross-speaker tests depend on. + ``gap`` is the margin between the same-speaker floor and the + different-speaker ceiling; a non-positive gap means the two distributions + overlap and no threshold can separate them. + """ + + rank1: float + same_min: float + same_mean: float + diff_max: float + diff_mean: float + + @property + def gap(self) -> float: + return self.same_min - self.diff_max + + @property + def is_separable(self) -> bool: + return self.rank1 == 1.0 and self.gap > 0.0 + + +@pytest.fixture(scope="module") +def quality(embedder, corpus, profiles) -> _CorpusQuality: + names = sorted(corpus) + probes = { + name: embedder.embed_sync(corpus[name][_SEGMENT_PASSAGE], SAMPLE_RATE) + for name in names + } + correct = sum( + max(names, key=lambda m: cosine_similarity(probes[n], profiles[m])) == n + for n in names + ) + same = [cosine_similarity(probes[n], profiles[n]) for n in names] + diff = [ + cosine_similarity(probes[a], profiles[b]) + for i, a in enumerate(names) + for b in names[i + 1 :] + ] + result = _CorpusQuality( + rank1=correct / len(names), + same_min=min(same), + same_mean=float(np.mean(same)), + diff_max=max(diff), + diff_mean=float(np.mean(diff)), + ) + _log.info( + "corpus=%s voices=%d rank-1=%.0f%% | same-speaker min=%.3f mean=%.3f " + "| different-speaker max=%.3f mean=%.3f | gap=%+.3f", + "real" if is_real_corpus() else "generated", + len(names), + result.rank1 * 100, + result.same_min, + result.same_mean, + result.diff_max, + result.diff_mean, + result.gap, + ) + return result + + +@pytest.fixture(scope="module") +def separable_corpus(corpus, quality) -> dict[str, dict[str, np.ndarray]]: + """The corpus, but only for tests that need speakers told apart.""" + if not quality.is_separable: + pytest.skip( + f"corpus cannot separate speakers (rank-1 {quality.rank1:.0%}, " + f"gap {quality.gap:+.3f}): cross-speaker thresholds cannot be " + f"measured against it. Point ${CORPUS_ENV} at human recordings.", + ) + return corpus + + +# ───────────────────────────────────────────────────────────────────────────── +# Helpers +# ───────────────────────────────────────────────────────────────────────────── + + +def _slice(pcm: np.ndarray, seconds: float, index: int = 0) -> np.ndarray: + """A ``seconds``-long slice taken ``index`` slices into the utterance. + + Offset from the start so slices land on voiced speech rather than the + leading silence every render begins with. + """ + n = int(seconds * SAMPLE_RATE) + start = int(1.0 * SAMPLE_RATE) + index * n + if start + n > len(pcm): + raise ValueError("corpus utterance too short for this slice") + return pcm[start : start + n] + + +class _CallSim: + """Feeds a SpeakerTracker on a synthetic timeline, as the live flow does. + + Mirrors ``_feed_segment`` in ``test_speaker_id.py``: audio is appended to + the ring with an explicit ``end_ts`` and the matching final transcript is + registered, so no wall clock is involved. + """ + + def __init__(self, tracker: SpeakerTracker) -> None: + self._tracker = tracker + self._now = 1_000.0 + self._offsets: dict[str, int] = {} + + async def utterance( + self, + diarization_id: str, + pcm: np.ndarray, + seconds: float, + *, + gap_s: float = 0.4, + ) -> None: + """Play ``seconds`` of ``pcm``, advancing through it on repeat calls.""" + n = int(seconds * SAMPLE_RATE) + offset = self._offsets.get(diarization_id, 0) + if offset + n > len(pcm): + offset = 0 + chunk = pcm[offset : offset + n] + self._offsets[diarization_id] = offset + n + + self._now += seconds + self._tracker._ring.append(chunk, SAMPLE_RATE, end_ts=self._now) + self._tracker.observe_final_transcript(diarization_id, end_ts=self._now) + await self._tracker.await_pending() + self._now += gap_s + + +def _make_tracker( + embedder: SpeakerEmbedder, + *, + enrolled: dict[int, np.ndarray] | None = None, + contact_id: int | None = 42, + on_captured=None, + on_suggested=None, +) -> SpeakerTracker: + return SpeakerTracker( + embedder=embedder, + enrolled_profiles=enrolled or {}, + call_contact_id=contact_id, + on_enrollment_captured=on_captured, + on_enrollment_suggested=on_suggested, + ) + + +# ───────────────────────────────────────────────────────────────────────────── +# Sanity and corpus reporting +# ───────────────────────────────────────────────────────────────────────────── + + +def test_corpus_embeddings_are_unit_norm_and_deterministic(embedder, corpus): + name = sorted(corpus)[0] + pcm = corpus[name][_PROFILE_PASSAGE] + first = embedder.embed_sync(pcm, SAMPLE_RATE) + again = embedder.embed_sync(pcm, SAMPLE_RATE) + + assert first.ndim == 1 and first.size > 0 + assert float(np.linalg.norm(first)) == pytest.approx(1.0, abs=1e-3) + assert cosine_similarity(first, again) > 0.999 + + +def test_corpus_quality_is_measured_not_assumed(quality): + """Records the corpus's separability; the cross-speaker gate reads it. + + A generated corpus is expected to fail ``is_separable`` — the assertion + here is only that the measurement is well-formed, so the report always + lands in the log for whoever is calibrating. + """ + assert 0.0 <= quality.rank1 <= 1.0 + assert quality.diff_max < 0.999, "corpus voices are duplicates" + + +# ───────────────────────────────────────────────────────────────────────────── +# Segment duration: what the model needs before an embedding means anything +# +# Self-comparison only — sound on any corpus. +# ───────────────────────────────────────────────────────────────────────────── + + +def _match_fraction(embedder, corpus, profiles, seconds: float) -> float: + """Fraction of all ``seconds``-long slices that match their own speaker. + + Averaged over every slice position of every voice: a single slice's score + swings wildly with which words it happens to contain, so only the + distribution says anything about duration. + """ + scores: list[float] = [] + for name, passages in corpus.items(): + pcm = passages[_SEGMENT_PASSAGE] + width = int(seconds * SAMPLE_RATE) + for start in range(SAMPLE_RATE, len(pcm) - width, width): + scores.append( + cosine_similarity( + embedder.embed_sync(pcm[start : start + width], SAMPLE_RATE), + profiles[name], + ), + ) + assert scores, f"corpus too short to slice at {seconds}s" + return sum(s >= SPEAKER_MATCH_THRESHOLD for s in scores) / len(scores) + + +def test_segment_floor_sits_above_the_unusable_regime(embedder, corpus, profiles): + """``SEGMENT_MIN_S`` must be on the usable side of the duration cliff. + + CAM++ pools frame statistics, so below roughly two seconds an embedding is + not a noisy version of the speaker — it is unrelated to them. This pins the + floor to the regime change rather than to a hand-picked number. + """ + at_floor = _match_fraction(embedder, corpus, profiles, SEGMENT_MIN_S) + below = _match_fraction(embedder, corpus, profiles, SEGMENT_MIN_S / 2) + + assert at_floor >= _USABLE_FRACTION_AT_FLOOR, ( + f"only {at_floor:.0%} of {SEGMENT_MIN_S}s slices match their own " + f"speaker; SEGMENT_MIN_S is too low" + ) + assert below <= _UNUSABLE_FRACTION_BELOW_FLOOR, ( + f"{below:.0%} of {SEGMENT_MIN_S / 2}s slices already match — the " + f"cliff has moved, so SEGMENT_MIN_S may be higher than it needs to be" + ) + + +def test_longer_segments_resemble_their_speaker_more_than_short_ones( + embedder, + corpus, + profiles, +): + """Per-speaker form of the same effect: the floor beats half the floor. + + Holds within each voice, so unlike the cross-speaker tests it does not + depend on the corpus separating different people. + """ + for name, passages in corpus.items(): + pcm = passages[_SEGMENT_PASSAGE] + short = cosine_similarity( + embedder.embed_sync(_slice(pcm, SEGMENT_MIN_S / 2), SAMPLE_RATE), + profiles[name], + ) + longer = cosine_similarity( + embedder.embed_sync(_slice(pcm, SEGMENT_MIN_S), SAMPLE_RATE), + profiles[name], + ) + assert longer - short >= _MONOTONIC_MARGIN, ( + f"{name}: {SEGMENT_MIN_S / 2}s scored {short:.3f}, " + f"{SEGMENT_MIN_S}s scored {longer:.3f} — expected at least " + f"{_MONOTONIC_MARGIN} better" + ) + + +def test_centroid_of_shortest_segments_converges_on_its_speaker( + embedder, + corpus, + profiles, +): + name = sorted(corpus)[0] + pcm = corpus[name][_SEGMENT_PASSAGE] + accumulator = CentroidAccumulator() + count = int((len(pcm) / SAMPLE_RATE - 1.0) // SEGMENT_MIN_S) + for index in range(count): + segment = _slice(pcm, SEGMENT_MIN_S, index) + accumulator.add(embedder.embed_sync(segment, SAMPLE_RATE), SEGMENT_MIN_S) + + score = cosine_similarity(accumulator.centroid, profiles[name]) + assert score >= SPEAKER_MATCH_THRESHOLD, ( + f"{name}: centroid of {count} x {SEGMENT_MIN_S}s segments scored " + f"{score:.3f} against its own profile" + ) + + +def test_realtime_window_can_reach_the_match_threshold(embedder, corpus, profiles): + """A realtime window must be able to match its speaker, or gating is inert. + + The scorer only ever gates on a *confident* verdict, so if no window of a + speaker's own audio can clear the threshold the verdict is permanently + "unknown" and ``EngagedGateVAD`` silently does nothing. That is the failure + the window length guards against, and it is invisible in production because + the gate fails open. + """ + matched = { + name + for name, passages in corpus.items() + if any( + cosine_similarity( + embedder.embed_sync( + _slice(passages[_SEGMENT_PASSAGE], REALTIME_WINDOW_S, i), + SAMPLE_RATE, + ), + profiles[name], + ) + >= REALTIME_MATCH_THRESHOLD + for i in range( + int( + (len(passages[_SEGMENT_PASSAGE]) / SAMPLE_RATE - 1.0) + // REALTIME_WINDOW_S, + ), + ) + ) + } + assert len(matched) >= len(corpus) / 2, ( + f"only {len(matched)}/{len(corpus)} speakers had any " + f"{REALTIME_WINDOW_S}s window reach {REALTIME_MATCH_THRESHOLD} — " + f"the floor gate cannot act" + ) + + +# ───────────────────────────────────────────────────────────────────────────── +# Whole-tracker behaviour, single speaker +# ───────────────────────────────────────────────────────────────────────────── + + +@pytest.mark.asyncio +async def test_single_speaker_long_utterances_is_one_pinned_voice( + embedder, + corpus, + profiles, +): + """The happy path: one enrolled speaker, ordinary sentence-length turns.""" + name = sorted(corpus)[0] + tracker = _make_tracker(embedder, enrolled={42: profiles[name]}) + sim = _CallSim(tracker) + pcm = corpus[name][_SEGMENT_PASSAGE] + + resolutions = [] + for _ in range(4): + await sim.utterance("S0", pcm, 2.5) + resolutions.append(tracker.resolve("S0")) + await tracker.finalize() + + assert len(tracker._speakers["S0"].clusters) == 1 + assert tracker._distinct_voice_count() == 1 + assert all(r is not None and r.contact_id == 42 for r in resolutions) + assert not any(r.provisional for r in resolutions) + + +@pytest.mark.asyncio +async def test_single_speaker_with_backchannels_stays_one_voice( + embedder, + corpus, + profiles, +): + """A real caller mixes sentences with short acknowledgements. + + Before short finals were buffered, each "yeah" embedded as noise, missed + ``CLUSTER_JOIN_SIM`` against the speaker's own cluster, and seeded a + phantom second one — so the caller's own short turns came back labelled + "Speaker 2" and ``provisional`` latched on for the rest of the call. + """ + name = sorted(corpus)[0] + tracker = _make_tracker(embedder, enrolled={42: profiles[name]}) + sim = _CallSim(tracker) + pcm = corpus[name][_SEGMENT_PASSAGE] + + resolutions = [] + for seconds in (2.5, 0.9, 2.5, 1.0, 2.5): + await sim.utterance("S0", pcm, seconds) + resolutions.append(tracker.resolve("S0")) + await tracker.finalize() + + assert len(tracker._speakers["S0"].clusters) == 1, ( + "one speaker split into " + f"{len(tracker._speakers['S0'].clusters)} voice clusters" + ) + assert all( + r is not None and r.contact_id == 42 for r in resolutions + ), f"attributions: {[(r.contact_id, r.label) for r in resolutions]}" + + +# ───────────────────────────────────────────────────────────────────────────── +# Cross-speaker: threshold placement and two-speaker calls +# +# Gated on `separable_corpus` — meaningless unless the model can tell this +# corpus's voices apart in the first place. +# ───────────────────────────────────────────────────────────────────────────── + + +@pytest.mark.parametrize( + "name,value", + [ + ("SPEAKER_MATCH_THRESHOLD", SPEAKER_MATCH_THRESHOLD), + ("CLUSTER_JOIN_SIM", CLUSTER_JOIN_SIM), + ("CROSS_ID_MERGE_SIM", CROSS_ID_MERGE_SIM), + ], +) +def test_threshold_sits_between_the_distributions( + separable_corpus, + quality, + name, + value, +): + """Every threshold must fall in the gap that separates the distributions.""" + assert quality.diff_max < value <= quality.same_min, ( + f"{name}={value} is outside the separating gap " + f"({quality.diff_max:.3f}, {quality.same_min:.3f}]" + ) + + +@pytest.mark.xfail( + strict=True, + reason=( + "F1: CROSS_ID_MERGE_SIM=0.3 is far below any plausible " + "different-speaker score, so _distinct_voice_count() collapses " + "genuinely different people into one voice. That is the gate " + "protecting auto-enrollment. Phase 1 raises it into the measured gap." + ), +) +def test_cross_id_merge_sim_does_not_merge_different_speakers( + separable_corpus, + quality, +): + assert CROSS_ID_MERGE_SIM > quality.diff_max, ( + f"CROSS_ID_MERGE_SIM={CROSS_ID_MERGE_SIM} merges different speakers " + f"scoring up to {quality.diff_max:.3f}" + ) + + +@pytest.mark.asyncio +@pytest.mark.xfail( + strict=True, + reason=( + "F1: CROSS_ID_MERGE_SIM=0.3 collapses two genuinely different " + "speakers on separate diarization ids into a single voice, so the " + "count that gates auto-enrollment reads 1 instead of 2." + ), +) +async def test_two_speakers_count_as_two_distinct_voices( + embedder, + separable_corpus, + profiles, +): + first, second = sorted(separable_corpus)[:2] + tracker = _make_tracker(embedder, enrolled={42: profiles[first]}) + sim = _CallSim(tracker) + + for _ in range(3): + await sim.utterance("S0", separable_corpus[first][_SEGMENT_PASSAGE], 2.5) + await sim.utterance("S1", separable_corpus[second][_SEGMENT_PASSAGE], 2.5) + await tracker.finalize() + + assert tracker._distinct_voice_count() == 2 + + +@pytest.mark.asyncio +@pytest.mark.xfail( + strict=True, + reason=( + "F1: with the voice count collapsed to 1, a two-person call passes " + "the single-voice gate and auto-enrolls a voiceprint blended from " + "both speakers. The stranger then matches the contact's stored " + "profile above SPEAKER_MATCH_THRESHOLD on every future call, and " + "auto-enrollment is write-once so it never self-corrects." + ), +) +async def test_two_speakers_do_not_contaminate_auto_enrollment( + embedder, + separable_corpus, + profiles, +): + """An unenrolled contact must not be enrolled from a shared-room call.""" + first, second = sorted(separable_corpus)[:2] + captured: dict = {} + tracker = _make_tracker( + embedder, + enrolled={}, # contact not yet enrolled: auto-enrollment is armed + on_captured=lambda emb, wav, dur: captured.update(embedding=emb), + ) + sim = _CallSim(tracker) + + for _ in range(9): + await sim.utterance("S0", separable_corpus[first][_SEGMENT_PASSAGE], 2.5) + await sim.utterance("S1", separable_corpus[second][_SEGMENT_PASSAGE], 2.5) + await tracker.finalize() + + if captured: + stranger = cosine_similarity(captured["embedding"], profiles[second]) + assert ( + stranger < SPEAKER_MATCH_THRESHOLD + ), f"stored voiceprint matches the other speaker at {stranger:.3f}" + + +@pytest.mark.asyncio +@pytest.mark.xfail( + strict=True, + reason=( + "F2: the manual-enrollment nudge reads the same collapsed voice " + "count as the enrollment gate (it needs >=2, enrollment needs ==1), " + "so it never fires and the Console fallback recorder is unreachable." + ), +) +async def test_multiple_voices_trigger_the_enrollment_suggestion( + embedder, + separable_corpus, +): + first, second = sorted(separable_corpus)[:2] + suggested: dict = {} + tracker = _make_tracker( + embedder, + enrolled={}, + on_suggested=lambda n: suggested.update(count=n), + ) + sim = _CallSim(tracker) + + for _ in range(3): + await sim.utterance("S0", separable_corpus[first][_SEGMENT_PASSAGE], 2.5) + await sim.utterance("S1", separable_corpus[second][_SEGMENT_PASSAGE], 2.5) + await tracker.finalize() + + assert suggested.get("count", 0) >= 2 diff --git a/tests/event_bus/test_persist_filters.py b/tests/event_bus/test_persist_filters.py index 465b477ef..a07778928 100644 --- a/tests/event_bus/test_persist_filters.py +++ b/tests/event_bus/test_persist_filters.py @@ -10,14 +10,26 @@ def test_parse_persist_tools_default_when_empty(): - assert parse_persist_tools("") == frozenset({"execute_code", "execute_function"}) - assert parse_persist_tools(None) == frozenset({"execute_code", "execute_function"}) + expected = frozenset({"act", "execute_code", "execute_function"}) + assert parse_persist_tools("") == expected + assert parse_persist_tools(None) == expected def test_parse_persist_tools_custom(): - assert parse_persist_tools("execute_code, other_tool ") == frozenset( + custom_tools = parse_persist_tools("execute_code, other_tool ") + assert custom_tools == frozenset( {"execute_code", "other_tool"}, ) + assert "act" not in custom_tools + + +def test_custom_allowlist_can_exclude_codeact_root(): + assert not should_persist_to_orchestra( + "ManagerMethod", + {"method": "act", "manager": "CodeActActor"}, + mode="allowlist", + tools=parse_persist_tools("execute_code,execute_function"), + ) def test_mode_all_persists_everything(): @@ -35,29 +47,29 @@ def test_mode_all_persists_everything(): ) -def test_allowlist_manager_method_execute_tools_only(): - tools = frozenset({"execute_code", "execute_function"}) +def test_allowlist_manager_method_selected_methods_only(): + tools = frozenset({"act", "execute_code", "execute_function"}) assert should_persist_to_orchestra( "ManagerMethod", - {"method": "execute_code", "manager": "CodeActActor"}, + {"method": "act", "manager": "CodeActActor"}, mode="allowlist", tools=tools, ) assert should_persist_to_orchestra( "ManagerMethod", - {"method": "execute_function", "manager": "CodeActActor"}, + {"method": "execute_code", "manager": "CodeActActor"}, mode="allowlist", tools=tools, ) - assert not should_persist_to_orchestra( + assert should_persist_to_orchestra( "ManagerMethod", - {"method": "ask", "manager": "ContactManager"}, + {"method": "execute_function", "manager": "CodeActActor"}, mode="allowlist", tools=tools, ) assert not should_persist_to_orchestra( "ManagerMethod", - {"method": "act", "manager": "CodeActActor"}, + {"method": "ask", "manager": "ContactManager"}, mode="allowlist", tools=tools, ) diff --git a/tests/gateway/channels/phone/test_views.py b/tests/gateway/channels/phone/test_views.py index fde353638..6747d2686 100644 --- a/tests/gateway/channels/phone/test_views.py +++ b/tests/gateway/channels/phone/test_views.py @@ -11,6 +11,7 @@ from __future__ import annotations +import json from types import SimpleNamespace from unittest.mock import AsyncMock, MagicMock, patch @@ -50,6 +51,7 @@ def _settings(monkeypatch: pytest.MonkeyPatch) -> None: ORCHESTRA_ADMIN_KEY=SimpleNamespace( get_secret_value=lambda: "test-admin-key", ), + DEPLOY_ENV="staging", ) monkeypatch.setattr(phone_views, "SETTINGS", stub) @@ -105,8 +107,10 @@ def test_auth_router_exposes_expected_paths() -> None: ("/end-conference", ["POST"]), ("/hang-up", ["POST"]), ("/hang-up-call", ["POST"]), + ("/recording-url", ["POST"]), ("/send-call", ["POST"]), ("/send-text", ["POST"]), + ("/start-recording", ["POST"]), ] @@ -496,16 +500,12 @@ def test_dispatch_livekit_agent_falls_back_to_legacy_livekit_agent_name( assert args[0] == "legacy_agent_name" -def test_dispatch_livekit_agent_forwards_record_and_linkage( +def test_dispatch_livekit_agent_passes_distinct_agent_name( client: TestClient, _phone_credentials: None, _settings: None, ) -> None: - """record + assistant/user/session linkage must reach the egress helper. - - Regression guard: the gateway port previously dropped ``record`` so no - recording was ever started for meet / dispatched calls. - """ + """Org meet rooms share one room but register per-assistant worker names.""" mock_create = AsyncMock() with patch( "unify.gateway.channels.phone.views.create_room_and_dispatch_agent", @@ -514,23 +514,215 @@ def test_dispatch_livekit_agent_forwards_record_and_linkage( resp = client.post( "/phone/dispatch-livekit-agent", json={ - "room_name": "unity_42_meet", + "room_name": "unity_call_CS_1", "livekit_agent_name": "unity_42", - "record": True, + }, + ) + + assert resp.status_code == 200 + args, _ = mock_create.await_args + assert args[0] == "unity_call_CS_1" # room_name + assert args[1] == "unity_42" # distinct agent worker name + + +# --------------------------------------------------------------------------- +# POST /phone/start-recording +# --------------------------------------------------------------------------- + + +def test_start_recording_forwards_room_and_linkage_ids( + client: TestClient, + _phone_credentials: None, + _settings: None, +) -> None: + """Linkage IDs must reach the egress helper. + + They ride the completion webhook and are the only way the finished file is + matched back to its transcript exchange. + """ + mock_egress = AsyncMock() + with patch( + "unify.gateway.channels.phone.views.start_room_egress", + new=mock_egress, + ): + resp = client.post( + "/phone/start-recording", + json={ + "room_name": "unity_call_CS_1", "assistant_id": "42", "user_id": "7", "call_session_id": "CS_1", + "provider_call_sid": "CA_abc", + "conference_name": "Unity_conf_1", }, ) assert resp.status_code == 200 - args, kwargs = mock_create.await_args - assert args[0] == "unity_42_meet" # room_name - assert args[1] == "unity_42" # distinct agent worker name - assert kwargs["record"] is True - assert kwargs["assistant_id"] == "42" - assert kwargs["user_id"] == "7" + args, kwargs = mock_egress.await_args + assert args[0] == "unity_call_CS_1" + assert args[1] == "42" assert kwargs["call_session_id"] == "CS_1" + assert kwargs["provider_call_sid"] == "CA_abc" + assert kwargs["conference_name"] == "Unity_conf_1" + + +def test_start_recording_rejects_missing_assistant_id( + client: TestClient, + _phone_credentials: None, + _settings: None, +) -> None: + """Without an assistant id the recording has no resolvable object prefix.""" + mock_egress = AsyncMock() + with patch( + "unify.gateway.channels.phone.views.start_room_egress", + new=mock_egress, + ): + resp = client.post( + "/phone/start-recording", + json={"room_name": "unity_call_CS_1"}, + ) + + assert resp.status_code == 400 + mock_egress.assert_not_awaited() + + +def test_start_recording_rejects_missing_room_name( + client: TestClient, + _phone_credentials: None, + _settings: None, +) -> None: + mock_egress = AsyncMock() + with patch( + "unify.gateway.channels.phone.views.start_room_egress", + new=mock_egress, + ): + resp = client.post( + "/phone/start-recording", + json={"assistant_id": "42"}, + ) + + assert resp.status_code == 400 + mock_egress.assert_not_awaited() + + +# --------------------------------------------------------------------------- +# POST /phone/recording-url +# --------------------------------------------------------------------------- + +_RECORDING_URI = ( + "gs://unity-call-recordings/staging/42/unity_call_abc_2026-07-27T12-48-25.mp3" +) + + +@pytest.fixture +def _recording_storage(monkeypatch: pytest.MonkeyPatch): + """Stub the GCS client so signing is exercised without real credentials.""" + from unify.gateway.channels.phone import views as phone_views + + monkeypatch.setenv( + "GCP_SA_KEY", + json.dumps({"type": "service_account", "project_id": "test"}), + ) + blob = MagicMock() + blob.exists.return_value = True + blob.generate_signed_url.return_value = "https://signed.example.com/audio.mp3" + client = MagicMock() + client.bucket.return_value.blob.return_value = blob + monkeypatch.setattr( + phone_views.Credentials, + "from_service_account_info", + classmethod(lambda cls, info: MagicMock()), + ) + monkeypatch.setattr(phone_views.storage, "Client", lambda credentials: client) + return blob + + +def test_recording_url_signs_an_existing_recording( + client: TestClient, + _phone_credentials: None, + _settings: None, + _recording_storage: MagicMock, +) -> None: + with patch( + "unify.gateway.channels.phone.views.require_assistant_ownership", + new=AsyncMock(), + ) as owns: + resp = client.post("/phone/recording-url", json={"gcs_uri": _RECORDING_URI}) + + assert resp.status_code == 200 + assert resp.json()["signed_url"] == "https://signed.example.com/audio.mp3" + assert resp.json()["expires_in_minutes"] == 60 + # Authorisation is keyed on the assistant segment of the object path. + assert owns.await_args.args[1] == "42" + assert _recording_storage.generate_signed_url.call_args.kwargs["method"] == "GET" + + +def test_recording_url_404s_when_the_object_was_never_written( + client: TestClient, + _phone_credentials: None, + _settings: None, + _recording_storage: MagicMock, +) -> None: + """An egress that failed before the completion gate still left a URL behind. + + 404 is the signal the reader turns into "Recording unavailable", so it must + not be collapsed into a generic error. + """ + _recording_storage.exists.return_value = False + + with patch( + "unify.gateway.channels.phone.views.require_assistant_ownership", + new=AsyncMock(), + ): + resp = client.post("/phone/recording-url", json={"gcs_uri": _RECORDING_URI}) + + assert resp.status_code == 404 + _recording_storage.generate_signed_url.assert_not_called() + + +@pytest.mark.parametrize( + ("gcs_uri", "expected_status"), + [ + # Not a gs:// URI at all. + ("https://storage.googleapis.com/unity-call-recordings/staging/42/a.mp3", 400), + # A different bucket: this endpoint must not become a general signer for + # the comms service account, which can read far more than recordings. + ("gs://assistant-message-attachments-staging/staging/42/a.mp3", 403), + # Another environment's recordings. + ("gs://unity-call-recordings/production/42/a.mp3", 403), + # No assistant segment — the legacy malformed shape, unattributable. + ("gs://unity-call-recordings/staging/unity__phone_2026.mp3", 400), + # Not an audio object. + ("gs://unity-call-recordings/staging/42/EG_abc.json", 400), + ], +) +def test_recording_url_refuses_objects_outside_this_envs_recordings( + client: TestClient, + _phone_credentials: None, + _settings: None, + _recording_storage: MagicMock, + gcs_uri: str, + expected_status: int, +) -> None: + with patch( + "unify.gateway.channels.phone.views.require_assistant_ownership", + new=AsyncMock(), + ) as owns: + resp = client.post("/phone/recording-url", json={"gcs_uri": gcs_uri}) + + assert resp.status_code == expected_status + # Rejected before any authorisation or signing work happens. + owns.assert_not_awaited() + _recording_storage.generate_signed_url.assert_not_called() + + +def test_recording_url_requires_a_uri( + client: TestClient, + _phone_credentials: None, + _settings: None, +) -> None: + resp = client.post("/phone/recording-url", json={}) + assert resp.status_code == 400 # --------------------------------------------------------------------------- diff --git a/tests/gateway/common/test_livekit.py b/tests/gateway/common/test_livekit.py index 8f1012527..400c35ddb 100644 --- a/tests/gateway/common/test_livekit.py +++ b/tests/gateway/common/test_livekit.py @@ -8,6 +8,7 @@ from __future__ import annotations +import re from unittest.mock import AsyncMock, MagicMock, patch import pytest @@ -294,15 +295,16 @@ async def test_create_room_and_dispatch_agent_passes_room_and_agent_through( @pytest.mark.asyncio -async def test_create_room_and_dispatch_agent_starts_egress_when_record( +async def test_create_room_and_dispatch_agent_never_starts_egress( _livekit_credentials: EnvCredentialStore, - monkeypatch: pytest.MonkeyPatch, ) -> None: - """record=True must start an audio-only egress with the linkage webhook.""" - from unify.gateway.common.livekit import create_room_and_dispatch_agent + """Dispatch must not record: egress at dispatch time races the join. - monkeypatch.setenv("GCP_SA_KEY", "sa-json") - monkeypatch.setenv("LIVEKIT_EGRESS_GCS_BUCKET", "unity-call-recordings") + Binding recording to dispatch is what produced aborted, file-less egress + jobs -- the compositor starts before any participant publishes. Recording is + started separately from the call-started path. + """ + from unify.gateway.common.livekit import create_room_and_dispatch_agent api = _fake_livekit_api() api.agent_dispatch = MagicMock() @@ -310,57 +312,184 @@ async def test_create_room_and_dispatch_agent_starts_egress_when_record( return_value=MagicMock(id="DISPATCH_123"), ) api.egress = MagicMock() + api.egress.start_room_composite_egress = AsyncMock() + + with patch( + "unify.gateway.common.livekit.get_livekit_api", + return_value=api, + ): + await create_room_and_dispatch_agent( + "unity_42_phone", + "unity_42_phone", + _livekit_credentials, + ) + + api.egress.start_room_composite_egress.assert_not_awaited() + + +# --------------------------------------------------------------------------- +# start_room_egress -- object layout, linkage webhook, and refusal rules +# --------------------------------------------------------------------------- + + +def _egress_api() -> MagicMock: + api = _fake_livekit_api() + api.egress = MagicMock() api.egress.start_room_composite_egress = AsyncMock( return_value=MagicMock(egress_id="EG_1"), ) + api.egress.list_egress = AsyncMock(return_value=MagicMock(items=[])) + return api + +@pytest.fixture +def _egress_env(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("GCP_SA_KEY", "sa-json") + monkeypatch.setenv("LIVEKIT_EGRESS_GCS_BUCKET", "unity-call-recordings") + + +@pytest.mark.asyncio +async def test_start_room_egress_requests_audio_only_mp3_with_linkage_webhook( + _livekit_credentials: EnvCredentialStore, + _egress_env: None, +) -> None: + from unify.gateway.common.livekit import start_room_egress + + api = _egress_api() with patch( "unify.gateway.common.livekit.get_livekit_api", return_value=api, ): - await create_room_and_dispatch_agent( - "unity_42_phone", + await start_room_egress( "unity_42_phone", + "42", _livekit_credentials, - record=True, - assistant_id="42", - user_id="7", + "7", provider_call_sid="CA_abc", + conference_name="Unity_conf_1", ) - api.agent_dispatch.create_dispatch.assert_awaited_once() api.egress.start_room_composite_egress.assert_awaited_once() request = api.egress.start_room_composite_egress.await_args.args[0] assert request.room_name == "unity_42_phone" assert request.audio_only is True assert request.file_outputs[0].gcp.bucket == "unity-call-recordings" - assert "provider_call_sid=CA_abc" in request.webhooks[0].url - assert "/livekit/recording-complete" in request.webhooks[0].url + # {env}/{assistant_id}/{room}_{timestamp}.mp3 -- the assistant prefix is + # what makes a stored recording resolvable back to its owner. + assert re.match( + r"^[a-z]+/42/unity_42_phone_\d{4}-\d{2}-\d{2}T\d{2}-\d{2}-\d{2}\.mp3$", + request.file_outputs[0].filepath, + ), request.file_outputs[0].filepath + webhook_url = request.webhooks[0].url + assert "/livekit/recording-complete" in webhook_url + assert "provider_call_sid=CA_abc" in webhook_url + assert "conference_name=Unity_conf_1" in webhook_url + assert "assistant_id=42" in webhook_url + api.aclose.assert_awaited_once() @pytest.mark.asyncio -async def test_create_room_and_dispatch_agent_no_egress_without_record( +@pytest.mark.parametrize("room_name", ["unity_42_gmeet", "unity_42_teams"]) +async def test_start_room_egress_skips_browser_meet_rooms( _livekit_credentials: EnvCredentialStore, + _egress_env: None, + room_name: str, ) -> None: - """record defaults False, so no egress is started for a bare dispatch.""" - from unify.gateway.common.livekit import create_room_and_dispatch_agent + """Browser-meet audio is bridged outside LiveKit, so egress can never work. - api = _fake_livekit_api() - api.agent_dispatch = MagicMock() - api.agent_dispatch.create_dispatch = AsyncMock( - return_value=MagicMock(id="DISPATCH_123"), + Requesting it anyway leaves a compositor waiting for a track that never + arrives until the room closes, then aborts with no file. + """ + from unify.gateway.common.livekit import start_room_egress + + api = _egress_api() + with patch( + "unify.gateway.common.livekit.get_livekit_api", + return_value=api, + ): + await start_room_egress(room_name, "42", _livekit_credentials) + + api.egress.start_room_composite_egress.assert_not_awaited() + + +@pytest.mark.asyncio +async def test_start_room_egress_refuses_without_assistant_id( + _livekit_credentials: EnvCredentialStore, + _egress_env: None, +) -> None: + """An empty assistant id collapses the object prefix, stranding the file.""" + from unify.gateway.common.livekit import start_room_egress + + api = _egress_api() + with patch( + "unify.gateway.common.livekit.get_livekit_api", + return_value=api, + ): + await start_room_egress("unity_42_phone", " ", _livekit_credentials) + + api.egress.start_room_composite_egress.assert_not_awaited() + + +@pytest.mark.asyncio +async def test_start_room_egress_skips_room_already_being_recorded( + _livekit_credentials: EnvCredentialStore, + _egress_env: None, +) -> None: + """Two starters on one room would record it twice into two files.""" + from unify.gateway.common.livekit import start_room_egress + + api = _egress_api() + api.egress.list_egress = AsyncMock( + return_value=MagicMock(items=[MagicMock(egress_id="EG_existing")]), ) - api.egress = MagicMock() - api.egress.start_room_composite_egress = AsyncMock() with patch( "unify.gateway.common.livekit.get_livekit_api", return_value=api, ): - await create_room_and_dispatch_agent( - "unity_42_phone", - "unity_42_phone", - _livekit_credentials, - ) + await start_room_egress("unity_42_phone", "42", _livekit_credentials) + api.egress.list_egress.assert_awaited_once() api.egress.start_room_composite_egress.assert_not_awaited() + + +@pytest.mark.asyncio +async def test_start_room_egress_starts_when_active_egress_lookup_fails( + _livekit_credentials: EnvCredentialStore, + _egress_env: None, +) -> None: + """A LiveKit hiccup on the dedupe check must not silently disable recording.""" + from unify.gateway.common.livekit import start_room_egress + + api = _egress_api() + api.egress.list_egress = AsyncMock(side_effect=RuntimeError("livekit down")) + + with patch( + "unify.gateway.common.livekit.get_livekit_api", + return_value=api, + ): + await start_room_egress("unity_42_phone", "42", _livekit_credentials) + + api.egress.start_room_composite_egress.assert_awaited_once() + + +@pytest.mark.asyncio +async def test_start_room_egress_swallows_livekit_errors( + _livekit_credentials: EnvCredentialStore, + _egress_env: None, +) -> None: + """A recording failure must never propagate into call setup.""" + from unify.gateway.common.livekit import start_room_egress + + api = _egress_api() + api.egress.start_room_composite_egress = AsyncMock( + side_effect=RuntimeError("egress rejected"), + ) + + with patch( + "unify.gateway.common.livekit.get_livekit_api", + return_value=api, + ): + await start_room_egress("unity_42_phone", "42", _livekit_credentials) + + api.aclose.assert_awaited_once() diff --git a/tests/task_scheduler/test_active_task_run_key.py b/tests/task_scheduler/test_active_task_run_key.py index d5e3947db..ae6769e73 100644 --- a/tests/task_scheduler/test_active_task_run_key.py +++ b/tests/task_scheduler/test_active_task_run_key.py @@ -2,7 +2,13 @@ from __future__ import annotations -from unify.task_scheduler.active_task import _resolve_active_task_run_key +import asyncio + +import pytest + +from unify.common._async_tool.loop_config import TOOL_LOOP_LINEAGE +from unify.events.task_run_lineage import CURRENT_TASK_RUN_LINEAGE +from unify.task_scheduler.active_task import ActiveTask, _resolve_active_task_run_key from unify.task_scheduler.machine_state import ( TaskRunProvenance, TaskRunReference, @@ -59,3 +65,49 @@ def test_resolve_run_key_none_without_inputs(): ) is None ) + + +class _CompletedHandle: + async def result(self) -> str: + return "task summary" + + +class _LineageCapturingActor: + def __init__(self) -> None: + self.run_lineage = None + self.tool_loop_lineage = None + + async def act(self, *args, **kwargs) -> _CompletedHandle: + self.run_lineage = CURRENT_TASK_RUN_LINEAGE.get() + self.tool_loop_lineage = TOOL_LOOP_LINEAGE.get() + return _CompletedHandle() + + +@pytest.mark.asyncio +async def test_active_task_scopes_lineage_before_watcher_result(): + """Actor startup inherits run lineage without retaining reset tokens on its handle.""" + + actor = _LineageCapturingActor() + task = await ActiveTask.create( + actor, + task_description="Summarize the inbox.", + task_id=42, + instance_id=0, + task_run_reference=TaskRunReference( + assistant_id="1", + run_key="live:scheduled:1:42:revision:once", + ), + ) + + assert actor.run_lineage is not None + assert actor.run_lineage.task_id == 42 + assert any("Task.run(task_id=42" in segment for segment in actor.tool_loop_lineage) + assert CURRENT_TASK_RUN_LINEAGE.get() is None + assert not TOOL_LOOP_LINEAGE.get() + + # This regression only exercises the handoff to the live watcher. Terminal + # persistence is covered independently and would require Orchestra here. + task._task_run_reference = None + result = await asyncio.create_task(task.result()) + + assert result == "task summary" diff --git a/tests/task_scheduler/test_creation_deletion.py b/tests/task_scheduler/test_creation_deletion.py index cc52a93f1..11a5ed876 100644 --- a/tests/task_scheduler/test_creation_deletion.py +++ b/tests/task_scheduler/test_creation_deletion.py @@ -1,6 +1,8 @@ from datetime import datetime, timedelta, timezone +from types import SimpleNamespace from tests.helpers import _handle_project +from tests.task_scheduler.test_task_revision_cas import _provider_event_task import pytest import unisdk @@ -8,6 +10,7 @@ from unify.common.tool_outcome import ToolErrorException from unify.session_details import SESSION_DETAILS from unify.task_scheduler.task_scheduler import TaskScheduler +from unify.task_scheduler import typed_tasks_client from unify.task_scheduler.types.priority import Priority from unify.task_scheduler.types.repetition import Frequency, RepeatPattern from unify.task_scheduler.types.schedule import Schedule @@ -63,6 +66,46 @@ def test_delete_task(): assert task_list == [] +def test_delete_task_provider_event_skips_redundant_log_delete( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """The provider-event branch of ``_delete_task`` must delete the task's + row exactly once, via the typed API's revision-CAS delete, and must not + attempt a second, redundant ``self._store.delete(logs=...)`` over the + same row (that path 404s under the collapsed task-identity data model). + + Built via ``__new__`` (bypassing ``__init__``) so this exercises the + real ``_delete_task`` control flow without provisioning a live backend. + """ + scheduler = TaskScheduler.__new__(TaskScheduler) + scheduler._num_tasks_cached = None + task = _provider_event_task(task_revision=5) + + store_delete_calls: list[list[int]] = [] + scheduler._store = SimpleNamespace( + get_rows=lambda *, filter, return_ids_only: [999], + delete=lambda *, logs: store_delete_calls.append(logs), + ) + monkeypatch.setattr(scheduler, "_ensure_not_active_task", lambda task_ids: None) + monkeypatch.setattr(scheduler, "_resolve_task_for_mutation", lambda task_id: task) + + typed_delete_calls: list[tuple[int, int]] = [] + monkeypatch.setattr( + typed_tasks_client, + "delete_task", + lambda *, task_id, expected_task_revision: typed_delete_calls.append( + (task_id, expected_task_revision), + ), + ) + + result = scheduler._delete_task(task_id=task.task_id, _root_applied=True) + + assert typed_delete_calls == [(task.task_id, 5)] + assert store_delete_calls == [] + assert result["outcome"] == "task deleted" + assert result["details"]["task_id"] == task.task_id + + @_handle_project def test_create_task_with_response_policy(): ts = TaskScheduler() diff --git a/tests/task_scheduler/test_prompt_builders.py b/tests/task_scheduler/test_prompt_builders.py index 93dadd267..27d3cfafc 100644 --- a/tests/task_scheduler/test_prompt_builders.py +++ b/tests/task_scheduler/test_prompt_builders.py @@ -134,7 +134,10 @@ def test_build_update_prompt_includes_provider_event_guidance() -> None: "resolve required resources → create with trigger_config filled → enable" in prompt ) - assert "not `live_ready`" in prompt + assert "only when the catalog row explicitly has `live_ready=false`" in prompt + assert "`null` means there is no native lifecycle gate" in prompt + assert "not that the trigger is unavailable" in prompt + assert "provisioning, and health checks" in prompt assert "delivery_only=true" in prompt assert "list_provider_trigger_resources" in prompt assert "never watch all of My Drive" in prompt @@ -159,4 +162,9 @@ def test_build_ask_prompt_includes_provider_event_discovery_guidance() -> None: assert "do not claim the provider lacks that trigger globally" in prompt assert "copy a selectable item's `trigger_config`" in prompt assert "do not invent provider ids" in prompt - assert "live_ready=false" in prompt + assert ( + "Only explicit live_ready=false, provisionable=false, or delivery_only=true" + in prompt + ) + assert "null means the catalog has no native lifecycle gate" in prompt + assert "provisioning, and health checks" in prompt diff --git a/tests/task_scheduler/test_provider_event_captured_instance.py b/tests/task_scheduler/test_provider_event_captured_instance.py index 0f5e3bfd9..82f52aa71 100644 --- a/tests/task_scheduler/test_provider_event_captured_instance.py +++ b/tests/task_scheduler/test_provider_event_captured_instance.py @@ -97,6 +97,7 @@ async def test_provider_event_start_leaves_definition_unarmed(): assert updates["captured_task_revision"] == 5 assert updates["revision"] == "rev-accepted-1" assert updates["state"] == "running" + assert datetime.fromisoformat(updates["started_at"]) await handle.stop(reason="test cleanup") diff --git a/tests/task_scheduler/test_provider_event_live_completion.py b/tests/task_scheduler/test_provider_event_live_completion.py index 9b2340162..5f05ec5ad 100644 --- a/tests/task_scheduler/test_provider_event_live_completion.py +++ b/tests/task_scheduler/test_provider_event_live_completion.py @@ -56,3 +56,41 @@ async def test_registered_provider_event_handle_preserves_definition_status(): definition_row = rows[0] assert definition_row.instance_id == 0 assert definition_row.status == Status.triggerable + + +@pytest.mark.asyncio +@_handle_project +async def test_provider_event_completion_terminalizes_run_without_definition_write(): + """Provider-event completion updates its run row while preserving its definition.""" + + actor = SimulatedActor(steps=0) + scheduler = TaskScheduler(actor=actor) + task_id = _seed_provider_event_definition(scheduler, task_revision=3) + run_updates: list[dict] = [] + + def _record_run_update(_reference, update): + run_updates.append(dict(update)) + + with ( + patch( + "unify.task_scheduler.task_scheduler.update_task_run_record", + side_effect=_record_run_update, + ), + patch( + "unify.task_scheduler.active_task.update_task_run_record", + side_effect=_record_run_update, + ), + ): + handle = await scheduler.start_provider_event_instance( + request=_request(task_id=task_id, operation_id="op-terminal-1"), + captured_task_revision=3, + provider_event_context={ + "kind": "provider_event_context", + "trust": "untrusted_data", + }, + ) + await asyncio.wait_for(handle.result(), timeout=5.0) + + assert any(update["state"] == "completed" for update in run_updates) + definition_row = scheduler._filter_tasks(filter=f"task_id == {task_id}")[0] + assert definition_row.status == Status.triggerable diff --git a/tests/task_scheduler/test_provider_trigger_catalog_pagination.py b/tests/task_scheduler/test_provider_trigger_catalog_pagination.py new file mode 100644 index 000000000..fbf122b58 --- /dev/null +++ b/tests/task_scheduler/test_provider_trigger_catalog_pagination.py @@ -0,0 +1,117 @@ +"""Pure unit tests for provider-trigger catalog filter/pagination forwarding.""" + +from __future__ import annotations + +from typing import Any + +import pytest + +from unify.session_details import SESSION_DETAILS +from unify.task_scheduler import typed_tasks_client +from unify.task_scheduler.task_scheduler import TaskScheduler + + +def test_get_trigger_catalog_forwards_optional_params_as_query_params( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setattr(SESSION_DETAILS.assistant, "agent_id", 42) + captured: dict[str, Any] = {} + + def fake_request(method: str, path: str, **kwargs: Any) -> Any: + captured["method"] = method + captured["path"] = path + captured["params"] = kwargs.get("params") + return {"info": {"available": True, "triggers": []}} + + monkeypatch.setattr(typed_tasks_client, "_request", fake_request) + monkeypatch.setattr(typed_tasks_client, "_info", lambda response: response["info"]) + + result = typed_tasks_client.get_trigger_catalog( + canonical_app_slug="google_calendar", + limit=10, + offset=20, + ) + + assert captured["method"] == "get" + assert captured["path"] == "/assistants/42/provider-triggers" + assert captured["params"] == { + "canonical_app_slug": "google_calendar", + "limit": 10, + "offset": 20, + } + assert result == {"available": True, "triggers": []} + + +def test_get_trigger_catalog_omits_unset_optional_params( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setattr(SESSION_DETAILS.assistant, "agent_id", 7) + captured: dict[str, Any] = {} + + def fake_request(method: str, path: str, **kwargs: Any) -> Any: + captured["params"] = kwargs.get("params") + return {"info": {"available": True, "triggers": []}} + + monkeypatch.setattr(typed_tasks_client, "_request", fake_request) + monkeypatch.setattr(typed_tasks_client, "_info", lambda response: response["info"]) + + typed_tasks_client.get_trigger_catalog() + + assert captured["params"] == {} + + +def test_list_provider_trigger_catalog_tool_forwards_params_to_client( + monkeypatch: pytest.MonkeyPatch, +) -> None: + captured: dict[str, Any] = {} + + def fake_get_trigger_catalog(**kwargs: Any) -> dict[str, Any]: + captured.update(kwargs) + return {"available": True, "triggers": []} + + monkeypatch.setattr( + typed_tasks_client, + "get_trigger_catalog", + fake_get_trigger_catalog, + ) + + # `_list_provider_trigger_catalog` never touches `self`, so the unbound + # method can be exercised without constructing a full TaskScheduler. + outcome = TaskScheduler._list_provider_trigger_catalog( + object(), + canonical_app_slug="github", + limit=5, + offset=15, + ) + + assert captured == { + "canonical_app_slug": "github", + "limit": 5, + "offset": 15, + } + assert outcome["outcome"] == "provider trigger catalog listed" + assert outcome["details"]["available"] is True + + +def test_list_provider_trigger_catalog_tool_defaults_forward_none( + monkeypatch: pytest.MonkeyPatch, +) -> None: + captured: dict[str, Any] = {} + + def fake_get_trigger_catalog(**kwargs: Any) -> dict[str, Any]: + captured.update(kwargs) + return {"available": True, "triggers": []} + + monkeypatch.setattr( + typed_tasks_client, + "get_trigger_catalog", + fake_get_trigger_catalog, + ) + + TaskScheduler._list_provider_trigger_catalog(object()) + + assert captured == { + "canonical_app_slug": None, + "limit": None, + "offset": None, + } diff --git a/tests/task_scheduler/test_recurring_template_resilience.py b/tests/task_scheduler/test_recurring_template_resilience.py index cc5813fa2..94835e807 100644 --- a/tests/task_scheduler/test_recurring_template_resilience.py +++ b/tests/task_scheduler/test_recurring_template_resilience.py @@ -69,6 +69,29 @@ def test_matching_scheduled_activation_passes(): ) +def test_equivalent_offset_scheduled_activation_passes(): + scheduler = object.__new__(TaskScheduler) + scheduler._validate_task_matches_provenance( + task=_task("2026-07-27T13:17:00+05:00"), + provenance=_provenance("2026-07-27T08:17:00Z"), + ) + + +def test_normalize_activation_datetime_treats_naive_values_as_utc(): + assert ( + TaskScheduler._normalize_activation_datetime( + "2026-07-27T08:17:00", + ) + == "2026-07-27T08:17:00+00:00" + ) + + +def test_normalize_activation_datetime_preserves_malformed_literal(): + assert TaskScheduler._normalize_activation_datetime("not-a-timestamp") == ( + "not-a-timestamp" + ) + + # --------------------------------------------------------------------------- # # ActiveTask.result finalization # # --------------------------------------------------------------------------- # @@ -79,6 +102,11 @@ async def result(self): raise RuntimeError("occurrence blew up") +class _SuccessfulHandle: + async def result(self): + return "completed inbox summary" + + class _FakeScheduler: def __init__(self): self.status_updates: list[tuple[int, str]] = [] @@ -101,7 +129,6 @@ def _active_task(scheduler: _FakeScheduler, *, rearmed: bool) -> ActiveTask: task._preserve_definition_status = False task._definition_rearmed = rearmed task._summary_scheduled = True - task._task_run_lineage_tokens = None async def _noop_persist(**kwargs): return None @@ -126,6 +153,19 @@ def test_failed_run_terminalizes_non_rearmed_definition(): assert scheduler.status_updates == [(10, "failed")] +def test_successful_result_survives_terminal_persistence_failure(): + scheduler = _FakeScheduler() + task = _active_task(scheduler, rearmed=False) + task._actor_handle = _SuccessfulHandle() + + async def _failing_persist(**kwargs): + raise RuntimeError("storage unavailable") + + task._persist_task_run_terminal_state = _failing_persist + + assert asyncio.run(task.result()) == "completed inbox summary" + + # --------------------------------------------------------------------------- # # offline_runner._mark_source_task_failed # # --------------------------------------------------------------------------- # diff --git a/unify/conversation_manager/comms_manager.py b/unify/conversation_manager/comms_manager.py index 112fd5c60..2e86baec4 100644 --- a/unify/conversation_manager/comms_manager.py +++ b/unify/conversation_manager/comms_manager.py @@ -2175,6 +2175,7 @@ def _normalize_recipients(value): call_session_id=event.get("call_session_id"), provider_call_sid=event.get("provider_call_sid"), room_name=event.get("room_name") or event.get("livekit_room"), + recording_started_at=event.get("recording_started_at"), ).to_json(), ) ack_now() diff --git a/unify/conversation_manager/conversation_manager.py b/unify/conversation_manager/conversation_manager.py index 209127752..2e8c43685 100644 --- a/unify/conversation_manager/conversation_manager.py +++ b/unify/conversation_manager/conversation_manager.py @@ -416,6 +416,12 @@ def __init__( # the exchange without a database filter query. self._recording_exchange_ids: dict[str, int] = {} + # Detached recording-start requests. Recording must never gate call + # setup, so the call-started handler fires the request without awaiting + # it; the set holds a strong reference so the task is not garbage + # collected mid-flight. + self._recording_start_tasks: set[asyncio.Task] = set() + # Groups messages into conversation-thread exchanges (SMS / WhatsApp / # Discord / MS Teams bot / Slack DMs and channels, and email). Maps a # per-conversation key to its exchange_id. 1:1 DMs reuse a single diff --git a/unify/conversation_manager/domains/call_manager.py b/unify/conversation_manager/domains/call_manager.py index 0302a2be5..a6712618f 100644 --- a/unify/conversation_manager/domains/call_manager.py +++ b/unify/conversation_manager/domains/call_manager.py @@ -17,6 +17,7 @@ CallEventSocketServer, CM_EVENT_SOCKET_ENV, ) +from unify.conversation_manager.speaker_id import VOICE_PROFILES_ENV from unify.logger import LOGGER from unify.common.hierarchical_logger import DEFAULT_ICON, ICONS from unify.helpers import ( @@ -113,6 +114,26 @@ class CallConfig: WORKER_REWARM_STALL_S = 60.0 +# Cookies Google sets on a signed-in session. A storage state missing all of +# them is a signed-out (or challenge) context and must never overwrite the +# durable blob the next join hydrates from. +GOOGLE_AUTH_COOKIE_NAMES = frozenset({"SID", "__Secure-1PSID", "__Secure-3PSID"}) + + +def _state_has_google_auth_cookies(state: str | None) -> bool: + if not state: + return False + try: + cookies = json.loads(state).get("cookies") or [] + except (json.JSONDecodeError, AttributeError): + return False + return any( + cookie.get("name") in GOOGLE_AUTH_COOKIE_NAMES + and "google.com" in (cookie.get("domain") or "") + for cookie in cookies + ) + + def _opener_opening_config(opener: str, *, source: str, briefing: str = "") -> dict: config = { "mode": "opener", @@ -618,8 +639,8 @@ async def _wait_for_worker_registered( def _get_voice_profiles( self, - contact: dict, - boss: dict, + contact: dict | None, + boss: dict | None, extra_contact_ids: list[int] | None = None, ) -> dict[str, list[float]]: """Fetch enrolled voice embeddings for the call participants. @@ -904,9 +925,31 @@ async def refresh_unify_meet_roster(self, participants: list[dict] | None) -> No self.unify_meet_participants = roster if self._event_broker is None: return + # Profiles otherwise only ride the initial dispatch, so anyone joining + # after the call started can never be voice-pinned. Off-thread: the + # lookup hits the backend and this runs on the event loop. + roster_contact_ids = [ + int(p["contact_id"]) + for p in roster + if isinstance(p, dict) + and p.get("contact_id") is not None + and p.get("kind") != "assistant" + ] + profiles = await asyncio.to_thread( + self._get_voice_profiles, + None, + None, + roster_contact_ids, + ) await self._event_broker.publish( "app:call:status", - json.dumps({"type": "unify_meet_roster", "participants": roster}), + json.dumps( + { + "type": "unify_meet_roster", + "participants": roster, + "voice_profiles": profiles, + }, + ), ) async def start_unify_meet( @@ -1188,6 +1231,102 @@ def _download() -> str: ) return True + async def _persist_meet_browser_state( + self, + base_url: str, + auth_key: str, + storage_state_name: str, + session_id: str, + ) -> bool: + """Write the live meet browser's refreshed cookies back to GCS. + + Google rotates session cookies on use, so the stored snapshot decays and + eventually lands the browser on a re-auth challenge. After a confirmed + signed-in join, snapshot the live context via the agent-service + (``POST /browser-states//save`` then ``GET``) and replace the + durable blob, keeping the stored session as fresh as the last join. The + upload is skipped unless the snapshot still carries Google auth cookies, + so a signed-out context can never clobber a good state. Best-effort: + failures log a warning and leave the existing blob untouched. + """ + bucket = (os.environ.get("MEET_BROWSER_STATE_BUCKET") or "").strip() + if not bucket: + return False + + blob_name = f"{storage_state_name}.json" + + try: + async with aiohttp.ClientSession() as session: + headers = {"authorization": f"Bearer {auth_key}"} + async with session.post( + f"{base_url}/browser-states/{storage_state_name}/save", + json={"sessionId": session_id}, + headers=headers, + timeout=aiohttp.ClientTimeout(total=15.0), + ) as save_resp: + if save_resp.status not in (200, 204): + detail = await save_resp.text() + LOGGER.warning( + f"{ICONS['ipc']} [LivekitCallManager] agent-service " + f"could not snapshot browser state " + f"{storage_state_name} (HTTP {save_resp.status}): " + f"{detail}; keeping the stored session", + ) + return False + async with session.get( + f"{base_url}/browser-states/{storage_state_name}", + headers=headers, + timeout=aiohttp.ClientTimeout(total=15.0), + ) as get_resp: + if get_resp.status != 200: + detail = await get_resp.text() + LOGGER.warning( + f"{ICONS['ipc']} [LivekitCallManager] could not read " + f"back browser state {storage_state_name} (HTTP " + f"{get_resp.status}): {detail}; keeping the stored " + "session", + ) + return False + state = (await get_resp.json()).get("state") + except (aiohttp.ClientError, asyncio.TimeoutError) as exc: + LOGGER.warning( + f"{ICONS['ipc']} [LivekitCallManager] failed to snapshot browser " + f"state {storage_state_name}: {exc!r}; keeping the stored session", + ) + return False + + if not _state_has_google_auth_cookies(state): + LOGGER.warning( + f"{ICONS['ipc']} [LivekitCallManager] snapshot of " + f"{storage_state_name} carries no Google auth cookies; keeping " + "the stored session", + ) + return False + + def _upload() -> None: + from google.cloud import storage + + client = storage.Client() + client.bucket(bucket).blob(blob_name).upload_from_string( + state, + content_type="application/json", + ) + + try: + await asyncio.to_thread(_upload) + except Exception as exc: # best-effort: the stored blob stays valid + LOGGER.warning( + f"{ICONS['ipc']} [LivekitCallManager] could not upload refreshed " + f"browser state to gs://{bucket}/{blob_name}: {exc!r}", + ) + return False + + LOGGER.info( + f"{ICONS['ipc']} [LivekitCallManager] refreshed browser state " + f"{storage_state_name} (gs://{bucket}/{blob_name})", + ) + return True + async def _start_meet( self, channel: str, @@ -1453,6 +1592,17 @@ async def _start_meet( f"status={body.get('status')})", ) + # The join proved the hydrated session is still signed in, and Google + # rotated its cookies in the process. Write the refreshed context back + # so the stored state never drifts further behind than the last call. + if storage_state_name and self._meet_session_id: + await self._persist_meet_browser_state( + base_url, + auth_key, + storage_state_name, + self._meet_session_id, + ) + if self._socket_server and self._meet_session_id: await self._socket_server.queue_for_clients( "app:call:status", @@ -1661,6 +1811,17 @@ async def _start_call_subprocess( if extra_env: for k, v in extra_env.items(): os.environ[k.upper()] = str(v) + # Voice profiles ride the dispatch metadata on the worker path; this + # path has no metadata, so without an env equivalent speaker pinning is + # silently off for every env-configured call. Cleared rather than left + # set when empty, or a previous call's profiles leak into this one. + # A 512-float embedding is ~10 KB of JSON, so contact + boss stays well + # inside the per-variable environment limit. + profiles = self._get_voice_profiles(contact, boss) + if profiles: + os.environ[VOICE_PROFILES_ENV] = json.dumps(profiles) + else: + os.environ.pop(VOICE_PROFILES_ENV, None) if socket_path: os.environ[CM_EVENT_SOCKET_ENV] = socket_path LOGGER.debug( diff --git a/unify/conversation_manager/domains/event_handlers.py b/unify/conversation_manager/domains/event_handlers.py index 62f47539a..e604b9616 100644 --- a/unify/conversation_manager/domains/event_handlers.py +++ b/unify/conversation_manager/domains/event_handlers.py @@ -291,6 +291,74 @@ def _active_voice_thread_medium(cm: "ConversationManager") -> Medium: return Medium.PHONE_CALL +async def _start_session_recording(event, cm: "ConversationManager") -> None: + """Request a recording for the LiveKit room backing a just-started session. + + Browser meets (Google Meet / Teams) are not requested at all: their audio + is bridged through the agent-service audio device and never reaches the + LiveKit room, so there is nothing for the compositor to mix. The gateway + enforces the same rule, this just avoids the pointless hop. + + Best-effort throughout -- a recording problem must never disturb a live + call, so every failure is logged and swallowed. + """ + if isinstance(event, (GoogleMeetStarted, TeamsMeetStarted)): + return + + from unify.settings import SETTINGS + + if ( + SETTINGS.conversation.LOCAL_COMMS_ENABLED + or SETTINGS.conversation.LOCAL_COMMS_MODE == "local" + ): + # Self-host owns recording in-process: its egress carries a + # ``/local/livekit/recording-complete`` callback that only the local + # ingress serves, whereas the gateway points completion webhooks at + # ADAPTERS_URL. Routing through the gateway here would upload a + # recording whose completion event nobody receives. + return + + call_manager = cm.call_manager + room_name = call_manager.room_name + if not room_name: + cm._session_logger.debug( + "recording", + f"{DEFAULT_ICON} [Recording] No room name on the active session; " + "skipping recording request", + ) + return + + if isinstance(event, UnifyMeetStarted): + call_session_id = call_manager.unify_meet_call_session_id or ( + event.call_session_id or "" + ) + provider_call_sid = "" + conference_name = "" + else: + call_session_id = call_manager.call_session_id + provider_call_sid = call_manager.provider_call_sid + conference_name = call_manager.conference_name + + from unify.conversation_manager.utils import start_call_recording + + try: + await asyncio.to_thread( + start_call_recording, + room_name, + str(call_manager.assistant_id or ""), + user_id=str(call_manager.user_id or ""), + call_session_id=call_session_id, + provider_call_sid=provider_call_sid, + conference_name=conference_name, + ) + except Exception as exc: + cm._session_logger.debug( + "recording", + f"{DEFAULT_ICON} [Recording] Failed to request recording for " + f"room {room_name}: {exc}", + ) + + def _call_not_answered_reason_text(reason: str) -> str: """Return the user-facing text for one telephony not-answered reason.""" @@ -1012,6 +1080,22 @@ async def _( conv_state = cm.contact_index.get_or_create_conversation(contact_id) conv_state.on_call = True + # Start recording now rather than at dispatch: this is the first moment the + # LiveKit room is known to exist and carry audio, which is what the Room + # Composite Egress compositor waits for. Started earlier (at SIP bridge + # setup or agent dispatch) it races the join and LiveKit kills the job with + # "Start signal not received", producing no file. Browser meets are excluded + # by the gateway: their audio never enters the LiveKit room. + # + # Detached deliberately: the request is a gateway round-trip that ends in a + # LiveKit API call, and everything below (the outbound call_answered status, + # the meet interaction-state sync, guidance delivery) is call-start UX that + # must not wait on it. A slow or unreachable gateway would otherwise stall + # the answer transition for as long as the request takes to time out. + _recording_task = asyncio.create_task(_start_session_recording(event, cm)) + cm._recording_start_tasks.add(_recording_task) + _recording_task.add_done_callback(cm._recording_start_tasks.discard) + if isinstance(event, UnifyMeetStarted) and cm.call_manager.is_outbound: await cm.event_broker.publish( "app:call:status", @@ -2092,12 +2176,14 @@ async def _( *args, **kwargs, ): - keys = [ - event.call_session_id, - event.provider_call_sid, - event.room_name, - event.conference_name, + # Identifier -> the exchange metadata key it was stored under at call end. + lookups = [ + ("call_session_id", event.call_session_id), + ("provider_call_sid", event.provider_call_sid), + ("room_name", event.room_name), + ("conference_name", event.conference_name), ] + keys = [value for _, value in lookups] name = next((key for key in keys if key), event.conference_name) exchange_id = None for key in keys: @@ -2105,6 +2191,26 @@ async def _( exchange_id = cm._recording_exchange_ids.pop(key, None) if exchange_id is not None: break + if exchange_id is None: + # The in-process map only covers a recording that arrives while the + # container that ran the call is still up. Egress finalises minutes + # after the room closes, by which time the pod has often been recycled, + # so fall back to the identifiers persisted on the exchange itself. + for metadata_key, value in lookups: + if not value: + continue + exchange_id = await asyncio.to_thread( + cm.transcript_manager.resolve_exchange_id_by_metadata, + metadata_key, + value, + ) + if exchange_id is not None: + cm._session_logger.debug( + "recording", + f"{DEFAULT_ICON} [RecordingReady] Recovered exchange " + f"{exchange_id} from stored {metadata_key} for {name}", + ) + break if exchange_id is not None: cm.transcript_manager.update_exchange_metadata( exchange_id, @@ -2112,6 +2218,7 @@ async def _( key: value for key, value in { "recording_url": event.recording_url, + "recording_started_at": event.recording_started_at, "recording_call_session_id": event.call_session_id, "recording_provider_call_sid": event.provider_call_sid, "recording_room_name": event.room_name, diff --git a/unify/conversation_manager/domains/managers_utils.py b/unify/conversation_manager/domains/managers_utils.py index fce9a5ed9..c1228546d 100644 --- a/unify/conversation_manager/domains/managers_utils.py +++ b/unify/conversation_manager/domains/managers_utils.py @@ -1,4 +1,3 @@ -from datetime import timedelta import asyncio import os from time import perf_counter @@ -23,7 +22,6 @@ ) from unify.conversation_manager.event_broker import get_event_broker from unify.conversation_manager.events import * -from unify.common.prompt_helpers import now as prompt_now from unify.events.event_bus import EVENT_BUS from unify.manager_registry import ManagerRegistry from unify.function_manager.primitives import default_runtime_scope @@ -1142,6 +1140,40 @@ def _derive_conversation_key( return None +def call_start_for_medium(call_manager, medium: "Medium"): + """The session-start instant this medium measures utterance offsets from.""" + if medium in (Medium.PHONE_CALL, Medium.WHATSAPP_CALL): + return call_manager.call_start_timestamp + if medium == Medium.UNIFY_MEET: + return call_manager.unify_meet_start_timestamp + if medium == Medium.GOOGLE_MEET: + return call_manager.google_meet_start_timestamp + if medium == Medium.TEAMS_MEET: + return call_manager.teams_meet_start_timestamp + return None + + +def call_utterance_stamp(call_start, spoken_at) -> str: + """``MM.SS`` from session start to this utterance, or "" outside a call. + + Measured from the utterance's own timestamp rather than the clock at logging + time. This runs on the transcript worker, so reading the clock here charges + every utterance for however long its write queued behind exchange creation + and context provisioning -- worst on a call's first utterance (18s observed + in staging), decaying as the pipeline warms. That pushed early offsets well + past their position in the audio. + + Still only an approximation of a position in the recording: the session-start + event precedes the egress compositor by a few seconds. Consumers that need + an exact position use the ``recording_started_at`` anchor on the exchange. + """ + if not call_start or spoken_at is None: + return "" + elapsed = int((spoken_at - call_start).total_seconds()) + minutes, seconds = divmod(max(0, elapsed), 60) + return f"{minutes:02d}.{seconds:02d}" + + def _conversation_exchange_metadata( event: "Event", medium: "Medium", @@ -1457,30 +1489,10 @@ async def log_message( if recovered is not None: exchange_id = recovered - call_utterance_timestamp = "" - call_start = ( - cm.call_manager.call_start_timestamp - if medium in (Medium.PHONE_CALL, Medium.WHATSAPP_CALL) - else ( - cm.call_manager.unify_meet_start_timestamp - if medium == Medium.UNIFY_MEET - else ( - cm.call_manager.google_meet_start_timestamp - if medium == Medium.GOOGLE_MEET - else ( - cm.call_manager.teams_meet_start_timestamp - if medium == Medium.TEAMS_MEET - else None - ) - ) - ) + call_utterance_timestamp = call_utterance_stamp( + call_start_for_medium(cm.call_manager, medium), + event.timestamp, ) - if call_start: - delta = prompt_now(as_string=False) - call_start - if role == "Assistant": - delta += timedelta(seconds=2) - minutes, seconds = divmod(int(delta.total_seconds()), 60) - call_utterance_timestamp = f"{minutes:02d}.{seconds:02d}" # publish transcript on a separate thread def _publish_transcript() -> int: diff --git a/unify/conversation_manager/domains/renderer.py b/unify/conversation_manager/domains/renderer.py index 3e6230773..fe2f23a96 100644 --- a/unify/conversation_manager/domains/renderer.py +++ b/unify/conversation_manager/domains/renderer.py @@ -1969,6 +1969,9 @@ def render_completed_actions( ) out += f"\n" out += f"{query}\n" + task_description = handle_data.get("task_description") + if task_description: + out += f"{task_description}\n" if terminal_event is not None: if terminal_event.get("success") is False: diff --git a/unify/conversation_manager/domains/task_execution.py b/unify/conversation_manager/domains/task_execution.py index e1b495457..5e308de4a 100644 --- a/unify/conversation_manager/domains/task_execution.py +++ b/unify/conversation_manager/domains/task_execution.py @@ -98,6 +98,7 @@ async def _register_live_task_handle( *, handle: "SteerableToolHandle", query: str, + task_description: str | None = None, ) -> int: """Register a deterministically started task with CM steering state.""" @@ -118,6 +119,8 @@ async def _register_live_task_handle( "initial_snapshot_state": getattr(cm, "_current_snapshot_state", None), "context_opted_in": False, } + if task_description: + cm.in_flight_actions[handle_id]["task_description"] = task_description asyncio.create_task( managers_utils.actor_watch_result( handle_id, @@ -165,7 +168,12 @@ async def _start_live_task_due_execution( f"Scheduled task due now: '{_task_due_label(event, activation)}' " f"(task_id={event.task_id})." ) - return await _register_live_task_handle(cm, handle=handle, query=query) + return await _register_live_task_handle( + cm, + handle=handle, + query=query, + task_description=activation.task_description, + ) async def _start_live_task_trigger_execution( @@ -180,6 +188,11 @@ async def _start_live_task_trigger_execution( ) scheduler = ManagerRegistry.get_task_scheduler() + task_description: str | None = None + try: + task_description = scheduler._get_task_or_raise(event.task_id).description + except ValueError: + task_description = None delegate = _ConversationTaskExecutionDelegate(cm.actor) delegate_token = current_task_execution_delegate.set(delegate) try: @@ -194,7 +207,12 @@ async def _start_live_task_trigger_execution( f"Task triggered via REST API: '{_task_trigger_label(event)}' " f"(task_id={event.task_id})." ) - return await _register_live_task_handle(cm, handle=handle, query=query) + return await _register_live_task_handle( + cm, + handle=handle, + query=query, + task_description=task_description, + ) def _current_task_assistant_id() -> str | None: @@ -792,7 +810,12 @@ async def _handle_provider_event_dispatch_requested_event( f"Provider event started task {event.task_id} " f"(operation {event.operation_id})." ) - await _register_live_task_handle(cm, handle=handle, query=query) + await _register_live_task_handle( + cm, + handle=handle, + query=query, + task_description=outcome.description, + ) cm.notifications_bar.push_notif("Tasks", query, event.timestamp) return False diff --git a/unify/conversation_manager/engaged_vad.py b/unify/conversation_manager/engaged_vad.py index d40f18e39..8461849d6 100644 --- a/unify/conversation_manager/engaged_vad.py +++ b/unify/conversation_manager/engaged_vad.py @@ -43,10 +43,20 @@ def __init__( *, inner: agents_vad.VAD, scorer: RealtimeSpeakerScorer, + feed_scorer: bool = True, ) -> None: + """``feed_scorer`` selects where the scorer's audio comes from. + + The scorer keeps one rolling window, so it must have exactly one audio + source; mixing two interleaves unrelated streams into the same window + and the embeddings become meaningless. On browser meets the caller + audio arrives over the PortAudio bridge rather than the LiveKit room, + so the call script feeds the scorer there and sets this False. + """ super().__init__(capabilities=inner.capabilities) self._inner = inner self._scorer = scorer + self._feed_scorer = feed_scorer @property def model(self) -> str: @@ -75,11 +85,12 @@ async def _feed() -> None: if isinstance(item, self._FlushSentinel): inner.flush() continue - scorer.add_audio( - bytes(item.data), - item.sample_rate, - item.num_channels, - ) + if self._gate._feed_scorer: + scorer.add_audio( + bytes(item.data), + item.sample_rate, + item.num_channels, + ) inner.push_frame(item) inner.end_input() diff --git a/unify/conversation_manager/events.py b/unify/conversation_manager/events.py index b0466e9d3..3a53426cc 100644 --- a/unify/conversation_manager/events.py +++ b/unify/conversation_manager/events.py @@ -582,6 +582,10 @@ class RecordingReady(Event): call_session_id: str | None = None provider_call_sid: str | None = None room_name: str | None = None + # ISO8601 instant the egress compositor began writing, i.e. t=0 of the audio + # file. Consumers time-align transcript utterances against this rather than + # against the call-started event, which precedes it by a few seconds. + recording_started_at: str | None = None @dataclass diff --git a/unify/conversation_manager/local_ingress.py b/unify/conversation_manager/local_ingress.py index a88894f95..d661eedb9 100644 --- a/unify/conversation_manager/local_ingress.py +++ b/unify/conversation_manager/local_ingress.py @@ -5,6 +5,7 @@ import secrets import time import uuid +from datetime import datetime, timezone from aiohttp import ClientSession, web @@ -980,6 +981,15 @@ async def _livekit_recording_complete(self, request: web.Request) -> web.Respons egress_info.room_name, ), "recording_url": recording_url, + # t=0 of the audio file, for time-aligning utterances. + "recording_started_at": ( + datetime.fromtimestamp( + egress_info.started_at / 1_000_000_000, + tz=timezone.utc, + ).isoformat() + if egress_info.started_at + else "" + ), }, }, ) diff --git a/unify/conversation_manager/medium_scripts/call.py b/unify/conversation_manager/medium_scripts/call.py index 5e9fc44e4..a6c026316 100644 --- a/unify/conversation_manager/medium_scripts/call.py +++ b/unify/conversation_manager/medium_scripts/call.py @@ -103,6 +103,14 @@ # Module-level logger created early for prewarm (before entrypoint runs). _log = FastBrainLogger() +# Channels where many people share one audio stream with no single primary, so +# every distinct voice needs its own "Speaker N" label even when nobody on the +# call is enrolled. Deliberately wider than the ("google_meet", "teams_meet") +# checks elsewhere in this module: those mean *browser* meets specifically — +# DOM scraping and the PortAudio bridge — whereas Unify Meet is LiveKit-native +# but just as multi-party. +MULTI_PARTY_CHANNELS = ("google_meet", "teams_meet", "unify_meet") + DEPLETED_CREDITS_FAST_BRAIN_RESPONSE = ( "Your credits are depleted, so I can't continue helping with setup or tasks " "until you top up. Please add credits in billing, then I'll pick this back up." @@ -878,10 +886,18 @@ async def stt_node( async def _audio_from_bridge(): tracker = self.speaker_tracker + scorer = self.realtime_scorer while True: pcm = await self.audio_bridge.capture_q.get() if tracker is not None: tracker.add_audio(pcm, _RATE, 1) + if scorer is not None: + # The LiveKit room carries no caller audio on a browser + # meet, so the scorer's usual feed inside EngagedGateVAD is + # silent and its verdict never leaves "unknown". This is + # the only real audio on this channel; the VAD wrapper is + # constructed with feed_scorer=False to keep it single-fed. + scorer.add_audio(pcm, _RATE, 1) samples = len(pcm) // 2 yield rtc.AudioFrame( data=pcm, @@ -1660,8 +1676,19 @@ def _resolve_contact_by_name(display_name: str) -> dict | None: # Pins Deepgram's per-call anonymous speaker ids to enrolled contact voice # profiles, accumulates an auto-enrollment on single-voice calls, and # suggests manual enrollment when multiple unattributable voices are heard. + # Dispatch metadata on the worker path; an env var on the legacy per-call + # subprocess path, which has no metadata to carry them. + _raw_profiles = (meta or {}).get("voice_profiles") + if not _raw_profiles: + try: + _raw_profiles = json.loads( + os.environ.get(speaker_id.VOICE_PROFILES_ENV, "") or "{}", + ) + except json.JSONDecodeError: + _log.error("Malformed VOICE_PROFILES env var; speaker pinning disabled") + _raw_profiles = {} voice_profiles: dict[int, list[float]] = {} - for _cid, _vec in ((meta or {}).get("voice_profiles") or {}).items(): + for _cid, _vec in (_raw_profiles or {}).items(): try: voice_profiles[int(_cid)] = [float(x) for x in _vec] except (TypeError, ValueError): @@ -1720,7 +1747,7 @@ def _on_enrollment_suggested(num_speakers: int) -> None: embedder=SPEAKER_EMBEDDER, enrolled_profiles=voice_profiles, call_contact_id=contact.get("contact_id"), - multi_party=channel in ("google_meet", "teams_meet"), + multi_party=channel in MULTI_PARTY_CHANNELS, on_enrollment_captured=_on_enrollment_captured, on_enrollment_suggested=_on_enrollment_suggested, ) @@ -1778,7 +1805,13 @@ def on_speaker_engagement(data: dict) -> None: engaged_speakers, ), ) - session_vad = EngagedGateVAD(inner=VAD, scorer=realtime_scorer) + session_vad = EngagedGateVAD( + inner=VAD, + scorer=realtime_scorer, + # Browser meets feed the scorer from the PortAudio bridge instead + # (see Assistant.stt_node); one source only. + feed_scorer=channel not in ("google_meet", "teams_meet"), + ) _log.config("Engaged-speaker floor gating active (call contact enrolled)") def _speaker_is_engaged(sid: str | None) -> bool: @@ -2599,7 +2632,12 @@ async def _on_job_shutdown(): if speaker_tracker is not None: # Flush pending embeddings and fire a partial auto-enrollment for # single-voice calls that ended before reaching the full target. + # finalize() also emits the attribution summary. await speaker_tracker.finalize() + if realtime_scorer is not None: + # Floor gating fails open, so an inert scorer looks exactly + # like a working one from the outside; the tally is the tell. + realtime_scorer.log_summary() if speaker_event_tasks: await asyncio.gather( *list(speaker_event_tasks), @@ -3185,6 +3223,17 @@ def on_status(data: dict) -> None: meet_session_id = data.get("session_id", "") elif event_type == "unify_meet_roster": incoming = data.get("participants") or [] + if speaker_tracker is not None: + # Late joiners are unpinnable otherwise: profiles are a + # snapshot taken at dispatch and this is the only point where + # the roster is known to have changed. + _profiles: dict[int, list[float]] = {} + for _cid, _vec in (data.get("voice_profiles") or {}).items(): + try: + _profiles[int(_cid)] = [float(x) for x in _vec] + except (TypeError, ValueError): + continue + speaker_tracker.add_enrolled_profiles(_profiles) if isinstance(incoming, list): unify_meet_roster.clear() unify_meet_roster.extend( @@ -4253,9 +4302,6 @@ async def _prepare_opening() -> tuple[str, str | dict | None]: _log.dispatch(f"Dispatching LiveKit agent {agent_name} into room {room_name}") dispatch_livekit_agent( room_name, - record=True, - assistant_id=SESSION_DETAILS.assistant.agent_id, - user_id=SESSION_DETAILS.user.id, agent_name=agent_name, call_session_id=call_session_id, ) diff --git a/unify/conversation_manager/speaker_id.py b/unify/conversation_manager/speaker_id.py index 51cc48438..4999f010c 100644 --- a/unify/conversation_manager/speaker_id.py +++ b/unify/conversation_manager/speaker_id.py @@ -20,11 +20,12 @@ import asyncio import io +import logging import os import tempfile import time import wave -from collections import deque +from collections import Counter, deque from concurrent.futures import ThreadPoolExecutor from dataclasses import dataclass, field from pathlib import Path @@ -32,10 +33,20 @@ import numpy as np +# Plain stdlib logger: this module is imported by the LiveKit voice-agent child +# process, where the heavyweight ``unify.logger`` import chain is best avoided. +# Handlers are inherited from whichever process hosts it. +_log = logging.getLogger(__name__) + # ───────────────────────────────────────────────────────────────────────────── # Model management # ───────────────────────────────────────────────────────────────────────────── +# Carries ``{contact_id: embedding}`` to the legacy per-call subprocess, which +# has no LiveKit job metadata to ride along in. JSON-encoded, same shape as the +# ``voice_profiles`` metadata key on the worker path. +VOICE_PROFILES_ENV = "VOICE_PROFILES" + SPEAKER_MODEL_NAME = "wespeaker_en_voxceleb_CAM++.onnx" SPEAKER_MODEL_URL = ( "https://github.com/k2-fsa/sherpa-onnx/releases/download/" @@ -59,12 +70,27 @@ # diarization ids as the same physical voice when gating auto-enrollment. # Streaming diarization often over-splits one talker into S0/S1/…; without this # merge those ids each count as a distinct voice and spuriously block -# enrollment. Same-speaker CAM++ scores typically land in 0.6–0.8 and different -# speakers below ~0.4. Set at half the same-speaker floor (0.6 → 0.3) so genuine -# over-splits clear the bar with ~50% relative margin while orthogonal -# different-speaker pairs stay separate. Raise gradually if real multi-speaker -# calls start collapsing; lower if over-splits still trip the suggestion. -CROSS_ID_MERGE_SIM = 0.3 +# enrollment. +# +# This asks the same question as ``CLUSTER_JOIN_SIM`` — "is this the same +# voice?" — only across diarization ids rather than within one, so the two are +# deliberately tied together. They previously diverged: this sat at 0.3, below +# any plausible different-speaker score, which made the merge unable to keep +# *anyone* apart. Since the merge is what gates auto-enrollment, a two-person +# call read as one voice and enrolled a blended voiceprint. +# +# PROVISIONAL: the same-speaker (0.6-0.8) and different-speaker (<0.4) bands +# these thresholds assume are inherited, not measured. Validate against a real +# multi-speaker corpus via ``$UNIFY_SPEAKER_TEST_CORPUS`` before treating the +# value as settled — see tests/conversation_manager/voice/speaker_corpus.py. +CROSS_ID_MERGE_SIM = CLUSTER_JOIN_SIM + +# Minimum accumulated speech before a voice cluster counts as a distinct +# person for enrollment/suggestion gating. Without a floor, one noisy or +# clipped segment seeds a cluster that reads as a whole extra speaker — +# blocking auto-enrollment and firing the "multiple voices" suggestion on what +# is really a single-speaker call. +MIN_VOICE_DURATION_S = 2.0 # Auto-enrollment bounds (seconds of accumulated speech from a single voice). ENROLLMENT_TARGET_S = 60.0 @@ -72,7 +98,17 @@ # Per-segment slicing bounds around a final transcript. SEGMENT_MAX_S = 15.0 -SEGMENT_MIN_S = 0.8 + +# Minimum audio behind one embedding. CAM++ pools frame statistics, so a short +# slice does not merely embed *noisily* — it embeds somewhere else entirely. +# Measured against the real extractor, a slice scored against its own speaker's +# profile: 0.8s -> 0.20, 1.0s -> 0.31, 1.5s -> 0.41, 2.0s -> 0.60, 3.0s -> 0.74. +# Below ~2s the result is unusable and averaging does not rescue it (a centroid +# of fifteen 0.8s segments plateaus near 0.32, never reaching the match +# threshold). Finals shorter than this are buffered per diarization id rather +# than discarded, so backchannels still contribute instead of seeding phantom +# clusters. See tests/conversation_manager/voice/test_speaker_id_real_model.py. +SEGMENT_MIN_S = 2.0 # Sample rate used for persisted enrollment audio and embedding input. ENROLLMENT_SAMPLE_RATE = 16000 @@ -82,10 +118,29 @@ # Realtime floor-gating scorer: rolling window size, inference cadence, and # how long a confident non-engaged verdict must persist before it gates the # floor (hysteresis against per-window jitter). -REALTIME_WINDOW_S = 1.0 -REALTIME_HOP_S = 0.25 +# +# The window is subject to the same duration floor as ``SEGMENT_MIN_S`` — it is +# the same extractor. At the previous 1.0s, windows of a speaker's *own* audio +# scored 0.16-0.35 against their *own* enrolled profile and never once reached +# the match threshold, so the verdict was permanently "unknown" and the gate +# never acted. At 2.0s every window clears it. The hop is widened in step to +# keep inference cost roughly constant. +REALTIME_WINDOW_S = 2.0 +REALTIME_HOP_S = 0.5 NON_ENGAGED_HYSTERESIS_S = 1.0 +# Acceptance threshold for the realtime scorer, kept separate from +# SPEAKER_MATCH_THRESHOLD even though they currently agree: the scorer judges +# short rolling windows rather than a settled cluster centroid, so its +# operating point has to be free to move without silently retuning who gets +# pinned on the transcript. +REALTIME_MATCH_THRESHOLD = 0.55 + +# Fraction of a full window that must be buffered before scoring. Guards the +# ramp-up after silence: a partial window is a short segment, with exactly the +# unusable-embedding problem the window length above exists to avoid. +REALTIME_MIN_WINDOW_FRACTION = 0.9 + # Windows quieter than this int16 RMS are treated as silence and produce an # "unknown" verdict instead of a garbage embedding. REALTIME_MIN_RMS = 250.0 @@ -432,6 +487,10 @@ class _SpeakerState: enrollment_audio: list[np.ndarray] = field(default_factory=list) enrollment_duration_s: float = 0.0 enrollment_sample_rate: int = ENROLLMENT_SAMPLE_RATE + # Finals too short to embed on their own, held at ENROLLMENT_SAMPLE_RATE + # until they add up to SEGMENT_MIN_S. Bounded: flushed as soon as they do. + pending_audio: list[np.ndarray] = field(default_factory=list) + pending_duration_s: float = 0.0 class SpeakerTracker: @@ -501,6 +560,57 @@ def __init__( self._suggestion_fired = False self._pending_tasks: set[asyncio.Task] = set() + # Attribution runs entirely off fire-and-forget tasks whose exceptions + # nothing retrieves, and every upstream failure (absent model, empty + # profile map) degrades silently — so the feature can be dead in + # production with no signal at all. These counters plus ``diagnostics`` + # are that signal. + self._segments_observed = 0 + self._segments_buffered = 0 + self._segments_dropped = 0 + self._segments_embedded = 0 + self._embed_failures = 0 + + _log.info( + "SpeakerTracker: %d enrolled profile(s), call_contact=%s " + "(enrolled=%s), multi_party=%s", + len(self._enrolled), + self._call_contact_id, + self._call_contact_enrolled, + self._multi_party, + ) + + def add_enrolled_profiles(self, profiles: dict[int, list[float]]) -> int: + """Merge in profiles that were not known when the call started. + + Enrolled profiles are otherwise a snapshot taken at dispatch, so anyone + who joins a multi-party call later cannot be voice-pinned however good + their enrollment is. Returns the number newly added. + + Only unknown contacts are added: an existing profile is left alone so a + late roster push cannot disturb pins already made on this call. Voices + already clustered are re-scored against the enlarged set on their next + segment, so a late joiner who has already spoken is picked up without + replaying anything. + """ + added = 0 + for contact_id, vector in (profiles or {}).items(): + try: + cid = int(contact_id) + except (TypeError, ValueError): + continue + if cid in self._enrolled: + continue + self._enrolled[cid] = np.asarray(vector, dtype=np.float32) + added += 1 + if added: + _log.info( + "SpeakerTracker: +%d enrolled profile(s) mid-call (%d total)", + added, + len(self._enrolled), + ) + return added + # ── audio ingestion ────────────────────────────────────────────────── def add_audio( @@ -525,16 +635,43 @@ def observe_final_transcript( *, end_ts: float | None = None, ) -> None: - """Register a final diarized transcript; schedules embedding work.""" + """Register a final diarized transcript; schedules embedding work. + + Finals carrying less than ``SEGMENT_MIN_S`` of audio — backchannels + like "yeah" or "mm-hm", which are a large share of real call turns — + are accumulated against their diarization id instead of embedded on + their own, and flushed once they add up. Embedding them individually + produced vectors unrelated to the speaker, which then seeded phantom + voice clusters and mislabelled the caller's own short turns. + """ end_ts = end_ts if end_ts is not None else time.time() window_start = max(self._last_final_ts, end_ts - SEGMENT_MAX_S) self._last_final_ts = end_ts if not speaker_id: return + self._segments_observed += 1 pcm, sample_rate = self._ring.slice(window_start, end_ts) duration_s = len(pcm) / sample_rate if sample_rate else 0.0 - if duration_s < SEGMENT_MIN_S: + if duration_s <= 0.0: return + + state = self._speakers.setdefault(speaker_id, _SpeakerState()) + if state.pending_audio or duration_s < SEGMENT_MIN_S: + # Normalized to one rate so buffered pieces can be concatenated; + # the extractor resamples internally either way. + state.pending_audio.append( + resample_pcm(pcm, sample_rate, ENROLLMENT_SAMPLE_RATE), + ) + state.pending_duration_s += duration_s + if state.pending_duration_s < SEGMENT_MIN_S: + self._segments_buffered += 1 + return + pcm = np.concatenate(state.pending_audio) + sample_rate = ENROLLMENT_SAMPLE_RATE + duration_s = state.pending_duration_s + state.pending_audio = [] + state.pending_duration_s = 0.0 + task = asyncio.create_task( self._process_segment(speaker_id, pcm, sample_rate, duration_s), ) @@ -548,7 +685,22 @@ async def _process_segment( sample_rate: int, duration_s: float, ) -> None: - embedding = await self._embedder.embed(pcm, sample_rate) + try: + embedding = await self._embedder.embed(pcm, sample_rate) + except Exception: + # Nothing awaits this task's result, so without an explicit log an + # unusable extractor (missing model, bad audio) silently disables + # attribution for the whole call. + self._embed_failures += 1 + _log.exception( + "speaker embedding failed for %s (%.1fs @ %dHz); " + "attribution degraded for this segment", + speaker_id, + duration_s, + sample_rate, + ) + return + self._segments_embedded += 1 state = self._speakers.setdefault(speaker_id, _SpeakerState()) cluster = self._assign_cluster(state, embedding, duration_s) self._try_pin(cluster) @@ -658,24 +810,45 @@ def _distinct_voice_count(self) -> int: separate clusters. Across ids, streaming diarization may over-split one talker (S0 vs S1); centroids that clear ``cross_id_merge_sim`` collapse into a single voice for enrollment / suggestion gating. + + Two properties this count has to have, because auto-enrollment is gated + on it and a wrong answer writes a permanent voiceprint: + + *Deterministic.* Groups are seeded from the longest-established voice + down, so the answer does not depend on ``dict`` iteration order. + + *No chaining.* A centroid joins a group only if it clears the threshold + against **every** member, not just the nearest one. Single-linkage would + let A~B and B~C collapse A and C together even when they are plainly + different people, which is precisely the merge that must not happen. + + Clusters below ``MIN_VOICE_DURATION_S`` are ignored: too little audio to + assert a distinct person, and counting them turns one clipped segment + into a phantom speaker that blocks enrollment. """ - centroids: list[np.ndarray] = [] - for state in self._speakers.values(): - for cluster in state.clusters: - centroid = cluster.accumulator.centroid - if centroid is not None: - centroids.append(centroid) - if not centroids: - return 0 - representatives: list[np.ndarray] = [] - for centroid in centroids: - if any( - cosine_similarity(centroid, rep) >= self._cross_id_merge_sim - for rep in representatives - ): - continue - representatives.append(centroid) - return len(representatives) + established = sorted( + ( + (cluster.accumulator.centroid, cluster.accumulator.total_duration_s) + for state in self._speakers.values() + for cluster in state.clusters + if cluster.accumulator.centroid is not None + and cluster.accumulator.total_duration_s >= MIN_VOICE_DURATION_S + ), + key=lambda item: item[1], + reverse=True, + ) + groups: list[list[np.ndarray]] = [] + for centroid, _duration in established: + for group in groups: + if all( + cosine_similarity(centroid, member) >= self._cross_id_merge_sim + for member in group + ): + group.append(centroid) + break + else: + groups.append([centroid]) + return len(groups) def _single_voice_enrollment_audio( self, @@ -729,13 +902,28 @@ def _fire_enrollment( task.add_done_callback(self._pending_tasks.discard) async def _emit_enrollment(self, pcm: np.ndarray, sample_rate: int) -> None: - embedding = await self._embedder.embed(pcm, sample_rate) - wav_bytes = pcm_to_wav_bytes(pcm, sample_rate) - fd, wav_path = tempfile.mkstemp(prefix="voice_enroll_", suffix=".wav") - with os.fdopen(fd, "wb") as f: - f.write(wav_bytes) duration_s = len(pcm) / sample_rate - self._on_enrollment_captured(embedding, wav_path, duration_s) + try: + embedding = await self._embedder.embed(pcm, sample_rate) + wav_bytes = pcm_to_wav_bytes(pcm, sample_rate) + fd, wav_path = tempfile.mkstemp(prefix="voice_enroll_", suffix=".wav") + with os.fdopen(fd, "wb") as f: + f.write(wav_bytes) + self._on_enrollment_captured(embedding, wav_path, duration_s) + except Exception: + # Fire-and-forget like the segment path: log or the enrollment is + # lost with no trace, and the contact stays silently unenrolled. + _log.exception( + "voice enrollment capture failed for contact %s (%.0fs)", + self._call_contact_id, + duration_s, + ) + return + _log.info( + "voice enrollment captured for contact %s (%.0fs of speech)", + self._call_contact_id, + duration_s, + ) def _check_suggestion(self) -> None: distinct = self._distinct_voice_count() @@ -760,9 +948,67 @@ async def await_pending(self) -> None: if self._pending_tasks: await asyncio.gather(*list(self._pending_tasks), return_exceptions=True) + def diagnostics(self) -> dict: + """Per-call attribution counters, for the end-of-call summary log. + + Answers "did voice fingerprinting actually do anything on this call?" + without needing the transcript: no enrolled profiles, every segment + rejected as too short, or embeddings failing all present as zeros here. + """ + pins = sum( + 1 + for state in self._speakers.values() + for cluster in state.clusters + if cluster.pinned_contact_id is not None + ) + return { + "enrolled_profiles": len(self._enrolled), + "diarization_ids": len(self._speakers), + "clusters": sum(len(s.clusters) for s in self._speakers.values()), + "distinct_voices": self._distinct_voice_count(), + "pinned_clusters": pins, + "segments_observed": self._segments_observed, + "segments_buffered": self._segments_buffered, + "segments_dropped": self._segments_dropped, + "segments_embedded": self._segments_embedded, + "embed_failures": self._embed_failures, + "enrollment_fired": self._enrollment_fired, + "suggestion_fired": self._suggestion_fired, + } + + def _flush_pending_segments(self) -> None: + """Embed any buffered short finals that reached the duration floor. + + Anything still under it is dropped rather than embedded: a call ending + mid-backchannel must not contribute an unusable vector to the last + speaker's cluster. + """ + for speaker_id, state in self._speakers.items(): + if not state.pending_audio: + continue + pcm = np.concatenate(state.pending_audio) + duration_s = state.pending_duration_s + state.pending_audio = [] + state.pending_duration_s = 0.0 + if duration_s < SEGMENT_MIN_S: + self._segments_dropped += 1 + continue + task = asyncio.create_task( + self._process_segment( + speaker_id, + pcm, + ENROLLMENT_SAMPLE_RATE, + duration_s, + ), + ) + self._pending_tasks.add(task) + task.add_done_callback(self._pending_tasks.discard) + async def finalize(self) -> None: """Call-end hook: flush pending work and fire a partial enrollment.""" await self.await_pending() + self._flush_pending_segments() + await self.await_pending() if not self._enrollment_fired: material = self._single_voice_enrollment_audio() if material is not None: @@ -770,6 +1016,11 @@ async def finalize(self) -> None: if duration_s >= self._enrollment_min_s: self._fire_enrollment(audio_parts, sample_rate) await self.await_pending() + stats = self.diagnostics() + _log.info( + "speaker attribution summary: %s", + " ".join(f"{k}={v}" for k, v in stats.items()), + ) # ── resolution ─────────────────────────────────────────────────────── @@ -780,6 +1031,13 @@ def resolve(self, speaker_id: str | None) -> SpeakerResolution | None: processed segment joined — so when an id carries several co-located voices the answer names the specific one, not a blurred average. ``provisional`` is set whenever the id spans more than one cluster. + + A provisional pin is still returned — it is the best guess available + for routing, and dropping it would lose attribution entirely — but it + is **not** reported as verified. ``verified`` is a claim that this + utterance's voice was positively matched, and once an id is known to + carry more than one voice the tracker cannot make that claim about any + single utterance under it. """ if not speaker_id: return None @@ -791,7 +1049,7 @@ def resolve(self, speaker_id: str | None) -> SpeakerResolution | None: if cluster.pinned_contact_id is not None: return SpeakerResolution( contact_id=cluster.pinned_contact_id, - verified=True, + verified=not provisional, provisional=provisional, source=LABEL_SOURCE_VOICE_PIN, ) @@ -966,9 +1224,10 @@ def __init__( profiles_provider: Callable[[], tuple[list[np.ndarray], list[np.ndarray]]], window_s: float = REALTIME_WINDOW_S, hop_s: float = REALTIME_HOP_S, - match_threshold: float = SPEAKER_MATCH_THRESHOLD, + match_threshold: float = REALTIME_MATCH_THRESHOLD, hysteresis_s: float = NON_ENGAGED_HYSTERESIS_S, min_rms: float = REALTIME_MIN_RMS, + min_window_fraction: float = REALTIME_MIN_WINDOW_FRACTION, ) -> None: self._embedder = embedder self._profiles_provider = profiles_provider @@ -977,6 +1236,7 @@ def __init__( self._match_threshold = match_threshold self._hysteresis_s = hysteresis_s self._min_rms = min_rms + self._min_window_fraction = min_window_fraction self._window: deque[np.ndarray] = deque() self._window_duration_s = 0.0 @@ -986,6 +1246,11 @@ def __init__( self.verdict: str = "unknown" self._non_engaged_since: float | None = None + # Floor gating fails open, so a scorer that never produces a confident + # verdict is indistinguishable from one that is working — the counts + # are what tell the two apart. + self._verdicts: Counter[str] = Counter() + self._infer_failures = 0 def add_audio( self, @@ -1014,7 +1279,7 @@ def add_audio( if ( self._since_infer_s < self._hop_s or self._busy - or self._window_duration_s < self._window_s * 0.5 + or self._window_duration_s < self._window_s * self._min_window_fraction ): return self._since_infer_s = 0.0 @@ -1054,7 +1319,15 @@ async def _infer(self, pcm: np.ndarray) -> None: else: self.verdict = "unknown" self._non_engaged_since = None + except Exception: + self._infer_failures += 1 + self.verdict = "unknown" + self._non_engaged_since = None + # Logged once per failure rather than swallowed: a broken extractor + # here silently reverts the call to ungated behaviour. + _log.exception("realtime speaker scoring failed; floor gate open") finally: + self._verdicts[self.verdict] += 1 self._busy = False @property @@ -1064,3 +1337,25 @@ def confidently_non_engaged(self) -> bool: self._non_engaged_since is not None and (time.time() - self._non_engaged_since) >= self._hysteresis_s ) + + def diagnostics(self) -> dict: + """Verdict tally for the end-of-call summary. + + An all-``unknown`` tally means the gate never acted: every window was + silence, ambiguous, or below threshold. + """ + return { + "windows_scored": sum(self._verdicts.values()), + "engaged": self._verdicts["engaged"], + "non_engaged": self._verdicts["non_engaged"], + "unknown": self._verdicts["unknown"], + "infer_failures": self._infer_failures, + } + + def log_summary(self) -> None: + """Emit the end-of-call verdict tally (call-side lifecycle hook).""" + stats = self.diagnostics() + _log.info( + "speaker floor gate summary: %s", + " ".join(f"{k}={v}" for k, v in stats.items()), + ) diff --git a/unify/conversation_manager/utils.py b/unify/conversation_manager/utils.py index 669867d7c..d1591bfbc 100644 --- a/unify/conversation_manager/utils.py +++ b/unify/conversation_manager/utils.py @@ -6,69 +6,108 @@ from unify.settings import SETTINGS -# dispatch LiveKit agent -def dispatch_livekit_agent( - room_name: str, - *, - record: bool = True, - assistant_id: str = "", - user_id: str = "", - agent_name: str | None = None, - call_session_id: str = "", -): - """ - Dispatch a LiveKit agent via the communication service. +def _post_to_comms(path: str, payload: dict, *, label: str, timeout: float) -> bool: + """POST a fire-and-forget control message to the comms gateway. - By default ``room_name`` is used as both the LiveKit room name and the - agent worker registration name. Pass ``agent_name`` when they must differ - (org multi-assistant rooms share one room but register distinct workers). - - This is a fire-and-forget operation - we dispatch and move on regardless of - the result. The function is resilient to: - - Missing UNITY_COMMS_URL (common in local/test environments) - - Network timeouts (expected behavior) - - Connection errors (service unavailable) - - Returns True if dispatch was attempted, False if skipped due to missing config. + Resilient to a missing ``UNITY_COMMS_URL`` (common in local/test + environments), timeouts, and connection errors: none of these may take a + live call down. Returns True when the request was attempted and not + rejected, False when it was skipped or refused. """ unity_comms_url = SETTINGS.conversation.COMMS_URL if not unity_comms_url: LOGGER.debug( - f"{DEFAULT_ICON} [dispatch_livekit_agent] Skipping: UNITY_COMMS_URL not configured. " - "Set this to enable LiveKit agent dispatch.", + f"{DEFAULT_ICON} [{label}] Skipping: UNITY_COMMS_URL not configured.", ) return False - livekit_agent_name = agent_name or room_name try: response = requests.post( - f"{unity_comms_url}/phone/dispatch-livekit-agent", + f"{unity_comms_url}{path}", # Authenticate as this assistant; the gateway accepts either a # valid user API key or the platform admin key here. headers={"Authorization": f"Bearer {SESSION_DETAILS.unify_key}"}, - json={ - "livekit_agent_name": livekit_agent_name, - "room_name": room_name, - "record": record, - "assistant_id": assistant_id, - "user_id": user_id, - "call_session_id": call_session_id, - }, - timeout=1, + json=payload, + timeout=timeout, ) if response.status_code != 200: - LOGGER.error( - f"{DEFAULT_ICON} Failed to dispatch LiveKit agent. {response.text}", - ) + LOGGER.error(f"{DEFAULT_ICON} [{label}] Refused: {response.text}") return False - else: - LOGGER.debug(f"{DEFAULT_ICON} LiveKit agent dispatched") + LOGGER.debug(f"{DEFAULT_ICON} [{label}] Accepted") except requests.exceptions.Timeout: - LOGGER.debug(f"{DEFAULT_ICON} LiveKit agent dispatched (timeout)") + LOGGER.debug(f"{DEFAULT_ICON} [{label}] Sent (response timed out)") except requests.exceptions.RequestException as e: - # Connection errors, DNS failures, etc. - don't crash, just log - LOGGER.error( - f"{DEFAULT_ICON} [dispatch_livekit_agent] Request failed (non-fatal): {e}", - ) + LOGGER.error(f"{DEFAULT_ICON} [{label}] Request failed (non-fatal): {e}") return False return True + + +# dispatch LiveKit agent +def dispatch_livekit_agent( + room_name: str, + *, + agent_name: str | None = None, + call_session_id: str = "", +): + """ + Dispatch a LiveKit agent via the communication service. + + By default ``room_name`` is used as both the LiveKit room name and the + agent worker registration name. Pass ``agent_name`` when they must differ + (org multi-assistant rooms share one room but register distinct workers). + + Dispatch only: recording is started from the call-started path via + :func:`start_call_recording`, once the room has a publishing participant. + """ + return _post_to_comms( + "/phone/dispatch-livekit-agent", + { + "livekit_agent_name": agent_name or room_name, + "room_name": room_name, + "call_session_id": call_session_id, + }, + label="dispatch_livekit_agent", + timeout=1, + ) + + +def start_call_recording( + room_name: str, + assistant_id: str, + *, + user_id: str = "", + call_session_id: str = "", + provider_call_sid: str = "", + conference_name: str = "", +): + """Ask the gateway to record the live LiveKit room backing this session. + + Called once the session is up, so the room exists and carries audio. The + linkage IDs travel with the request and come back on the completion webhook, + which is how the finished file is matched to its transcript exchange. + + The gateway skips rooms it cannot capture and rooms already being recorded, + so calling this more than once for one session is harmless. + """ + if not room_name or not str(assistant_id).strip(): + LOGGER.debug( + f"{DEFAULT_ICON} [start_call_recording] Skipping: room_name and " + f"assistant_id are both required (room={room_name!r}, " + f"assistant_id={assistant_id!r}).", + ) + return False + return _post_to_comms( + "/phone/start-recording", + { + "room_name": room_name, + "assistant_id": str(assistant_id), + "user_id": str(user_id or ""), + "call_session_id": call_session_id, + "provider_call_sid": provider_call_sid, + "conference_name": conference_name, + }, + label="start_call_recording", + # Recording is off the critical path for call setup, but the gateway + # does a LiveKit round-trip here, so allow more than the dispatch hop. + timeout=3, + ) diff --git a/unify/events/persist_filters.py b/unify/events/persist_filters.py index 6db852008..abbd98252 100644 --- a/unify/events/persist_filters.py +++ b/unify/events/persist_filters.py @@ -17,7 +17,7 @@ from .types.tool_loop import ToolLoopKind _DEFAULT_ALLOWLIST_TOOLS: frozenset[str] = frozenset( - {"execute_code", "execute_function"}, + {"act", "execute_code", "execute_function"}, ) _TOOL_LOOP_MATCH_KINDS: frozenset[str] = frozenset( { diff --git a/unify/gateway/channels/phone/views.py b/unify/gateway/channels/phone/views.py index 8fc26cb37..714dc8e7f 100644 --- a/unify/gateway/channels/phone/views.py +++ b/unify/gateway/channels/phone/views.py @@ -21,10 +21,13 @@ external callers (Twilio webhooks, Unity admin clients) see no change. -The endpoint set matches the original 1:1:: +The ported endpoints preserve the original wire behaviour 1:1; the two +recording routes are additions that have no counterpart in the original:: auth_router: POST /dispatch-livekit-agent -- creates LiveKit room + dispatches agent + POST /start-recording -- starts egress on an already-live room + POST /recording-url -- signs playback for a stored recording POST /send-call -- creates outbound Twilio call -> SIP -> LiveKit POST /send-text -- sends SMS via Twilio GET /available-countries -- static list of supported countries @@ -37,16 +40,26 @@ unauth_router: POST /conference-status -- Twilio webhook for conference lifecycle events POST /twiml -- TwiML response for outbound call leg + +Recording lives here because the comms service is the only one holding a +service-account key for the recordings bucket: it starts the egress that +writes the object and signs the reads that play it back. Orchestra proxies +user-facing playback through ``/recording-url`` rather than being granted its +own cross-project access to that bucket. """ from __future__ import annotations import json import logging +import re +from datetime import timedelta from urllib.parse import quote_plus import httpx from fastapi import APIRouter, HTTPException, Request, Response +from google.cloud import storage +from google.oauth2.service_account import Credentials from livekit.api import ( CreateSIPInboundTrunkRequest, SIPInboundTrunkInfo, @@ -68,9 +81,10 @@ ensure_phone_dispatch_rule, get_livekit_api, make_sip_uri, + start_room_egress, ) from unify.gateway.common.twilio import build_twilio_client -from unify.gateway.credentials import EnvCredentialStore +from unify.gateway.credentials import CredentialStore, EnvCredentialStore from unify.settings import SETTINGS logger = logging.getLogger("unify.gateway.channels.phone") @@ -89,6 +103,89 @@ async def _json_object_or_empty(request: Request) -> dict: return payload +# --------------------------------------------------------------------------- +# Call-recording playback +# --------------------------------------------------------------------------- + +_GS_URI_RE = re.compile(r"^gs://([^/]+)/(.+)$") +# Recording objects are written as {deploy_env}/{assistant_id}/{room}_{ts}.mp3 +# by ``_start_room_egress``. The assistant segment is what authorises playback, +# so a path that does not carry one cannot be served. +_RECORDING_PATH_RE = re.compile(r"^(?P[^/]+)/(?P[^/]+)/[^/]+\.mp3$") + +RECORDING_URL_TTL = timedelta(hours=1) + + +def _recordings_bucket(credentials: CredentialStore) -> str: + return credentials.get_optional( + "LIVEKIT_EGRESS_GCS_BUCKET", + "unity-call-recordings", + ) + + +def _parse_recording_uri(gcs_uri: str, credentials: CredentialStore) -> tuple[str, str]: + """Split a ``gs://`` URI, refusing anything outside this env's recordings. + + Pinning both the bucket and the environment prefix keeps this endpoint from + being used as a general-purpose signer for the comms service account, which + can read considerably more than call recordings. + """ + match = _GS_URI_RE.match(gcs_uri) + if not match: + raise HTTPException(status_code=400, detail="gcs_uri must be a gs:// URI") + bucket_name, object_path = match.group(1), match.group(2) + + expected_bucket = _recordings_bucket(credentials) + if bucket_name != expected_bucket: + raise HTTPException( + status_code=403, + detail="Bucket is not the recordings bucket", + ) + + path_match = _RECORDING_PATH_RE.match(object_path) + if not path_match: + raise HTTPException(status_code=400, detail="Not a recording object path") + if path_match.group("env") != SETTINGS.DEPLOY_ENV: + raise HTTPException( + status_code=403, + detail="Recording belongs to a different environment", + ) + return bucket_name, object_path + + +def _assistant_id_from_recording_path(object_path: str) -> str: + match = _RECORDING_PATH_RE.match(object_path) + if not match: + raise HTTPException(status_code=400, detail="Not a recording object path") + return match.group("assistant_id") + + +def _sign_recording( + bucket_name: str, + object_path: str, + credentials: CredentialStore, +) -> dict: + """Sign a GET for one recording object, or 404 when it was never written.""" + creds_json_raw = credentials.get_optional("GCP_SA_KEY", "") + if not creds_json_raw: + raise HTTPException(status_code=500, detail="GCP_SA_KEY not configured") + creds = Credentials.from_service_account_info(json.loads(creds_json_raw)) + blob = storage.Client(credentials=creds).bucket(bucket_name).blob(object_path) + + if not blob.exists(): + raise HTTPException(status_code=404, detail="Recording not found") + + signed_url = blob.generate_signed_url( + version="v4", + expiration=RECORDING_URL_TTL, + method="GET", + ) + return { + "signed_url": signed_url, + "expires_in_minutes": int(RECORDING_URL_TTL.total_seconds() // 60), + } + + # --------------------------------------------------------------------------- # Helpers (module-local; promote to unify/gateway/common/ if reused) # --------------------------------------------------------------------------- @@ -171,36 +268,93 @@ def _create_conference_response(sip_uri: str) -> VoiceResponse: @auth_router.post("/dispatch-livekit-agent") async def dispatch_livekit_agent(request: Request): - """Create a LiveKit room, dispatch the agent, and start recording. - - The runtime dispatch always requests ``record`` so the call is captured to - GCS via an audio-only Room Composite Egress. ``assistant_id``/``user_id`` - (and, when known, the ``call_session_id``/``provider_call_sid``/ - ``conference_name`` linkage IDs) are threaded into the egress completion - webhook so the recording can be attributed back to the call. + """Create a LiveKit room and dispatch the agent into it. ``livekit_agent_name`` is honoured as the agent worker registration name (distinct from ``room_name`` for org multi-assistant meet rooms that share a single room); it falls back to ``room_name`` for single-assistant callers. + + Recording is not started here -- see ``/phone/start-recording``, which the + runtime calls once the session is live. """ credentials = EnvCredentialStore() data = await _json_object_or_empty(request) room_name = data.get("room_name") or data.get("livekit_agent_name", "") agent_name = data.get("livekit_agent_name") or room_name - await create_room_and_dispatch_agent( + await create_room_and_dispatch_agent(room_name, agent_name, credentials) + return {"success": True} + + +@auth_router.post("/start-recording") +async def start_recording(request: Request): + """Start the call recording for an already-live LiveKit room. + + Called from the runtime's call-started path, so the room is known to exist + and carry a publishing participant -- the precondition Room Composite + Egress needs. ``assistant_id`` is required (it owns the object prefix); + ``call_session_id``/``provider_call_sid``/``conference_name`` are threaded + into the completion webhook as linkage IDs so the finished recording can be + resolved back to its transcript exchange. + + Idempotent: a room already being recorded is left alone, so a retried or + duplicated call-started event cannot double-record the room. + """ + credentials = EnvCredentialStore() + data = await _json_object_or_empty(request) + room_name = data.get("room_name", "") + assistant_id = str(data.get("assistant_id", "") or "") + if not room_name or not assistant_id: + raise HTTPException( + status_code=400, + detail="room_name and assistant_id are required", + ) + # The assistant id selects the GCS prefix the recording lands under and the + # Pub/Sub topic the completion event is published to, so a user-key caller + # must own it. + await require_assistant_ownership(request, assistant_id) + await start_room_egress( room_name, - agent_name, + assistant_id, credentials, - record=bool(data.get("record", True)), - assistant_id=data.get("assistant_id", ""), - user_id=data.get("user_id", ""), - call_session_id=data.get("call_session_id", ""), - provider_call_sid=data.get("provider_call_sid", ""), - conference_name=data.get("conference_name", ""), + str(data.get("user_id", "") or ""), + call_session_id=str(data.get("call_session_id", "") or ""), + provider_call_sid=str(data.get("provider_call_sid", "") or ""), + conference_name=str(data.get("conference_name", "") or ""), ) return {"success": True} +@auth_router.post("/recording-url") +async def recording_url(request: Request): + """Mint a short-lived playback URL for a stored call recording. + + The recordings bucket lives in the comms project and this service already + holds a service-account key for it, so it is the only place that can both + read the object and sign for it. Orchestra proxies user-facing reads here + rather than being granted its own cross-project access. + + A signed URL is returned rather than the audio itself: players seek with + HTTP range requests, which GCS serves natively and a proxy would have to + reimplement while pushing every megabyte through this service. + + Responses are deliberately distinguishable -- 404 means the object is not + in the bucket (an egress that failed before the completion gate existed + still recorded a URL on its exchange), 403 means the caller may not hear + this assistant's calls. + """ + credentials = EnvCredentialStore() + data = await _json_object_or_empty(request) + gcs_uri = str(data.get("gcs_uri", "") or "").strip() + if not gcs_uri: + raise HTTPException(status_code=400, detail="gcs_uri is required") + + bucket_name, object_path = _parse_recording_uri(gcs_uri, credentials) + assistant_id = _assistant_id_from_recording_path(object_path) + await require_assistant_ownership(request, assistant_id) + + return _sign_recording(bucket_name, object_path, credentials) + + def _admin_headers() -> dict: """Bearer headers for Orchestra admin API calls.""" return { diff --git a/unify/gateway/common/livekit.py b/unify/gateway/common/livekit.py index f184f16d1..03f1657b8 100644 --- a/unify/gateway/common/livekit.py +++ b/unify/gateway/common/livekit.py @@ -7,10 +7,8 @@ changes (env reads -> ``CredentialStore.get``) so the channels stay decoupled from any deployment-specific config layer. -Scope today -=========== - -This module ships the four helpers Phase B.2 (``phone/``) needs: +Helpers +======= * ``get_livekit_api`` -- LiveKit ``LiveKitAPI`` client factory. * ``make_sip_uri`` -- builds the SIP URI used to bridge a Twilio @@ -20,14 +18,19 @@ correct ``unity_{id}_{medium}`` room. * ``create_room_and_dispatch_agent`` -- creates a LiveKit room and dispatches the LiveKit agent that owns the call session. - -The other helpers from the communication-side module -(``make_room_name``, ``start_room_egress``, ``verify_livekit_webhook``) -are also already present in -``unify.conversation_manager.local_providers.livekit`` for the -self-hosted single-process path. When the next channel migration -needs them outside that path, port them here and deprecate the -local_providers copy in a focused commit. +* ``start_room_egress`` -- starts the audio-only Room Composite + Egress that captures a live call to GCS. + +Recording is deliberately *not* wired into agent dispatch. Egress must +be started once the room has a publishing participant, so it is driven +from the call-started path (``/phone/start-recording``) rather than from +room creation. Starting it at dispatch time races the SIP bridge and the +agent join, and LiveKit fails such a job with "Start signal not +received" after producing no file at all. + +``unify.conversation_manager.local_providers.livekit`` carries a +parallel copy of the same helpers for the self-hosted single-process +path. """ from __future__ import annotations @@ -48,6 +51,7 @@ SIPDispatchRuleInfo, WebhookConfig, ) +from livekit.protocol.egress import ListEgressRequest from livekit.protocol.sip import ( DeleteSIPDispatchRuleRequest, ListSIPDispatchRuleRequest, @@ -61,6 +65,44 @@ _log = logging.getLogger("unify.gateway.common.livekit") +# Room suffixes whose audio never reaches the LiveKit room. Browser meets +# (Google Meet / Teams) bridge caller audio through the agent-service +# PortAudio device, so the LiveKit room carries no remote track for the +# compositor to mix -- see ``Assistant.stt_node`` in +# ``conversation_manager/medium_scripts/call.py``. Room Composite Egress on +# these rooms cannot produce a file: it stays alive for as long as the room +# does and then fails with "Start signal not received". Recording these +# channels needs a capture point on the bridge, not LiveKit egress. +UNRECORDABLE_ROOM_SUFFIXES = ("_gmeet", "_teams") + + +def room_supports_egress(room_name: str) -> bool: + """Whether a Room Composite Egress can ever capture audio for this room.""" + return not room_name.endswith(UNRECORDABLE_ROOM_SUFFIXES) + + +async def has_active_egress(livekit_api: LiveKitAPI, room_name: str) -> bool: + """Whether an egress job is already running for *room_name*. + + Several call paths can converge on one room (SIP bridge setup, agent + dispatch, a watchdog respawn). Without this check each one starts its own + Room Composite Egress, so the room is recorded twice into two separate + files and billed twice. Treated as "no active egress" when the lookup + itself fails, so a LiveKit hiccup cannot silently disable recording. + """ + try: + listed = await livekit_api.egress.list_egress( + ListEgressRequest(room_name=room_name, active=True), + ) + except Exception as exc: + _log.warning( + "could not list active egress for room %r, starting anyway: %s", + room_name, + exc, + ) + return False + return bool(listed.items) + def get_livekit_api(credentials: CredentialStore) -> LiveKitAPI: """Construct a LiveKit API client from configured credentials. @@ -302,7 +344,34 @@ async def _start_room_egress( event so the assistant runtime can attach it to the transcript exchange. The linkage query params are threaded through the webhook URL so the completion handler can resolve the correct session / exchange. + + No-ops when the room cannot be captured, when the caller has no assistant + to attribute the file to, or when a job is already recording the room. """ + if not room_supports_egress(room_name): + _log.info( + "skipping egress for room %r: channel audio does not reach the " + "LiveKit room", + room_name, + ) + return + if not str(assistant_id).strip(): + # The object path is {env}/{assistant_id}/{room}.mp3, so an empty id + # collapses the per-assistant prefix and strands the file where no + # lookup by assistant will find it. + _log.error( + "refusing egress for room %r: assistant_id is required to build " + "the recording path", + room_name, + ) + return + if await has_active_egress(livekit_api, room_name): + _log.info( + "skipping egress for room %r: a job is already recording it", + room_name, + ) + return + gcs_credentials = credentials.get_optional("GCP_SA_KEY", "") gcs_bucket = credentials.get_optional( "LIVEKIT_EGRESS_GCS_BUCKET", @@ -389,20 +458,12 @@ async def create_room_and_dispatch_agent( agent_name: str, credentials: CredentialStore, metadata: dict | None = None, - *, - record: bool = False, - assistant_id: str | int = "", - user_id: str | int = "", - call_session_id: str = "", - provider_call_sid: str = "", - conference_name: str = "", ) -> Any: """Create a LiveKit room and dispatch an agent into it. - When ``record`` is set, an audio-only Room Composite Egress is also started - for the room so the call is captured to GCS. Egress is best-effort: a - recording failure is logged but never fails the agent dispatch (which is - the critical path for the call to connect). + Dispatch only. Recording is started separately once the room is live (see + ``start_room_egress``); binding it to dispatch starts the compositor before + any participant publishes, which LiveKit fails with no file produced. Returns the LiveKit dispatch object. Re-raises dispatch failures after logging so the caller's error handler sees the exception. @@ -423,24 +484,6 @@ async def create_room_and_dispatch_agent( agent_name, getattr(dispatch, "id", "?"), ) - if record: - try: - await _start_room_egress( - livekit_api, - room_name, - str(assistant_id), - str(user_id), - credentials, - call_session_id=call_session_id, - provider_call_sid=provider_call_sid, - conference_name=conference_name, - ) - except Exception as exc: - _log.error( - "agent dispatched but failed to start egress for room %r: %s", - room_name, - exc, - ) return dispatch except Exception as exc: _log.error( @@ -454,12 +497,15 @@ async def create_room_and_dispatch_agent( __all__ = [ + "UNRECORDABLE_ROOM_SUFFIXES", "create_room_and_dispatch_agent", "delete_sip_dispatch_rule", "ensure_call_scoped_dispatch_rule", "ensure_phone_dispatch_rule", "get_livekit_api", + "has_active_egress", "make_call_scoped_sip_uri", "make_sip_uri", + "room_supports_egress", "start_room_egress", ] diff --git a/unify/settings.py b/unify/settings.py index 4ea1f458f..c600a7e23 100644 --- a/unify/settings.py +++ b/unify/settings.py @@ -144,8 +144,8 @@ class ProductionSettings(BaseSettings): # Comma-separated tool/method names when EVENTBUS_ORCHESTRA_PERSIST_MODE # is ``allowlist`` and the event is **not** under a execution lineage - # (default: CodeAct execution boundaries + tool results). - EVENTBUS_ORCHESTRA_PERSIST_TOOLS: str = "execute_code,execute_function" + # (default: CodeAct action boundary + execution boundaries + tool results). + EVENTBUS_ORCHESTRA_PERSIST_TOOLS: str = "act,execute_code,execute_function" # ───────────────────────────────────────────────────────────────────────── # EventBus Pub/Sub Streaming (Live Actions) diff --git a/unify/task_scheduler/active_task.py b/unify/task_scheduler/active_task.py index b22f4cf22..96ba20d24 100644 --- a/unify/task_scheduler/active_task.py +++ b/unify/task_scheduler/active_task.py @@ -10,6 +10,7 @@ import functools import asyncio import textwrap +from contextlib import nullcontext from datetime import datetime, timezone from typing import Optional, Dict, TYPE_CHECKING, List, Any @@ -23,8 +24,7 @@ ) from unify.common._async_tool.messages import forward_handle_call from unify.events.task_run_lineage import ( - push_task_run_lineage, - reset_task_run_lineage, + task_run_lineage_scope, ) from .machine_state import ( TaskRunProvenance, @@ -166,7 +166,6 @@ def __init__( self._was_stopped: bool = False self._last_intent: Optional[str] = None self._last_intent_reason: Optional[str] = None - self._task_run_lineage_tokens = None self._definition_rearmed = False self._preserve_definition_status = False @@ -207,7 +206,7 @@ async def create( """ delegate = current_task_execution_delegate.get() review_token = None - lineage_tokens = None + run_key: str | None = None # Materialize/adopt the durable Tasks/Executions row before EventBus lineage # so every nested event can carry the join key ``run_key``. materialized_task_run_reference = task_run_reference @@ -244,10 +243,6 @@ async def create( "Tasks/Executions (task_id=%s)", task_id, ) - lineage_tokens = push_task_run_lineage( - task_id=int(task_id), - run_key=run_key, - ) if task_entrypoint_review is not None: review_token = current_post_run_review_context.set( PostRunReviewContext( @@ -262,37 +257,43 @@ async def create( ) try: try: - if delegate is not None: - actor_steerable_handle = await delegate.start_task_run( - task_description=task_description, - entrypoint=entrypoint, - parent_chat_context=_parent_chat_context, - clarification_up_q=_clarification_up_q, - clarification_down_q=_clarification_down_q, - guidelines=task_guidelines, - entrypoint_kwargs=entrypoint_kwargs, - entrypoint_repair_attempts=entrypoint_repair_attempts, - entrypoint_repair_context=entrypoint_repair_context, - destination=destination, - ) - else: - if fallback_actor is None: - raise RuntimeError( - "Task execution requires an actor when no run-scoped delegate is active.", + lineage_scope = ( + task_run_lineage_scope(task_id=int(task_id), run_key=run_key) + if task_id is not None and instance_id is not None + else nullcontext() + ) + with lineage_scope: + if delegate is not None: + actor_steerable_handle = await delegate.start_task_run( + task_description=task_description, + entrypoint=entrypoint, + parent_chat_context=_parent_chat_context, + clarification_up_q=_clarification_up_q, + clarification_down_q=_clarification_down_q, + guidelines=task_guidelines, + entrypoint_kwargs=entrypoint_kwargs, + entrypoint_repair_attempts=entrypoint_repair_attempts, + entrypoint_repair_context=entrypoint_repair_context, + destination=destination, + ) + else: + if fallback_actor is None: + raise RuntimeError( + "Task execution requires an actor when no run-scoped delegate is active.", + ) + actor_steerable_handle = await fallback_actor.act( + task_description, + guidelines=task_guidelines, + _parent_chat_context=_parent_chat_context, + _clarification_up_q=_clarification_up_q, + _clarification_down_q=_clarification_down_q, + entrypoint=entrypoint, + entrypoint_kwargs=entrypoint_kwargs, + entrypoint_repair_attempts=entrypoint_repair_attempts, + entrypoint_repair_context=entrypoint_repair_context, + destination=destination, + persist=False, ) - actor_steerable_handle = await fallback_actor.act( - task_description, - guidelines=task_guidelines, - _parent_chat_context=_parent_chat_context, - _clarification_up_q=_clarification_up_q, - _clarification_down_q=_clarification_down_q, - entrypoint=entrypoint, - entrypoint_kwargs=entrypoint_kwargs, - entrypoint_repair_attempts=entrypoint_repair_attempts, - entrypoint_repair_context=entrypoint_repair_context, - destination=destination, - persist=False, - ) except Exception as exc: if materialized_task_run_reference is not None: await asyncio.to_thread( @@ -307,8 +308,6 @@ async def create( ), }, ) - reset_task_run_lineage(lineage_tokens) - lineage_tokens = None raise finally: if review_token is not None: @@ -320,7 +319,6 @@ async def create( scheduler=scheduler, task_run_reference=materialized_task_run_reference, ) - instance._task_run_lineage_tokens = lineage_tokens instance._definition_rearmed = bool(definition_rearmed) instance._preserve_definition_status = bool(preserve_definition_status) return instance @@ -376,8 +374,6 @@ async def interject( result_summary=f"Task cancelled: {stop_reason}", ), ) - reset_task_run_lineage(self._task_run_lineage_tokens) - self._task_run_lineage_tokens = None return await self._actor_handle.interject(message) # type: ignore[arg-type] @@ -412,8 +408,6 @@ async def stop( ), ) asyncio.create_task(self._save_final_summary("cancelled")) - reset_task_run_lineage(self._task_run_lineage_tokens) - self._task_run_lineage_tokens = None @functools.wraps(BaseActiveTask.pause, updated=()) async def pause(self) -> Optional[str]: @@ -533,51 +527,58 @@ async def result(self) -> str: finally: try: - if ( - final_status - and not self._was_stopped - and self._scheduler - and self._task_id is not None - and not self._preserve_definition_status - ): - definition_status = final_status - if self._definition_rearmed: - # Rearm-on-start already advanced the definition to the - # next open slot; restore scheduled/triggerable status - # regardless of the run outcome. A failed occurrence - # belongs to the run row — it must never terminalize - # the recurring definition and disarm the schedule. - task = self._scheduler._get_task_or_raise(self._task_id) - definition_status = ( - "triggerable" - if task.trigger is not None and task.repeat is None - else "scheduled" - ) - self._scheduler._update_task_definition_status( # type: ignore[attr-defined] - task_id=self._task_id, - new_status=definition_status, - ) + if final_status and not self._was_stopped: await self._persist_task_run_terminal_state( state=final_status, result_summary=ret, error=str(error) if error is not None else None, ) - if not getattr(self, "_summary_scheduled", False): - try: - logger.info( - "--- Scheduling save_final_summary for %s.%s with status: %s ---", - self._task_id, - self._instance_id, - final_status, + if ( + self._scheduler + and self._task_id is not None + and not self._preserve_definition_status + ): + definition_status = final_status + if self._definition_rearmed: + # Rearm-on-start already advanced the definition to the + # next open slot; restore scheduled/triggerable status + # regardless of the run outcome. A failed occurrence + # belongs to the run row — it must never terminalize + # the recurring definition and disarm the schedule. + task = self._scheduler._get_task_or_raise(self._task_id) + definition_status = ( + "triggerable" + if task.trigger is not None and task.repeat is None + else "scheduled" ) - asyncio.create_task(self._save_final_summary(final_status)) - self._summary_scheduled = True # type: ignore[attr-defined] - except Exception as summary_e: - logger.error("Error creating summary task: %s", summary_e) - finally: - reset_task_run_lineage(self._task_run_lineage_tokens) - self._task_run_lineage_tokens = None + self._scheduler._update_task_definition_status( # type: ignore[attr-defined] + task_id=self._task_id, + new_status=definition_status, + ) + + if not getattr(self, "_summary_scheduled", False): + try: + logger.info( + "--- Scheduling save_final_summary for %s.%s with status: %s ---", + self._task_id, + self._instance_id, + final_status, + ) + asyncio.create_task( + self._save_final_summary(final_status) + ) + self._summary_scheduled = True # type: ignore[attr-defined] + except Exception as summary_e: + logger.error( + "Error creating summary task: %s", summary_e + ) + except Exception: + logger.exception( + "Task completion maintenance failed (task_id=%s, instance_id=%s)", + self._task_id, + self._instance_id, + ) if error and final_status == "failed": raise error diff --git a/unify/task_scheduler/prompt_builders.py b/unify/task_scheduler/prompt_builders.py index c99fb6863..fedfcfa8b 100644 --- a/unify/task_scheduler/prompt_builders.py +++ b/unify/task_scheduler/prompt_builders.py @@ -239,7 +239,8 @@ def build_ask_prompt( "The catalog and connection list are connection-gated: they only show apps with an active connection on this assistant.", "If the user asks about an app with no eligible connection or no triggers listed, say that clearly, guide them to connect the integration first, then re-check — do not claim the provider lacks that trigger globally.", "When config_schema requires a resource, list resources and copy a selectable item's `trigger_config` fields; do not invent provider ids.", - "Rows with live_ready=false, provisionable=false, or delivery_only=true cannot be enabled yet — say so clearly.", + "Only explicit live_ready=false, provisionable=false, or delivery_only=true blocks creation or enablement. " + "null means the catalog has no native lifecycle gate for that field, so it is not a blocker; still complete the normal connection, schema/resource, provisioning, and health checks.", "Request full source_body only when the user explicitly asks to inspect raw event data.", ], ) @@ -445,9 +446,11 @@ def build_update_prompt( "Use provider-event triggers for third-party SaaS events configured in the trigger catalog.", "Authoring order: list catalog → list eligible connections → describe schema → " "resolve required resources → create with trigger_config filled → enable.", - "Stop before create/enable when the catalog row is not `live_ready`, " + "Stop before create/enable only when the catalog row explicitly has `live_ready=false`, " "`provisionable=false`, or `delivery_only=true` (for example Chat batch rows). " - "Tell the user that trigger is not available yet; do not invent a workaround.", + "`null` means there is no native lifecycle gate for that field, not that the trigger is unavailable; " + "still complete the normal connection, schema/resource, provisioning, and health checks. " + "Tell the user when an explicitly blocked trigger is unavailable; do not invent a workaround.", ( f"Use `{ask_fname}` for discovery tools (catalog, connections, schema, " f"and resource listing) before creating the task." diff --git a/unify/task_scheduler/provider_event_dispatch.py b/unify/task_scheduler/provider_event_dispatch.py index 238a4bc1c..47bed6290 100644 --- a/unify/task_scheduler/provider_event_dispatch.py +++ b/unify/task_scheduler/provider_event_dispatch.py @@ -74,6 +74,7 @@ class LiveProviderEventDispatchOutcome: adopted_only: bool launch_identity: str | None = None terminal_reason: str | None = None + description: str | None = None def validate_provider_event_dispatch_request( diff --git a/unify/task_scheduler/provider_event_execution.py b/unify/task_scheduler/provider_event_execution.py index 8eff90eb8..0bff2adb2 100644 --- a/unify/task_scheduler/provider_event_execution.py +++ b/unify/task_scheduler/provider_event_execution.py @@ -8,6 +8,7 @@ from __future__ import annotations import asyncio +import dataclasses from typing import TYPE_CHECKING from unify.session_details import SESSION_DETAILS @@ -45,6 +46,14 @@ def resolve_captured_task_revision(*, task_id: int) -> int: return int(revision) +def _resolve_task_description(*, task_id: int) -> str | None: + """Return the authored task description for one live provider-event dispatch.""" + + scheduler = TaskScheduler() + task = scheduler._get_provider_event_definition(task_id=task_id) + return task.description + + async def handle_provider_event_live_dispatch( request: ProviderEventDispatchRequest, ) -> tuple[LiveProviderEventDispatchOutcome, SteerableToolHandle | None]: @@ -63,8 +72,9 @@ async def handle_provider_event_live_dispatch( if session_assistant and session_assistant != str(request.assistant_id): raise ProviderEventDispatchValidationError("assistant_id_mismatch") - captured_task_revision, event_context = await asyncio.gather( + captured_task_revision, task_description, event_context = await asyncio.gather( asyncio.to_thread(resolve_captured_task_revision, task_id=request.task_id), + asyncio.to_thread(_resolve_task_description, task_id=request.task_id), asyncio.to_thread(fetch_provider_event_context, request), ) await asyncio.to_thread(verify_precreated_provider_event_run, request) @@ -88,6 +98,7 @@ async def handle_provider_event_live_dispatch( adopted_only=True, launch_identity=claimed.launch_identity or launch_identity, terminal_reason=claimed.terminal_reason, + description=task_description, ), None, ) @@ -119,4 +130,5 @@ async def handle_provider_event_live_dispatch( launch_identity=launch_identity, captured_task_revision=captured_task_revision, ) + outcome = dataclasses.replace(outcome, description=task_description) return outcome, handle diff --git a/unify/task_scheduler/task_scheduler.py b/unify/task_scheduler/task_scheduler.py index dce0b8cf4..4121ebb8d 100644 --- a/unify/task_scheduler/task_scheduler.py +++ b/unify/task_scheduler/task_scheduler.py @@ -10,7 +10,7 @@ import random import threading from contextlib import contextmanager -from datetime import datetime, timedelta +from datetime import datetime, timedelta, timezone from typing import ( Any, Callable, @@ -159,6 +159,12 @@ def _missing_certification_value(value: Any) -> bool: return value in (None, "", [], {}) +def _now_iso() -> str: + """Return the current UTC timestamp in ISO-8601 format.""" + + return datetime.now(timezone.utc).isoformat() + + _UNSET = _UnsetSentinel() @@ -835,14 +841,18 @@ def _get_task_for_source_log_id( @staticmethod def _normalize_activation_datetime(value: Any) -> str | None: - """Normalize scheduler timestamps into comparable ISO strings.""" + """Normalize scheduler timestamps into comparable UTC ISO strings.""" if value is None: return None + text = str(value) try: - return datetime.fromisoformat(str(value).replace("Z", "+00:00")).isoformat() + parsed = datetime.fromisoformat(text.replace("Z", "+00:00")) except ValueError: - return str(value) + return text + if parsed.tzinfo is None: + parsed = parsed.replace(tzinfo=timezone.utc) + return parsed.astimezone(timezone.utc).isoformat() def _validate_task_matches_provenance( self, @@ -1293,6 +1303,7 @@ async def start_provider_event_instance( "source_task_log_id": int(source_task_log_id), "revision": request.accepted_revision, "captured_task_revision": captured_task_revision, + "started_at": _now_iso(), }, ) @@ -1919,8 +1930,6 @@ def _delete_task( ) except TaskRevisionConflictError as exc: return task_revision_conflict_outcome(exc) - if log_ids: - self._store.delete(logs=log_ids) else: self._store.delete(logs=log_ids) removed_count = len(log_ids) @@ -2455,7 +2464,13 @@ def _write_log_entries( entries=entries, ) - def _list_provider_trigger_catalog(self) -> ToolOutcome: + def _list_provider_trigger_catalog( + self, + *, + canonical_app_slug: str | None = None, + limit: int | None = None, + offset: int | None = None, + ) -> ToolOutcome: """List staged provider triggers visible for this assistant's connected apps. Returns catalog metadata plus trigger slugs/config schemas for apps the @@ -2463,9 +2478,18 @@ def _list_provider_trigger_catalog(self) -> ToolOutcome: trigger list usually means no matching connection yet, not that the provider lacks the trigger globally. Prefer connecting the app first, then re-list the catalog before enabling a provider-event task. + + The unfiltered catalog can be large. Once + ``list_provider_trigger_connections`` shows which app/backend is + connected, pass that app's ``canonical_app_slug`` here to narrow the + response, and use ``limit``/``offset`` to page through the rest. """ - catalog = typed_tasks_client.get_trigger_catalog() + catalog = typed_tasks_client.get_trigger_catalog( + canonical_app_slug=canonical_app_slug, + limit=limit, + offset=offset, + ) return { "outcome": "provider trigger catalog listed", "details": annotate_provider_trigger_catalog( diff --git a/unify/task_scheduler/typed_tasks_client.py b/unify/task_scheduler/typed_tasks_client.py index d38f61a6d..fb5703725 100644 --- a/unify/task_scheduler/typed_tasks_client.py +++ b/unify/task_scheduler/typed_tasks_client.py @@ -242,7 +242,12 @@ def get_trigger_health(*, task_id: int) -> dict[str, Any]: return _info(response) -def get_trigger_catalog() -> dict[str, Any]: +def get_trigger_catalog( + *, + canonical_app_slug: str | None = None, + limit: int | None = None, + offset: int | None = None, +) -> dict[str, Any]: """List staged provider triggers for this assistant's connected apps.""" from unify.session_details import SESSION_DETAILS @@ -250,9 +255,19 @@ def get_trigger_catalog() -> dict[str, Any]: agent_id = SESSION_DETAILS.assistant.agent_id if agent_id is None: raise ValueError("assistant agent_id is required to list provider triggers") + params = { + key: value + for key, value in ( + ("canonical_app_slug", canonical_app_slug), + ("limit", limit), + ("offset", offset), + ) + if value is not None + } response = _request( "get", f"/assistants/{int(agent_id)}/provider-triggers", + params=params, ) return _info(response) diff --git a/unify/transcript_manager/base.py b/unify/transcript_manager/base.py index 2f856673d..1ea8503c0 100644 --- a/unify/transcript_manager/base.py +++ b/unify/transcript_manager/base.py @@ -229,6 +229,14 @@ def resolve_message_id_by_provider_sid( """Look up a transcript message_id by provider_message_sid metadata.""" raise NotImplementedError + def resolve_exchange_id_by_metadata( + self, + key: str, + value: str, + ) -> int | None: + """Return the exchange whose ``metadata[key]`` equals ``value``, if any.""" + raise NotImplementedError + def update_exchange_metadata( self, exchange_id: int, @@ -237,7 +245,10 @@ def update_exchange_metadata( destination: Optional[str] = None, ) -> Exchange: """ - Update (or create) metadata for the specified exchange and return the updated Exchange. + Merge ``metadata`` into the specified exchange and return the updated Exchange. + + Keys absent from ``metadata`` retain their stored values; the exchange + accumulates metadata from independent writers across a session. """ raise NotImplementedError diff --git a/unify/transcript_manager/simulated.py b/unify/transcript_manager/simulated.py index 403b0869a..734683824 100644 --- a/unify/transcript_manager/simulated.py +++ b/unify/transcript_manager/simulated.py @@ -786,13 +786,27 @@ def get_exchange_metadata(self, exchange_id: int) -> Exchange: ) return ex + def resolve_exchange_id_by_metadata( + self, + key: str, + value: str, + ) -> int | None: + """Simulated metadata-keyed exchange lookup over the in-memory store.""" + needle = str(value or "").strip() + if not needle: + return None + for exchange_id, exchange in self._sim_exchanges.items(): + if str((exchange.metadata or {}).get(key, "")) == needle: + return int(exchange_id) + return None + def update_exchange_metadata( self, exchange_id: int, metadata: Dict[str, Any], ) -> Exchange: """ - Simulated upsert of exchange metadata in the in-memory store. + Simulated merge of exchange metadata into the in-memory store. """ sched = maybe_tool_log_scheduled( "SimulatedTranscriptManager.update_exchange_metadata", @@ -810,9 +824,11 @@ def update_exchange_metadata( medium="", ) else: + merged = dict(cur.metadata or {}) + merged.update(dict(metadata or {})) cur = Exchange( exchange_id=cur.exchange_id, - metadata=dict(metadata or {}), + metadata=merged, medium=cur.medium, ) self._sim_exchanges[int(exchange_id)] = cur diff --git a/unify/transcript_manager/transcript_manager.py b/unify/transcript_manager/transcript_manager.py index da5548717..0ed1a82a8 100644 --- a/unify/transcript_manager/transcript_manager.py +++ b/unify/transcript_manager/transcript_manager.py @@ -1529,6 +1529,37 @@ def get_exchange_metadata( f"Failed to reconstruct Exchange for exchange_id={exchange_id}.", ) from exc + def resolve_exchange_id_by_metadata( + self, + key: str, + value: str, + ) -> int | None: + """Look up an exchange by one of its metadata identifiers. + + Server-side match on ``metadata.{key}`` across every readable Exchanges + root. Recovers the exchange for an asynchronous session artifact (e.g. a + recording that lands after the process that ran the call has gone) where + no in-process mapping survives. + """ + needle = str(value or "").strip() + if not needle: + return None + escaped = needle.replace("\\", "\\\\").replace('"', '\\"') + for context in self._read_exchange_contexts(): + rows = unisdk.get_logs( + context=context, + filter=f'metadata.{key} == "{escaped}"', + limit=1, + ) + if not rows: + continue + entries = getattr(rows[0], "entries", {}) or {} + try: + return int(entries.get("exchange_id")) + except (TypeError, ValueError): + return None + return None + def update_exchange_metadata( self, exchange_id: int, @@ -1536,22 +1567,32 @@ def update_exchange_metadata( *, destination: str | None = None, ) -> Exchange: - """Update or create exchange metadata in one routed root.""" + """Merge keys into an exchange's metadata in one routed root. + + Merges rather than replaces: exchange metadata accumulates across a + session's lifetime from independent writers. A call stores its session + identifiers when it ends and the recording URL arrives minutes later on + a separate event, so a replacing write would drop whichever keys it did + not carry -- including the identifiers needed to resolve the exchange in + the first place. Keys present in ``metadata`` win over stored values. + """ try: context = self._exchanges_context_for_destination(destination) except ToolErrorException as exc: return exc.payload # type: ignore[return-value] # Try update first - row_ids = unisdk.get_logs( + rows = unisdk.get_logs( context=context, filter=f"exchange_id == {int(exchange_id)}", - return_ids_only=True, + limit=1, ) - if row_ids: + if rows: + merged = dict(rows[0].entries.get("metadata") or {}) + merged.update(dict(metadata or {})) unisdk.update_logs( - logs=row_ids, + logs=rows[0].id, context=context, - entries={"metadata": dict(metadata or {})}, + entries={"metadata": merged}, overwrite=True, ) else: