From 42aeb1571cc1f1ed4d73f05b420474be970021ab Mon Sep 17 00:00:00 2001 From: Salman Mohammed Date: Tue, 1 Sep 2026 17:41:09 -0400 Subject: [PATCH 1/5] feat(desktop): add Pi agent preset (#7208) ## Summary Add Pi as a selectable desktop ACP runtime via `pi-acp`, including discovery guidance and its official logo. ### Related issue Follow-up to closed #6546. ### Testing - Focused Tauri preset and desktop logo tests - TypeScript typecheck Generated with Codex --------- Signed-off-by: Salman Mohammed --- crates/buzz-acp/README.md | 9 ++-- desktop/public/harness-logos/CREDITS.md | 1 + desktop/public/harness-logos/pi.svg | 21 ++++++++ .../src/managed_agents/discovery/presets.rs | 51 +++++++++++++++++++ .../features/onboarding/ui/RuntimeIcon.tsx | 1 + .../settings/ui/harnessCatalogCopy.test.mjs | 11 ++++ .../settings/ui/harnessCatalogCopy.ts | 2 + 7 files changed, 91 insertions(+), 5 deletions(-) create mode 100644 desktop/public/harness-logos/pi.svg create mode 100644 desktop/src/features/settings/ui/harnessCatalogCopy.test.mjs diff --git a/crates/buzz-acp/README.md b/crates/buzz-acp/README.md index cf36111a93..41d9a214bd 100644 --- a/crates/buzz-acp/README.md +++ b/crates/buzz-acp/README.md @@ -283,7 +283,7 @@ Buzz Desktop supports registering any ACP-speaking agent tool as a selectable ru **Tier-1 — compiled-in runtimes** (Goose, Claude Code, Codex, Buzz Agent): have auto-installers, auth probes, and first-class onboarding. Their IDs (`goose`, `claude`, `codex`, `buzz-agent`) are reserved and cannot be overridden. -**Tier-2 — preset catalog** (Cursor, Oh My Pi, Grok Build, OpenCode, Kimi Code, Amp, Hermes Agent, OpenClaw): static `HarnessDefinition` entries in `desktop/src-tauri/src/managed_agents/discovery.rs` (`PRESET_HARNESSES`). They are always present in the runtime catalog, PATH-probed for availability, not editable or deletable by the user. Displayed with bundled logos; if not installed, a docs link appears instead. +**Tier-2 — preset catalog** (Cursor, Oh My Pi, Pi, Grok Build, OpenCode, Kimi Code, Amp, Hermes Agent, OpenClaw): static `HarnessDefinition` entries in `desktop/src-tauri/src/managed_agents/discovery/presets.rs` (`PRESET_HARNESSES`). They are always present in the runtime catalog, PATH-probed for availability, not editable or deletable by the user. Displayed with bundled logos; if not installed, a docs link appears instead. > **Note — OpenClaw:** `openclaw acp` is a Gateway-backed bridge; PATH availability shows "Available" even when the OpenClaw Gateway daemon is not running. This is expected tier-2 semantics (same class as a preset with unconfigured auth). The Gateway URL is configured via `OPENCLAW_GATEWAY_URL` (or the equivalent env var from OpenClaw's docs) — set it in the agent's **env vars** in Edit Agent, not in the definition env (the preset definition carries no env entries). Note that `openclaw acp` executes tools inside the Gateway daemon, not the Desktop process, so Desktop-injected `BUZZ_*` env vars do NOT reach the execution locus unless you also set them on the Gateway's own environment. @@ -327,10 +327,9 @@ Invalid files (bad JSON, unknown id, empty command) are skipped with a warning a To add a new runtime to the tier-2 gallery: 1. **Verify the ACP entrypoint** from the vendor's own documentation — do not rely on a PR description alone. Test with the actual binary. -2. **Add a `HarnessDefinition` entry** to the `PRESET_HARNESSES` slice in `desktop/src-tauri/src/managed_agents/discovery.rs`. Fill `id`, `label`, `command`, `args`, `install_instructions_url`, `install_hint`. Leave `env` empty unless the harness requires a specific env var to enable ACP mode. -3. **Add the preset id to `BUILTIN_IDS`** in `desktop/src-tauri/src/managed_agents/custom_harnesses.rs` so custom JSON files cannot shadow it. -4. **Add a bundled logo** (64×64 PNG or optimised SVG) to `desktop/public/harness-logos/.png` and add a corresponding entry to `PRESET_LOGOS` in `desktop/src/features/onboarding/ui/RuntimeIcon.tsx`. Record the source and license in `desktop/public/harness-logos/CREDITS.md`. Only bundle a mark whose upstream license permits redistribution; skipping this step is caught by `presetLogos.test.mjs`, which asserts every `PRESET_HARNESSES` id has a mapped logo that exists on disk. -5. Run `cargo test --lib` and `just desktop-typecheck` to verify everything compiles. +2. **Add a `PresetHarness` entry** to the `PRESET_HARNESSES` slice in `desktop/src-tauri/src/managed_agents/discovery/presets.rs`. Fill `id`, `label`, `command`, `args`, `install_instructions_url`, `install_hint`, and `underlying_cli` when the command wraps a separately installed CLI. Preset ids are automatically reserved so custom JSON files cannot shadow them. +3. **Add a bundled logo** (64×64 PNG or optimised SVG) to `desktop/public/harness-logos/.png` and add a corresponding entry to `PRESET_LOGOS` in `desktop/src/features/onboarding/ui/RuntimeIcon.tsx`. Record the source and license in `desktop/public/harness-logos/CREDITS.md`. Only bundle a mark whose upstream license permits redistribution; skipping this step is caught by `presetLogos.test.mjs`, which asserts every `PRESET_HARNESSES` id has a mapped logo that exists on disk. +4. Run `cargo test --lib` and `just desktop-typecheck` to verify everything compiles. The built-in `BUILTIN_IDS` set (`goose`, `claude`, `codex`, `buzz-agent`, and all current preset ids) is the reserved namespace; every other id is available for custom harnesses. diff --git a/desktop/public/harness-logos/CREDITS.md b/desktop/public/harness-logos/CREDITS.md index 716c43e1ae..dee5aa257e 100644 --- a/desktop/public/harness-logos/CREDITS.md +++ b/desktop/public/harness-logos/CREDITS.md @@ -13,6 +13,7 @@ license permits redistribution. | `hermes.png` | [NousResearch/hermes-agent](https://github.com/NousResearch/hermes-agent) | `6ad632b` | MIT © 2025 Nous Research | `website/static/img/logo.png` | Cropped the baked-in border frame, padded to square, resized to 64×64, quantised to a 16-colour palette | | `openclaw.svg` | [openclaw/openclaw](https://github.com/openclaw/openclaw) | `b06f40a` | MIT © 2026 OpenClaw Foundation | `ui/public/favicon.svg` | Removed the SMIL animation elements (renders the upstream rest pose statically — verified pixel-identical to the upstream frame at t=0); minified paths | | `omp.svg` | [can1357/oh-my-pi](https://github.com/can1357/oh-my-pi) | `667111575ebba136dadfd6989379e7f67e0d40d9` | MIT © 2025 Mario Zechner; © 2025–2026 Can Bölük | `assets/icon.svg` | None | +| `pi.svg` | [earendil-works/pi-website](https://github.com/earendil-works/pi-website) | `2f5e410b97474d0a34ec2500aa1aa58d6c3f992c` | MIT © 2026 Earendil Inc. and contributors | `src/favicon.svg` | None | | `kimi.png` | [MoonshotAI/kimi-cli](https://github.com/MoonshotAI/kimi-cli) | `4a550effdfcb29a25a5d325bf935296cc50cd417` | Apache-2.0; NOTICE: Kimi Code CLI © 2025 Moonshot AI | `web/public/logo.png` | None | | `grok.svg` | [SpaceXAI brand guidelines](https://x.ai/legal/brand-guidelines) | Retrieved 2026-07-25 | xAI Brand Guidelines: marks may be used to accurately refer to xAI or its services; logos must be used exactly as provided | `SpaceXAI_Grok_Assets.zip` → `Grok_Logomark_Dark.svg` | None | diff --git a/desktop/public/harness-logos/pi.svg b/desktop/public/harness-logos/pi.svg new file mode 100644 index 0000000000..c28d624233 --- /dev/null +++ b/desktop/public/harness-logos/pi.svg @@ -0,0 +1,21 @@ + + + + + + diff --git a/desktop/src-tauri/src/managed_agents/discovery/presets.rs b/desktop/src-tauri/src/managed_agents/discovery/presets.rs index fd85309451..61c589770c 100644 --- a/desktop/src-tauri/src/managed_agents/discovery/presets.rs +++ b/desktop/src-tauri/src/managed_agents/discovery/presets.rs @@ -90,6 +90,15 @@ pub(super) fn preset_catalog_entry( } pub(super) const PRESET_HARNESSES: &[PresetHarness] = &[ + PresetHarness { + id: "pi", + label: "Pi", + command: "pi-acp", + args: &[], + install_instructions_url: "https://github.com/svkozak/pi-acp", + install_hint: "Buzz talks to Pi through the pi-acp adapter. Install Pi with `npm install -g --ignore-scripts @earendil-works/pi-coding-agent`, then install the adapter with `npm install -g pi-acp`.", + underlying_cli: Some("pi"), + }, PresetHarness { id: "devin", label: "Devin", @@ -347,6 +356,48 @@ mod tests { assert_eq!(entry.source, HarnessSource::Preset); } + #[test] + fn pi_preset_uses_zero_arg_adapter_and_reports_missing_component() { + let preset = PRESET_HARNESSES + .iter() + .find(|preset| preset.id == "pi") + .expect("Pi preset should be present"); + + assert_eq!(preset.label, "Pi"); + assert_eq!(preset.command, "pi-acp"); + assert!(preset.args.is_empty()); + assert_eq!(preset.underlying_cli, Some("pi")); + + let available = preset_catalog_entry(preset, |command| match command { + "pi-acp" => Some(PathBuf::from("/usr/local/bin/pi-acp")), + "pi" => Some(PathBuf::from("/usr/local/bin/pi")), + _ => None, + }); + assert_eq!(available.availability, AcpAvailabilityStatus::Available); + assert_eq!(available.command.as_deref(), Some("pi-acp")); + assert!(available.default_args.is_empty()); + assert_eq!( + available.underlying_cli_path.as_deref(), + Some("/usr/local/bin/pi") + ); + + let adapter_missing = preset_catalog_entry(preset, |command| { + (command == "pi").then(|| PathBuf::from("/usr/local/bin/pi")) + }); + assert_eq!( + adapter_missing.availability, + AcpAvailabilityStatus::AdapterMissing + ); + assert!(adapter_missing.command.is_none()); + assert!(adapter_missing.default_args.is_empty()); + + let not_installed = preset_catalog_entry(preset, |_| None); + assert_eq!( + not_installed.availability, + AcpAvailabilityStatus::NotInstalled + ); + } + #[test] fn adapter_missing_when_underlying_cli_present() { let entry = preset_catalog_entry(&ADAPTER_PRESET, |command| { diff --git a/desktop/src/features/onboarding/ui/RuntimeIcon.tsx b/desktop/src/features/onboarding/ui/RuntimeIcon.tsx index 5b247c31f7..57f1a703d1 100644 --- a/desktop/src/features/onboarding/ui/RuntimeIcon.tsx +++ b/desktop/src/features/onboarding/ui/RuntimeIcon.tsx @@ -19,6 +19,7 @@ const RUNTIME_LOGOS: Record = { export const PRESET_LOGOS: Record = { devin: "/harness-logos/devin.svg", omp: "/harness-logos/omp.svg", + pi: "/harness-logos/pi.svg", grok: "/harness-logos/grok.svg", opencode: "/harness-logos/opencode.svg", kimi: "/harness-logos/kimi.png", diff --git a/desktop/src/features/settings/ui/harnessCatalogCopy.test.mjs b/desktop/src/features/settings/ui/harnessCatalogCopy.test.mjs new file mode 100644 index 0000000000..86a3f6b760 --- /dev/null +++ b/desktop/src/features/settings/ui/harnessCatalogCopy.test.mjs @@ -0,0 +1,11 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { harnessDescription } from "./harnessCatalogCopy.ts"; + +test("Pi catalog entry has its curated product description", () => { + assert.equal( + harnessDescription("pi"), + "A minimal terminal coding harness, connected through the pi-acp adapter.", + ); +}); diff --git a/desktop/src/features/settings/ui/harnessCatalogCopy.ts b/desktop/src/features/settings/ui/harnessCatalogCopy.ts index 9a71ff70f9..f91dada83f 100644 --- a/desktop/src/features/settings/ui/harnessCatalogCopy.ts +++ b/desktop/src/features/settings/ui/harnessCatalogCopy.ts @@ -27,6 +27,8 @@ const HARNESS_DESCRIPTIONS: Record = { cursor: "Cursor's coding agent, connected to Buzz through its ACP server.", // Source: https://github.com/can1357/oh-my-pi omp: "A terminal coding agent with integrated development tools.", + // Sources: https://pi.dev/docs/latest, https://github.com/svkozak/pi-acp + pi: "A minimal terminal coding harness, connected through the pi-acp adapter.", // Source: https://build.x.ai (docs unavailable during research; kept // deliberately conservative). grok: "xAI's coding agent, connected to Buzz through its ACP entrypoint.", From beb76406c12ab8a7af9b2fcf7547c353e3369c34 Mon Sep 17 00:00:00 2001 From: ravarora2 <130506156+ravarora2@users.noreply.github.com> Date: Tue, 1 Sep 2026 18:31:30 -0400 Subject: [PATCH 2/5] feat(relay): add detailed readiness metrics (#7149) ## Summary This PR makes relay readiness failures diagnosable without weakening the existing fail-closed readiness contract. It distinguishes Postgres pool acquisition from query execution, Redis pool acquisition, deletion-catalog validation, and the overall two-second deadline; exports a bounded Prometheus contract for rollout dashboards; and fixes the concurrency, shutdown, and listener-boundary semantics needed for those signals to be trustworthy. ## Why The previous `/_readiness` implementation exposed only an aggregate ready/not-ready result. During a rollout, operators could not tell whether a pod was blocked on: - acquiring a Postgres connection; - executing the Postgres readiness query; - acquiring a Redis connection; - validating the deletion catalog; or - the shared readiness deadline. Adding metrics to the existing handler also exposed three correctness hazards that this PR resolves: 1. the same handler is mounted on both the public application listener and the private Kubernetes health listener, so public requests could otherwise distort rollout telemetry; 2. concurrent probes can finish out of order, allowing an older result to overwrite newer current-state gauges; and 3. a probe started before SIGTERM can finish afterward, incorrectly return `200 ready`, and resurrect ready gauges while the process is draining. ## Behavior ### Readiness evaluation - Postgres, Redis, and deletion-catalog checks still run under one shared two-second deadline. - Postgres distinguishes pool acquisition timeout/error from query timeout/error. - Redis distinguishes pool acquisition timeout/error. This does not claim a Redis command round trip. - The deletion catalog distinguishes operation timeout/error. - Multiple failures are reported as `multiple_dependencies_failed`; exhaustion of the shared deadline is reported as `overall_timeout` when no more specific completed outcome wins. - Readiness remains fail-closed: every dependency must succeed for `200 {"status":"ready"}`. ### Ordered publication and shutdown `ReadinessCoordinator` is process-owned and uses one mutex as the linearization point for probe generations, current-state publication, and terminal shutdown. - Every completed dependency attempt may contribute its truthful counter and duration observation. - Only the newest admissible probe generation may publish current-state gauges. - An older, slower probe cannot overwrite a newer probe's gauges. - `begin_shutdown()` and probe commit serialize through the same coordinator. - Once shutdown commits, an in-flight probe cannot return ready or publish ready/current dependency gauges, even if its dependency work later succeeds. Shutdown without dependency evaluation records only: - `buzz_readiness_checks_total{reason="shutting_down"}`; and - `buzz_readiness_state{check="overall"} = 0`. It does **not** fabricate dependency failures, dependency state changes, or zero-duration latency samples. If shutdown wins after an in-flight evaluation actually ran, those completed dependency attempts may remain as attempt telemetry, but they cannot overwrite shutdown-dominant current state. ### Listener and response contract - The private health listener's `/_readiness` route is the sole authority for rollout readiness telemetry. - The public application listener retains `/_readiness` for compatibility, evaluates the same dependencies, and preserves the existing response shape, but it emits no `buzz_readiness_*` metrics. - Ready responses remain `200 {"status":"ready"}`. - Shutdown responses remain `503 {"status":"shutting_down"}`. - Failed private-health responses include the bounded `reason` plus `postgres`, `redis`, and `deletion_catalog` booleans. - Failed public compatibility responses retain the dependency booleans but omit the new detailed reason. - No header, query parameter, path value, or other request-controlled value becomes a metric label. ## Prometheus contract The final schema is intentionally capped at **99 raw Prometheus series per pod**. | Metric | Type | Labels | Raw series/pod | |---|---|---|---:| | `buzz_readiness_checks_total` | counter | `reason` | 12 | | `buzz_readiness_dependency_checks_total` | counter | `dependency`, typed `outcome` | 11 | | `buzz_readiness_check_duration_seconds` | histogram | `check` | 72 | | `buzz_readiness_state` | gauge | `check` | 4 | | **Total** | | | **99** | ### Closed label sets `reason`: ```text ready shutting_down postgres_pool_timeout postgres_pool_error postgres_query_timeout postgres_query_error redis_pool_timeout redis_pool_error deletion_catalog_timeout deletion_catalog_error overall_timeout multiple_dependencies_failed ``` Valid `dependency` / `outcome` pairs are enforced by typed enums: - `postgres`: `success | pool_timeout | pool_error | operation_timeout | operation_error` - `redis`: `success | pool_timeout | pool_error` - `deletion_catalog`: `success | operation_timeout | operation_error` `check` is `overall | postgres | redis | deletion_catalog`. The readiness histogram has 15 configured buckets concentrated around the two-second deadline: ```text 0.001, 0.005, 0.01, 0.025, 0.05, 0.1, 0.25, 0.5, 0.75, 1.0, 1.25, 1.5, 1.75, 2.0, 2.5, +Inf ``` The contract deliberately removes the redundant overall `result` label and the histogram `outcome` label. Pod, ReplicaSet, version, rollout, raw error, SQL, URL, tenant, user, community, pubkey, and request-controlled values are prohibited application labels; infrastructure enrichment can supply deployment identity outside the application metric. ## Postgres production seam and CI The real `Db::readiness_check` path now supports deterministic production-seam testing while preserving the same acquisition/query implementation used by the relay. The isolated PostgreSQL lane automatically discovers and executes three ignored integration tests covering: - a held sole connection causing pool timeout, followed by recovery after release; - closed-pool acquisition error; - acquisition success followed by query timeout; - classified query error; - cancellation while waiting for a connection; - cancellation during an in-flight query; and - eventual pool recovery with waiter/in-flight state balanced. This prevents the central SQLx/Postgres behavior from being merely compiled but never executed in CI. ## Tests and verification - `cargo fmt --all -- --check` - `./scripts/test-postgres-test-discovery.sh` - complete `cargo test -p buzz-db` - focused `cargo test -p buzz-relay readiness` - real-router `cargo test -p buzz-relay real_health_route_` - controlled PostgreSQL execution of all three ignored readiness tests - production health router -> real `GET /_readiness` -> Prometheus render assertions - public-router requests proving zero rollout telemetry - deterministic out-of-order A/B probe tests - SIGTERM-during-probe tests proving shutdown dominance - exported metric name/type/exact-label/bucket assertions - exported raw-series allowlist and 99-series ceiling assertion The route-to-scrape regression test uses the production Prometheus builder and verifies the `2`, `2.5`, and `+Inf` readiness buckets. It fails if the health recording call, health route, public/health boundary, generation fence, shutdown fence, or bucket override is removed. ## Risk assessment **Medium.** This changes the live readiness publication path and adds synchronization around probe commit/shutdown. The risk is bounded by: - preserving the existing dependencies and shared two-second deadline; - keeping readiness fail-closed; - using a short, process-local mutex only at begin/commit linearization points, not across dependency awaits; - retaining the public compatibility endpoint while isolating its telemetry; - enforcing typed, low-cardinality labels and an exported series ceiling; and - covering the production router, Prometheus exposition, real PostgreSQL seam, concurrency ordering, and shutdown races. ## Operational notes - Dashboards should treat `buzz_readiness_state` as the latest sampled current state, not as an event stream. - `shutting_down` should be excluded from dependency-failure alerts because no dependency failure is implied. - Do not use `default_zero()` for missing current-state data; missing/stale is unknown, not healthy. - Use histogram buckets, heatmaps, max, or average until the Datadog distribution metadata confirms percentiles are enabled; do not title a widget p95 before that live readback. ## Non-goals This PR does not add the broader process-startup lifecycle, worker/listener supervision, shutdown coordinator, WebSocket/huddle handoff, client recovery telemetry, dashboard mutations, Datadog configuration changes, or deployment changes. Those remain separate follow-up work. ## References - [Staging dev relay image runbook](https://github.com/block/buzz/blob/main/docs/staging-dev-relay-images.md) - Readiness telemetry contract: `deploy/charts/buzz/README.md` --------- Signed-off-by: Ravneet Arora --- crates/buzz-db/src/lib.rs | 3 +- crates/buzz-db/src/runtime/mod.rs | 61 +- crates/buzz-db/src/runtime/tests.rs | 147 +++++ crates/buzz-relay/src/lib.rs | 1 + crates/buzz-relay/src/main.rs | 5 +- crates/buzz-relay/src/metrics.rs | 66 ++- crates/buzz-relay/src/readiness.rs | 836 ++++++++++++++++++++++++++++ crates/buzz-relay/src/router.rs | 616 ++++++++++++++++++-- crates/buzz-relay/src/state.rs | 20 + deploy/charts/buzz/README.md | 21 + 10 files changed, 1730 insertions(+), 46 deletions(-) create mode 100644 crates/buzz-relay/src/readiness.rs diff --git a/crates/buzz-db/src/lib.rs b/crates/buzz-db/src/lib.rs index e096960ad4..30c46e2fd9 100644 --- a/crates/buzz-db/src/lib.rs +++ b/crates/buzz-db/src/lib.rs @@ -31,7 +31,8 @@ pub mod error; mod test_support; pub use runtime::{ - insert_mentions, migration, replica_fence, Db, DbConfig, DbPoolStats, ReadSession, + insert_mentions, migration, replica_fence, Db, DbConfig, DbPoolStats, DbReadinessOutcome, + ReadSession, }; pub(crate) use runtime::{ insert_mentions_in_transaction, observability, route_proof, ReadSessionInner, RouteDecision, diff --git a/crates/buzz-db/src/runtime/mod.rs b/crates/buzz-db/src/runtime/mod.rs index 693d75d66b..214cc4bca6 100644 --- a/crates/buzz-db/src/runtime/mod.rs +++ b/crates/buzz-db/src/runtime/mod.rs @@ -425,6 +425,26 @@ pub struct DbPoolStats { pub max: u32, } +/// Bounded outcome of the Postgres portion of a relay readiness check. +/// +/// The variants deliberately separate waiting for a pooled connection from +/// executing the health query. Callers may safely use the variant names as +/// low-cardinality metric labels; detailed SQLx errors remain in logs rather +/// than becoming labels. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum DbReadinessOutcome { + /// A writer-pool connection was acquired and `SELECT 1` succeeded. + Success, + /// No writer-pool connection became available before the readiness deadline. + PoolTimeout, + /// The writer pool returned a non-timeout acquisition error. + PoolError, + /// A connection was acquired, but `SELECT 1` exceeded the readiness deadline. + QueryTimeout, + /// A connection was acquired, but `SELECT 1` returned an error. + QueryError, +} + /// Configuration for the Postgres connection pool. #[derive(Debug, Clone)] pub struct DbConfig { @@ -931,11 +951,50 @@ impl Db { migration::run_migrations(&self.pool).await } - /// Returns `true` if the database is reachable (used by readiness probes). + /// Returns `true` if the database is reachable. pub async fn ping(&self) -> bool { sqlx::query("SELECT 1").execute(&self.pool).await.is_ok() } + /// Checks writer-pool acquisition and query execution against one deadline. + /// + /// Unlike [`Self::ping`], this preserves whether readiness was blocked while + /// borrowing a connection or failed after a connection had been acquired. + /// The query runs on the already-acquired connection so the two phases + /// cannot be collapsed into a second implicit pool acquisition. + pub async fn readiness_check(&self, deadline: tokio::time::Instant) -> DbReadinessOutcome { + self.readiness_check_sql(deadline, "SELECT 1").await + } + + /// Production-bound seam for classifying failures after pool acquisition. + /// Tests vary only the SQL so timeout/error/cancellation paths execute the + /// same acquisition and classification code as [`Self::readiness_check`]. + async fn readiness_check_sql( + &self, + deadline: tokio::time::Instant, + query: &'static str, + ) -> DbReadinessOutcome { + let mut connection = match tokio::time::timeout_at(deadline, self.pool.acquire()).await { + Err(_) => return DbReadinessOutcome::PoolTimeout, + Ok(Err(sqlx::Error::PoolTimedOut)) => return DbReadinessOutcome::PoolTimeout, + Ok(Err(error)) => { + tracing::debug!(error = %error, "Postgres readiness pool acquisition failed"); + return DbReadinessOutcome::PoolError; + } + Ok(Ok(connection)) => connection, + }; + + match tokio::time::timeout_at(deadline, sqlx::query(query).execute(&mut *connection)).await + { + Err(_) => DbReadinessOutcome::QueryTimeout, + Ok(Err(error)) => { + tracing::debug!(error = %error, "Postgres readiness query failed"); + DbReadinessOutcome::QueryError + } + Ok(Ok(_)) => DbReadinessOutcome::Success, + } + } + /// Returns pool utilisation stats for metrics emission. /// /// `size` — total connections (idle + active) diff --git a/crates/buzz-db/src/runtime/tests.rs b/crates/buzz-db/src/runtime/tests.rs index a2938112bc..3022a15e96 100644 --- a/crates/buzz-db/src/runtime/tests.rs +++ b/crates/buzz-db/src/runtime/tests.rs @@ -19,6 +19,153 @@ async fn setup_db() -> Db { Db::from_pool(pool) } +#[tokio::test] +#[ignore = "requires Postgres"] +async fn readiness_check_distinguishes_pool_exhaustion_from_success() { + let database_url = crate::test_support::database_url(); + let pool = sqlx::postgres::PgPoolOptions::new() + .max_connections(1) + .connect(&database_url) + .await + .expect("connect size-one readiness test pool"); + let held = pool + .acquire() + .await + .expect("hold the only readiness test connection"); + let db = Db::from_pool(pool); + + let exhausted = db + .readiness_check(tokio::time::Instant::now() + std::time::Duration::from_millis(25)) + .await; + assert_eq!(exhausted, DbReadinessOutcome::PoolTimeout); + + drop(held); + let recovered = db + .readiness_check(tokio::time::Instant::now() + std::time::Duration::from_secs(1)) + .await; + assert_eq!(recovered, DbReadinessOutcome::Success); +} + +#[tokio::test] +#[ignore = "requires Postgres"] +async fn readiness_check_classifies_closed_pool_query_timeout_and_query_error() { + let database_url = crate::test_support::database_url(); + + let closed_pool = sqlx::postgres::PgPoolOptions::new() + .max_connections(1) + .connect(&database_url) + .await + .expect("connect closed readiness test pool"); + closed_pool.close().await; + let closed = Db::from_pool(closed_pool) + .readiness_check(tokio::time::Instant::now() + std::time::Duration::from_secs(1)) + .await; + assert_eq!(closed, DbReadinessOutcome::PoolError); + + let pool = sqlx::postgres::PgPoolOptions::new() + .max_connections(1) + .connect(&database_url) + .await + .expect("connect query classification test pool"); + let db = Db::from_pool(pool); + + let timed_out = db + .readiness_check_sql( + tokio::time::Instant::now() + std::time::Duration::from_millis(25), + "SELECT pg_sleep(0.2)", + ) + .await; + assert_eq!(timed_out, DbReadinessOutcome::QueryTimeout); + + let query_error = db + .readiness_check_sql( + tokio::time::Instant::now() + std::time::Duration::from_secs(1), + "SELECT 1 / 0", + ) + .await; + assert_eq!(query_error, DbReadinessOutcome::QueryError); + + assert_eq!( + db.readiness_check(tokio::time::Instant::now() + std::time::Duration::from_secs(1)) + .await, + DbReadinessOutcome::Success, + "query failures must return the acquired connection to the pool" + ); +} + +#[tokio::test] +#[ignore = "requires Postgres"] +async fn readiness_check_cancellation_balances_waiter_and_inflight_connection() { + let database_url = crate::test_support::database_url(); + let pool = sqlx::postgres::PgPoolOptions::new() + .max_connections(1) + .connect(&database_url) + .await + .expect("connect cancellation readiness test pool"); + let held = pool + .acquire() + .await + .expect("hold sole connection before waiter cancellation"); + let db = Db::from_pool(pool); + + let waiting_db = db.clone(); + let waiting = tokio::spawn(async move { + waiting_db + .readiness_check(tokio::time::Instant::now() + std::time::Duration::from_secs(5)) + .await + }); + tokio::time::sleep(std::time::Duration::from_millis(25)).await; + waiting.abort(); + assert!(waiting + .await + .expect_err("waiting check must be cancelled") + .is_cancelled()); + drop(held); + + assert_eq!( + db.readiness_check(tokio::time::Instant::now() + std::time::Duration::from_secs(1)) + .await, + DbReadinessOutcome::Success, + "cancelled pool waiter must not consume the released connection" + ); + + let querying_db = db.clone(); + let querying = tokio::spawn(async move { + querying_db + .readiness_check_sql( + tokio::time::Instant::now() + std::time::Duration::from_secs(5), + "SELECT pg_sleep(5)", + ) + .await + }); + tokio::time::sleep(std::time::Duration::from_millis(25)).await; + querying.abort(); + assert!(querying + .await + .expect_err("querying check must be cancelled") + .is_cancelled()); + + let recovered = tokio::time::timeout(std::time::Duration::from_secs(5), async { + loop { + let outcome = db + .readiness_check( + tokio::time::Instant::now() + std::time::Duration::from_millis(250), + ) + .await; + match outcome { + DbReadinessOutcome::Success => break outcome, + DbReadinessOutcome::PoolTimeout => tokio::task::yield_now().await, + unexpected => panic!( + "cancelled in-flight query produced unexpected recovery outcome: {unexpected:?}" + ), + } + } + }) + .await + .expect("cancelled in-flight query must return or replace its connection"); + assert_eq!(recovered, DbReadinessOutcome::Success); +} + async fn make_community(pool: &PgPool) -> Uuid { let id = Uuid::new_v4(); let host = format!("communities-of-channels-{}.example", id.simple()); diff --git a/crates/buzz-relay/src/lib.rs b/crates/buzz-relay/src/lib.rs index e762c14b1e..123440c041 100644 --- a/crates/buzz-relay/src/lib.rs +++ b/crates/buzz-relay/src/lib.rs @@ -35,6 +35,7 @@ pub mod nip11; pub mod protocol; /// Durable NIP-PL matcher and delivery worker. pub mod push_runtime; +mod readiness; /// Axum router construction. pub mod router; /// Shared application state. diff --git a/crates/buzz-relay/src/main.rs b/crates/buzz-relay/src/main.rs index bb8715508e..260dfaed68 100644 --- a/crates/buzz-relay/src/main.rs +++ b/crates/buzz-relay/src/main.rs @@ -1,5 +1,4 @@ use std::collections::{HashMap, HashSet}; -use std::sync::atomic::Ordering; use std::sync::Arc; use tracing::{error, info, warn}; @@ -1312,7 +1311,7 @@ async fn serve( }); let (shutdown_tx, _) = tokio::sync::watch::channel(false); - let shutdown_flag = Arc::clone(&state.shutting_down); + let shutdown_state = Arc::clone(&state); let drain_conn_manager = Arc::clone(&state.conn_manager); let drain_jitter_ms = state.config.drain_jitter_ms; let tx = shutdown_tx.clone(); @@ -1345,7 +1344,7 @@ async fn serve( // sleeps. Not implemented here. This comment records the plan only. let shutdown_handle = tokio::spawn(async move { shutdown_signal().await; - shutdown_flag.store(true, Ordering::Relaxed); + shutdown_state.begin_shutdown(); info!("Shutdown signal received — readiness now returns 503"); // 5s grace: let K8s stop routing new traffic before we close listeners. tokio::time::sleep(std::time::Duration::from_secs(5)).await; diff --git a/crates/buzz-relay/src/metrics.rs b/crates/buzz-relay/src/metrics.rs index 16e521a44e..fb484b0174 100644 --- a/crates/buzz-relay/src/metrics.rs +++ b/crates/buzz-relay/src/metrics.rs @@ -32,6 +32,11 @@ const LATENCY_BUCKETS_MS: [f64; 11] = [ /// Seconds-scale buckets for internal processing histograms (event, search, audit). const DURATION_BUCKETS_S: [f64; 10] = [0.001, 0.005, 0.01, 0.025, 0.05, 0.1, 0.25, 0.5, 1.0, 5.0]; +/// Readiness buckets concentrate resolution near the two-second failure budget. +const READINESS_DURATION_BUCKETS_S: [f64; 15] = [ + 0.001, 0.005, 0.01, 0.025, 0.05, 0.1, 0.25, 0.5, 0.75, 1.0, 1.25, 1.5, 1.75, 2.0, 2.5, +]; + /// Seconds-scale buckets for Git hydration and pack streams. const GIT_DURATION_BUCKETS_S: [f64; 13] = [ 0.01, 0.05, 0.1, 0.25, 0.5, 1.0, 2.5, 5.0, 10.0, 30.0, 60.0, 120.0, 300.0, @@ -56,16 +61,8 @@ const GIT_PACK_BUCKETS: [f64; 9] = [0.0, 1.0, 2.0, 4.0, 8.0, 16.0, 32.0, 64.0, 1 /// Integer-count buckets for fan-out recipient histograms. const FANOUT_BUCKETS: [f64; 9] = [0.0, 1.0, 5.0, 10.0, 25.0, 50.0, 100.0, 500.0, 1000.0]; -/// Install the global metrics recorder and spawn the Prometheus HTTP exporter. -/// -/// `build()` returns the recorder + exporter future and internally spawns -/// the upkeep task, so no separate upkeep call is needed. -/// -/// Must be called from within a Tokio runtime. -/// Panics if a recorder is already installed or the port is in use. -pub fn install(port: u16, gauge_idle_timeout_secs: u64) { - let (recorder, exporter) = PrometheusBuilder::new() - .with_http_listener(([0, 0, 0, 0], port)) +fn configured_prometheus_builder(gauge_idle_timeout_secs: u64) -> PrometheusBuilder { + PrometheusBuilder::new() // Remove gauge series that the relay intentionally stops emitting. .idle_timeout( MetricKindMask::GAUGE, @@ -102,6 +99,11 @@ pub fn install(port: u16, gauge_idle_timeout_secs: u64) { &GIT_DURATION_BUCKETS_S, ) .expect("valid git compaction duration bucket boundaries") + .set_buckets_for_metric( + Matcher::Full("buzz_readiness_check_duration_seconds".to_owned()), + &READINESS_DURATION_BUCKETS_S, + ) + .expect("valid readiness duration bucket boundaries") .set_buckets_for_metric( Matcher::Full("buzz_git_hydrate_bytes".to_owned()), &GIT_BYTES_BUCKETS, @@ -139,13 +141,57 @@ pub fn install(port: u16, gauge_idle_timeout_secs: u64) { &FANOUT_BUCKETS, ) .expect("valid fanout bucket boundaries") +} + +/// Install the global metrics recorder and spawn the Prometheus HTTP exporter. +/// +/// `build()` returns the recorder + exporter future and internally spawns +/// the upkeep task, so no separate upkeep call is needed. +/// +/// Must be called from within a Tokio runtime. +/// Panics if a recorder is already installed or the port is in use. +pub fn install(port: u16, gauge_idle_timeout_secs: u64) { + let (recorder, exporter) = configured_prometheus_builder(gauge_idle_timeout_secs) + .with_http_listener(([0, 0, 0, 0], port)) .build() .expect("metrics exporter must build exactly once"); metrics::set_global_recorder(recorder).expect("global recorder must be set exactly once"); + describe_readiness_metrics(); tokio::spawn(exporter); } +/// Register the frozen readiness metric descriptions with the active recorder. +pub(crate) fn describe_readiness_metrics() { + metrics::describe_counter!( + "buzz_readiness_checks_total", + "Kubernetes health-listener readiness probes by terminal bounded reason" + ); + metrics::describe_counter!( + "buzz_readiness_dependency_checks_total", + "Completed readiness dependency attempts by dependency and bounded outcome" + ); + metrics::describe_histogram!( + "buzz_readiness_check_duration_seconds", + metrics::Unit::Seconds, + "Completed readiness check duration without outcome label multiplication" + ); + metrics::describe_gauge!( + "buzz_readiness_state", + "Latest publishable readiness state by check, where 1 is ready and 0 is not ready" + ); +} + +#[cfg(test)] +pub(crate) fn readiness_test_recorder() -> ( + metrics_exporter_prometheus::PrometheusRecorder, + metrics_exporter_prometheus::PrometheusHandle, +) { + let recorder = configured_prometheus_builder(300).build_recorder(); + let handle = recorder.handle(); + (recorder, handle) +} + /// Axum middleware that records CAKE framework HTTP metrics. /// /// Emits: diff --git a/crates/buzz-relay/src/readiness.rs b/crates/buzz-relay/src/readiness.rs new file mode 100644 index 0000000000..36a36c228b --- /dev/null +++ b/crates/buzz-relay/src/readiness.rs @@ -0,0 +1,836 @@ +//! Readiness dependency evaluation and ordered metrics publication. +//! +//! [`ReadinessCoordinator`] is process-owned. Its mutex is the linearization +//! point shared by health-probe commits and terminal shutdown, so an older +//! evaluation can never overwrite newer gauges or publish ready after shutdown. + +use std::future::Future; +use std::sync::{Arc, Mutex, MutexGuard, PoisonError}; +use std::time::Duration; + +use buzz_db::{Db, DbReadinessOutcome}; +use tokio::time::Instant; + +const READINESS_TIMEOUT: Duration = Duration::from_secs(2); + +/// Closed label set exported by `buzz_readiness_checks_total{reason}`. +#[cfg(test)] +pub(crate) const READINESS_REASON_LABELS: [&str; 12] = [ + "ready", + "shutting_down", + "postgres_pool_timeout", + "postgres_pool_error", + "postgres_query_timeout", + "postgres_query_error", + "redis_pool_timeout", + "redis_pool_error", + "deletion_catalog_timeout", + "deletion_catalog_error", + "overall_timeout", + "multiple_dependencies_failed", +]; + +/// Maximum raw Prometheus series emitted by readiness for one pod. +/// +/// - 12 overall reasons +/// - 11 valid dependency/outcome pairs (Postgres 5, Redis 3, catalog 3) +/// - 4 histograms x (15 configured buckets + `+Inf` + count + sum) = 72 +/// - 4 current-state gauges +#[cfg(test)] +pub(crate) const READINESS_RAW_SERIES_PER_POD: usize = 12 + 11 + (4 * 18) + 4; + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum PostgresOutcome { + Success, + PoolTimeout, + PoolError, + QueryTimeout, + QueryError, +} + +impl PostgresOutcome { + fn label(self) -> &'static str { + match self { + Self::Success => "success", + Self::PoolTimeout => "pool_timeout", + Self::PoolError => "pool_error", + Self::QueryTimeout => "operation_timeout", + Self::QueryError => "operation_error", + } + } + + fn is_success(self) -> bool { + self == Self::Success + } + + fn is_timeout(self) -> bool { + matches!(self, Self::PoolTimeout | Self::QueryTimeout) + } +} + +impl From for PostgresOutcome { + fn from(outcome: DbReadinessOutcome) -> Self { + match outcome { + DbReadinessOutcome::Success => Self::Success, + DbReadinessOutcome::PoolTimeout => Self::PoolTimeout, + DbReadinessOutcome::PoolError => Self::PoolError, + DbReadinessOutcome::QueryTimeout => Self::QueryTimeout, + DbReadinessOutcome::QueryError => Self::QueryError, + } + } +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum RedisOutcome { + Success, + PoolTimeout, + PoolError, +} + +impl RedisOutcome { + fn label(self) -> &'static str { + match self { + Self::Success => "success", + Self::PoolTimeout => "pool_timeout", + Self::PoolError => "pool_error", + } + } + + fn is_success(self) -> bool { + self == Self::Success + } + + fn is_timeout(self) -> bool { + self == Self::PoolTimeout + } +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum DeletionCatalogOutcome { + Success, + OperationTimeout, + OperationError, +} + +impl DeletionCatalogOutcome { + fn label(self) -> &'static str { + match self { + Self::Success => "success", + Self::OperationTimeout => "operation_timeout", + Self::OperationError => "operation_error", + } + } + + fn is_success(self) -> bool { + self == Self::Success + } + + fn is_timeout(self) -> bool { + self == Self::OperationTimeout + } +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum ReadinessReason { + Ready, + ShuttingDown, + PostgresPoolTimeout, + PostgresPoolError, + PostgresQueryTimeout, + PostgresQueryError, + RedisPoolTimeout, + RedisPoolError, + DeletionCatalogTimeout, + DeletionCatalogError, + OverallTimeout, + MultipleDependenciesFailed, +} + +impl ReadinessReason { + pub(crate) fn label(self) -> &'static str { + match self { + Self::Ready => "ready", + Self::ShuttingDown => "shutting_down", + Self::PostgresPoolTimeout => "postgres_pool_timeout", + Self::PostgresPoolError => "postgres_pool_error", + Self::PostgresQueryTimeout => "postgres_query_timeout", + Self::PostgresQueryError => "postgres_query_error", + Self::RedisPoolTimeout => "redis_pool_timeout", + Self::RedisPoolError => "redis_pool_error", + Self::DeletionCatalogTimeout => "deletion_catalog_timeout", + Self::DeletionCatalogError => "deletion_catalog_error", + Self::OverallTimeout => "overall_timeout", + Self::MultipleDependenciesFailed => "multiple_dependencies_failed", + } + } +} + +#[derive(Debug, Clone, Copy)] +pub(crate) struct TimedOutcome { + outcome: O, + duration: Duration, +} + +impl TimedOutcome { + #[cfg(test)] + pub(crate) fn new(outcome: O, duration: Duration) -> Self { + Self { outcome, duration } + } +} + +#[derive(Debug, Clone, Copy)] +pub(crate) struct ReadinessEvaluation { + postgres: Option>, + redis: Option>, + deletion_catalog: Option>, + pub(crate) reason: ReadinessReason, + total_duration: Duration, +} + +impl ReadinessEvaluation { + pub(crate) fn shutting_down() -> Self { + Self { + postgres: None, + redis: None, + deletion_catalog: None, + reason: ReadinessReason::ShuttingDown, + total_duration: Duration::ZERO, + } + } + + #[cfg(test)] + pub(crate) fn from_results( + postgres: TimedOutcome, + redis: TimedOutcome, + deletion_catalog: TimedOutcome, + total_duration: Duration, + ) -> Self { + Self::for_dependencies(postgres, redis, deletion_catalog, total_duration) + } + + fn for_dependencies( + postgres: TimedOutcome, + redis: TimedOutcome, + deletion_catalog: TimedOutcome, + total_duration: Duration, + ) -> Self { + let reason = final_reason(postgres.outcome, redis.outcome, deletion_catalog.outcome); + Self { + postgres: Some(postgres), + redis: Some(redis), + deletion_catalog: Some(deletion_catalog), + reason, + total_duration, + } + } + + pub(crate) fn is_ready(self) -> bool { + self.reason == ReadinessReason::Ready + } + + pub(crate) fn postgres_ready(self) -> bool { + self.postgres + .is_some_and(|result| result.outcome.is_success()) + } + + pub(crate) fn redis_ready(self) -> bool { + self.redis.is_some_and(|result| result.outcome.is_success()) + } + + pub(crate) fn deletion_catalog_ready(self) -> bool { + self.deletion_catalog + .is_some_and(|result| result.outcome.is_success()) + } + + fn dependencies_ran(self) -> bool { + self.postgres.is_some() || self.redis.is_some() || self.deletion_catalog.is_some() + } +} + +fn final_reason( + postgres: PostgresOutcome, + redis: RedisOutcome, + deletion_catalog: DeletionCatalogOutcome, +) -> ReadinessReason { + let failure_count = usize::from(!postgres.is_success()) + + usize::from(!redis.is_success()) + + usize::from(!deletion_catalog.is_success()); + + if failure_count == 0 { + return ReadinessReason::Ready; + } + if failure_count > 1 { + let all_failures_are_timeouts = (postgres.is_success() || postgres.is_timeout()) + && (redis.is_success() || redis.is_timeout()) + && (deletion_catalog.is_success() || deletion_catalog.is_timeout()); + return if all_failures_are_timeouts { + ReadinessReason::OverallTimeout + } else { + ReadinessReason::MultipleDependenciesFailed + }; + } + + match postgres { + PostgresOutcome::PoolTimeout => ReadinessReason::PostgresPoolTimeout, + PostgresOutcome::PoolError => ReadinessReason::PostgresPoolError, + PostgresOutcome::QueryTimeout => ReadinessReason::PostgresQueryTimeout, + PostgresOutcome::QueryError => ReadinessReason::PostgresQueryError, + PostgresOutcome::Success => match redis { + RedisOutcome::PoolTimeout => ReadinessReason::RedisPoolTimeout, + RedisOutcome::PoolError => ReadinessReason::RedisPoolError, + RedisOutcome::Success => match deletion_catalog { + DeletionCatalogOutcome::OperationTimeout => ReadinessReason::DeletionCatalogTimeout, + DeletionCatalogOutcome::OperationError => ReadinessReason::DeletionCatalogError, + DeletionCatalogOutcome::Success => ReadinessReason::Ready, + }, + }, + } +} + +async fn timed(future: F) -> TimedOutcome +where + F: Future, +{ + let started_at = Instant::now(); + let outcome = future.await; + TimedOutcome { + outcome, + duration: started_at.elapsed(), + } +} + +async fn evaluate_dependencies( + postgres: P, + redis: R, + deletion_catalog: D, +) -> ReadinessEvaluation +where + P: Future, + R: Future, + D: Future, +{ + let started_at = Instant::now(); + let (postgres, redis, deletion_catalog) = + tokio::join!(timed(postgres), timed(redis), timed(deletion_catalog),); + ReadinessEvaluation::for_dependencies(postgres, redis, deletion_catalog, started_at.elapsed()) +} + +async fn redis_check(pool: &deadpool_redis::Pool, deadline: Instant) -> RedisOutcome { + match tokio::time::timeout_at(deadline, pool.get()).await { + Err(_) => RedisOutcome::PoolTimeout, + Ok(Err(error)) => { + tracing::debug!(error = %error, "Redis readiness pool acquisition failed"); + RedisOutcome::PoolError + } + Ok(Ok(_connection)) => RedisOutcome::Success, + } +} + +async fn deletion_catalog_check(db: &Db, deadline: Instant) -> DeletionCatalogOutcome { + match tokio::time::timeout_at(deadline, db.validate_deletion_serving_catalog()).await { + Err(_) => DeletionCatalogOutcome::OperationTimeout, + Ok(Err(error)) => { + tracing::debug!(error = %error, "Deletion catalog readiness validation failed"); + DeletionCatalogOutcome::OperationError + } + Ok(Ok(())) => DeletionCatalogOutcome::Success, + } +} + +#[async_trait::async_trait] +pub(crate) trait ReadinessEvaluator: Send + Sync { + async fn evaluate(&self, db: &Db, redis_pool: &deadpool_redis::Pool) -> ReadinessEvaluation; +} + +struct ProductionReadinessEvaluator; + +#[async_trait::async_trait] +impl ReadinessEvaluator for ProductionReadinessEvaluator { + async fn evaluate(&self, db: &Db, redis_pool: &deadpool_redis::Pool) -> ReadinessEvaluation { + let deadline = Instant::now() + READINESS_TIMEOUT; + evaluate_dependencies( + async { db.readiness_check(deadline).await.into() }, + redis_check(redis_pool, deadline), + deletion_catalog_check(db, deadline), + ) + .await + } +} + +#[derive(Debug, Clone, Copy)] +pub(crate) struct ProbeTicket { + generation: u64, +} + +#[derive(Debug, Clone, Copy)] +pub(crate) enum ProbeStart { + Evaluate(ProbeTicket), + ShuttingDown, +} + +#[derive(Debug, Default)] +struct PublicationState { + next_generation: u64, + latest_published_generation: u64, + shutdown_generation: Option, +} + +/// Serializes readiness result publication with terminal process shutdown. +pub(crate) struct ReadinessCoordinator { + state: Mutex, + evaluator: Arc, +} + +impl Default for ReadinessCoordinator { + fn default() -> Self { + Self { + state: Mutex::new(PublicationState::default()), + evaluator: Arc::new(ProductionReadinessEvaluator), + } + } +} + +impl ReadinessCoordinator { + #[cfg(test)] + pub(crate) fn with_evaluator(evaluator: Arc) -> Self { + Self { + state: Mutex::new(PublicationState::default()), + evaluator, + } + } + + fn lock_state(&self) -> MutexGuard<'_, PublicationState> { + self.state.lock().unwrap_or_else(PoisonError::into_inner) + } + + pub(crate) async fn evaluate( + &self, + db: &Db, + redis_pool: &deadpool_redis::Pool, + ) -> ReadinessEvaluation { + self.evaluator.evaluate(db, redis_pool).await + } + + /// Allocates a health-probe generation or records a truthful shutdown fast path. + pub(crate) fn begin_probe(&self) -> ProbeStart { + let mut state = self.lock_state(); + if state.shutdown_generation.is_some() { + let evaluation = ReadinessEvaluation::shutting_down(); + record_attempt_metrics(&evaluation, ReadinessReason::ShuttingDown); + record_overall_state(false); + return ProbeStart::ShuttingDown; + } + + state.next_generation = state.next_generation.saturating_add(1); + ProbeStart::Evaluate(ProbeTicket { + generation: state.next_generation, + }) + } + + /// Commits one completed health probe through the shared publication fence. + pub(crate) fn finish_probe( + &self, + ticket: ProbeTicket, + evaluation: ReadinessEvaluation, + ) -> ReadinessEvaluation { + let mut state = self.lock_state(); + if state.shutdown_generation.is_some() { + record_attempt_metrics(&evaluation, ReadinessReason::ShuttingDown); + return ReadinessEvaluation::shutting_down(); + } + + record_attempt_metrics(&evaluation, evaluation.reason); + if ticket.generation > state.latest_published_generation { + record_current_state(&evaluation); + state.latest_published_generation = ticket.generation; + } + evaluation + } + + /// Returns whether a compatibility/public readiness evaluation may start. + pub(crate) fn public_evaluation_allowed(&self) -> bool { + self.lock_state().shutdown_generation.is_none() + } + + /// Makes shutdown dominate a public request that was already in flight. + pub(crate) fn finish_public_evaluation( + &self, + evaluation: ReadinessEvaluation, + ) -> ReadinessEvaluation { + if self.lock_state().shutdown_generation.is_some() { + ReadinessEvaluation::shutting_down() + } else { + evaluation + } + } + + /// Commits terminal shutdown and immediately publishes overall not-ready. + pub(crate) fn begin_shutdown(&self) { + let mut state = self.lock_state(); + if state.shutdown_generation.is_none() { + let generation = state.next_generation.saturating_add(1); + state.shutdown_generation = Some(generation); + record_overall_state(false); + } + } +} + +fn record_attempt_metrics(evaluation: &ReadinessEvaluation, reason: ReadinessReason) { + metrics::counter!( + "buzz_readiness_checks_total", + "reason" => reason.label(), + ) + .increment(1); + + if !evaluation.dependencies_ran() { + return; + } + + metrics::histogram!( + "buzz_readiness_check_duration_seconds", + "check" => "overall", + ) + .record(evaluation.total_duration.as_secs_f64()); + + if let Some(result) = evaluation.postgres { + record_dependency_attempt("postgres", result.outcome.label(), result.duration); + } + if let Some(result) = evaluation.redis { + record_dependency_attempt("redis", result.outcome.label(), result.duration); + } + if let Some(result) = evaluation.deletion_catalog { + record_dependency_attempt("deletion_catalog", result.outcome.label(), result.duration); + } +} + +fn record_dependency_attempt(dependency: &'static str, outcome: &'static str, duration: Duration) { + metrics::counter!( + "buzz_readiness_dependency_checks_total", + "dependency" => dependency, + "outcome" => outcome, + ) + .increment(1); + metrics::histogram!( + "buzz_readiness_check_duration_seconds", + "check" => dependency, + ) + .record(duration.as_secs_f64()); +} + +fn record_current_state(evaluation: &ReadinessEvaluation) { + record_overall_state(evaluation.is_ready()); + if let Some(result) = evaluation.postgres { + record_dependency_state("postgres", result.outcome.is_success()); + } + if let Some(result) = evaluation.redis { + record_dependency_state("redis", result.outcome.is_success()); + } + if let Some(result) = evaluation.deletion_catalog { + record_dependency_state("deletion_catalog", result.outcome.is_success()); + } +} + +fn record_overall_state(ready: bool) { + metrics::gauge!("buzz_readiness_state", "check" => "overall").set(if ready { + 1.0 + } else { + 0.0 + }); +} + +fn record_dependency_state(dependency: &'static str, ready: bool) { + metrics::gauge!("buzz_readiness_state", "check" => dependency).set(if ready { + 1.0 + } else { + 0.0 + }); +} + +#[cfg(test)] +mod tests { + use metrics_util::debugging::{DebugValue, DebuggingRecorder}; + use metrics_util::CompositeKey; + + use super::*; + + fn ready_evaluation() -> ReadinessEvaluation { + ReadinessEvaluation::from_results( + TimedOutcome::new(PostgresOutcome::Success, Duration::from_millis(35)), + TimedOutcome::new(RedisOutcome::Success, Duration::from_millis(10)), + TimedOutcome::new(DeletionCatalogOutcome::Success, Duration::from_millis(20)), + Duration::from_millis(35), + ) + } + + fn redis_failure_evaluation() -> ReadinessEvaluation { + ReadinessEvaluation::from_results( + TimedOutcome::new(PostgresOutcome::Success, Duration::from_millis(35)), + TimedOutcome::new(RedisOutcome::PoolTimeout, Duration::from_secs(2)), + TimedOutcome::new(DeletionCatalogOutcome::Success, Duration::from_millis(20)), + Duration::from_secs(2), + ) + } + + fn exact_metric<'a>( + snapshot: &'a [( + CompositeKey, + Option, + Option, + DebugValue, + )], + name: &str, + labels: &[(&str, &str)], + ) -> Option<&'a DebugValue> { + snapshot.iter().find_map(|(key, _, _, value)| { + let actual = key + .key() + .labels() + .map(|label| (label.key(), label.value())) + .collect::>(); + (key.key().name() == name + && actual.len() == labels.len() + && labels.iter().all(|expected| actual.contains(expected))) + .then_some(value) + }) + } + + fn gauge_value( + snapshot: &[( + CompositeKey, + Option, + Option, + DebugValue, + )], + check: &str, + ) -> f64 { + let value = exact_metric(snapshot, "buzz_readiness_state", &[("check", check)]) + .expect("readiness gauge"); + let DebugValue::Gauge(value) = value else { + panic!("readiness state must be a gauge"); + }; + value.into_inner() + } + + #[tokio::test(start_paused = true)] + async fn evaluation_preserves_a_completed_check_when_another_times_out() { + let evaluation = evaluate_dependencies( + async { + tokio::time::sleep(Duration::from_millis(35)).await; + PostgresOutcome::Success + }, + async { + tokio::time::sleep(Duration::from_secs(2)).await; + RedisOutcome::PoolTimeout + }, + async { + tokio::time::sleep(Duration::from_millis(10)).await; + DeletionCatalogOutcome::Success + }, + ) + .await; + + assert_eq!(evaluation.reason, ReadinessReason::RedisPoolTimeout); + assert_eq!( + evaluation.postgres.map(|result| result.duration), + Some(Duration::from_millis(35)) + ); + assert_eq!( + evaluation.redis.map(|result| result.duration), + Some(Duration::from_secs(2)) + ); + } + + #[test] + fn simultaneous_dependency_timeouts_are_an_overall_timeout() { + assert_eq!( + final_reason( + PostgresOutcome::PoolTimeout, + RedisOutcome::PoolTimeout, + DeletionCatalogOutcome::Success, + ), + ReadinessReason::OverallTimeout + ); + } + + #[test] + fn dependency_types_expose_only_valid_outcome_pairs() { + assert_eq!( + [ + PostgresOutcome::Success, + PostgresOutcome::PoolTimeout, + PostgresOutcome::PoolError, + PostgresOutcome::QueryTimeout, + PostgresOutcome::QueryError, + ] + .map(PostgresOutcome::label), + [ + "success", + "pool_timeout", + "pool_error", + "operation_timeout", + "operation_error", + ] + ); + assert_eq!( + [ + RedisOutcome::Success, + RedisOutcome::PoolTimeout, + RedisOutcome::PoolError, + ] + .map(RedisOutcome::label), + ["success", "pool_timeout", "pool_error"] + ); + assert_eq!( + [ + DeletionCatalogOutcome::Success, + DeletionCatalogOutcome::OperationTimeout, + DeletionCatalogOutcome::OperationError, + ] + .map(DeletionCatalogOutcome::label), + ["success", "operation_timeout", "operation_error"] + ); + assert_eq!(READINESS_RAW_SERIES_PER_POD, 99); + } + + #[test] + fn slow_older_failure_cannot_overwrite_newer_success_gauges() { + let coordinator = ReadinessCoordinator::default(); + let ProbeStart::Evaluate(slow_a) = coordinator.begin_probe() else { + panic!("serving probe A"); + }; + let ProbeStart::Evaluate(fast_b) = coordinator.begin_probe() else { + panic!("serving probe B"); + }; + let recorder = DebuggingRecorder::new(); + let snapshotter = recorder.snapshotter(); + + metrics::with_local_recorder(&recorder, || { + coordinator.finish_probe(fast_b, ready_evaluation()); + coordinator.finish_probe(slow_a, redis_failure_evaluation()); + }); + let snapshot = snapshotter.snapshot().into_vec(); + + assert_eq!(gauge_value(&snapshot, "overall"), 1.0); + assert_eq!(gauge_value(&snapshot, "redis"), 1.0); + assert!(matches!( + exact_metric( + &snapshot, + "buzz_readiness_checks_total", + &[("reason", "ready")] + ), + Some(DebugValue::Counter(1)) + )); + assert!(matches!( + exact_metric( + &snapshot, + "buzz_readiness_checks_total", + &[("reason", "redis_pool_timeout")] + ), + Some(DebugValue::Counter(1)) + )); + } + + #[test] + fn slow_older_success_cannot_overwrite_newer_failure_gauges() { + let coordinator = ReadinessCoordinator::default(); + let ProbeStart::Evaluate(slow_a) = coordinator.begin_probe() else { + panic!("serving probe A"); + }; + let ProbeStart::Evaluate(fast_b) = coordinator.begin_probe() else { + panic!("serving probe B"); + }; + let recorder = DebuggingRecorder::new(); + let snapshotter = recorder.snapshotter(); + + metrics::with_local_recorder(&recorder, || { + coordinator.finish_probe(fast_b, redis_failure_evaluation()); + coordinator.finish_probe(slow_a, ready_evaluation()); + }); + let snapshot = snapshotter.snapshot().into_vec(); + + assert_eq!(gauge_value(&snapshot, "overall"), 0.0); + assert_eq!(gauge_value(&snapshot, "postgres"), 1.0); + assert_eq!(gauge_value(&snapshot, "redis"), 0.0); + assert_eq!(gauge_value(&snapshot, "deletion_catalog"), 1.0); + } + + #[test] + fn shutdown_fast_path_preserves_dependency_state_and_histograms() { + let coordinator = ReadinessCoordinator::default(); + let recorder = DebuggingRecorder::new(); + let snapshotter = recorder.snapshotter(); + + metrics::with_local_recorder(&recorder, || { + let ProbeStart::Evaluate(ticket) = coordinator.begin_probe() else { + panic!("initial serving probe"); + }; + coordinator.finish_probe(ticket, ready_evaluation()); + coordinator.begin_shutdown(); + assert!(matches!( + coordinator.begin_probe(), + ProbeStart::ShuttingDown + )); + }); + let after = snapshotter.snapshot().into_vec(); + + for dependency in ["postgres", "redis", "deletion_catalog"] { + assert_eq!( + gauge_value(&after, dependency), + 1.0, + "shutdown must not fabricate {dependency} state" + ); + } + for check in ["overall", "postgres", "redis", "deletion_catalog"] { + assert!( + matches!( + exact_metric( + &after, + "buzz_readiness_check_duration_seconds", + &[("check", check)] + ), + Some(DebugValue::Histogram(values)) if values.len() == 1 + ), + "shutdown fast path must not add a {check} duration" + ); + } + assert_eq!(gauge_value(&after, "overall"), 0.0); + assert!(matches!( + exact_metric( + &after, + "buzz_readiness_checks_total", + &[("reason", "shutting_down")] + ), + Some(DebugValue::Counter(1)) + )); + } + + #[test] + fn shutdown_dominates_an_in_flight_success_without_resurrecting_gauges() { + let coordinator = ReadinessCoordinator::default(); + let ProbeStart::Evaluate(ticket) = coordinator.begin_probe() else { + panic!("serving probe"); + }; + let recorder = DebuggingRecorder::new(); + let snapshotter = recorder.snapshotter(); + + let response = metrics::with_local_recorder(&recorder, || { + coordinator.begin_shutdown(); + coordinator.finish_probe(ticket, ready_evaluation()) + }); + let snapshot = snapshotter.snapshot().into_vec(); + + assert_eq!(response.reason, ReadinessReason::ShuttingDown); + assert_eq!(gauge_value(&snapshot, "overall"), 0.0); + assert!( + exact_metric(&snapshot, "buzz_readiness_state", &[("check", "postgres")]).is_none() + ); + assert!(matches!( + exact_metric( + &snapshot, + "buzz_readiness_dependency_checks_total", + &[("dependency", "postgres"), ("outcome", "success")] + ), + Some(DebugValue::Counter(1)) + )); + } +} diff --git a/crates/buzz-relay/src/router.rs b/crates/buzz-relay/src/router.rs index dd0fde6fdc..61aedf70be 100644 --- a/crates/buzz-relay/src/router.rs +++ b/crates/buzz-relay/src/router.rs @@ -24,6 +24,7 @@ use crate::audio; use crate::connection::handle_connection; use crate::metrics::track_metrics; use crate::nip11::{nip11_document, relay_info_handler}; +use crate::readiness::{self, ReadinessEvaluation, ReadinessReason}; use crate::state::AppState; /// Build the axum [`Router`] with all relay routes, middleware, and CORS configuration. @@ -67,7 +68,7 @@ pub fn build_router(state: Arc) -> Router { // Health endpoints .route("/health", get(health_handler)) .route("/_liveness", get(liveness_handler)) - .route("/_readiness", get(readiness_handler)) + .route("/_readiness", get(public_readiness_handler)) // Nostr HTTP bridge (NIP-98 auth) .route("/events", post(api::bridge::submit_event)) .route("/query", post(api::bridge::query_events)) @@ -294,7 +295,7 @@ async fn admin_spa_document(state: &AppState, accept: &str) -> axum::response::R pub fn build_health_router(state: Arc) -> Router { Router::new() .route("/_liveness", get(liveness_handler)) - .route("/_readiness", get(readiness_handler)) + .route("/_readiness", get(kubernetes_readiness_handler)) .route("/_status", get(status_handler)) .route("/_mesh", get(mesh_status_handler)) .with_state(state) @@ -406,11 +407,36 @@ async fn liveness_handler() -> impl IntoResponse { (StatusCode::OK, "ok") } -/// Readiness probe — checks shutdown flag, Postgres, and Redis connectivity. -async fn readiness_handler(State(state): State>) -> impl IntoResponse { - use std::time::Duration; +/// Compatibility endpoint on the public listener. It evaluates dependencies +/// and preserves the existing response contract but never records rollout +/// telemetry. +async fn public_readiness_handler(State(state): State>) -> impl IntoResponse { + if !state.readiness.public_evaluation_allowed() { + return readiness_response(ReadinessEvaluation::shutting_down(), false); + } - if state.shutting_down.load(Ordering::Relaxed) { + let evaluation = state.readiness.evaluate(&state.db, &state.redis_pool).await; + let evaluation = state.readiness.finish_public_evaluation(evaluation); + readiness_response(evaluation, false) +} + +/// Kubernetes health-listener endpoint. All rollout metrics flow through the +/// process-owned coordinator so shutdown and probe generations are ordered. +async fn kubernetes_readiness_handler(State(state): State>) -> impl IntoResponse { + let readiness::ProbeStart::Evaluate(ticket) = state.readiness.begin_probe() else { + return readiness_response(ReadinessEvaluation::shutting_down(), true); + }; + + let evaluation = state.readiness.evaluate(&state.db, &state.redis_pool).await; + let evaluation = state.readiness.finish_probe(ticket, evaluation); + readiness_response(evaluation, true) +} + +fn readiness_response( + evaluation: ReadinessEvaluation, + include_reason: bool, +) -> axum::response::Response { + if evaluation.reason == ReadinessReason::ShuttingDown { return ( StatusCode::SERVICE_UNAVAILABLE, Json(json!({"status": "shutting_down"})), @@ -418,33 +444,23 @@ async fn readiness_handler(State(state): State>) -> impl IntoRespo .into_response(); } - let check = async { - let (pg_ok, redis_ok, deletion_catalog_ok) = tokio::join!( - state.db.ping(), - async { state.redis_pool.get().await.is_ok() }, - async { state.db.validate_deletion_serving_catalog().await.is_ok() }, - ); - (pg_ok, redis_ok, deletion_catalog_ok) - }; + let pg_ok = evaluation.postgres_ready(); + let redis_ok = evaluation.redis_ready(); + let deletion_catalog_ok = evaluation.deletion_catalog_ready(); - let (pg_ok, redis_ok, deletion_catalog_ok) = - tokio::time::timeout(Duration::from_secs(2), check) - .await - .unwrap_or((false, false, false)); - - if pg_ok && redis_ok && deletion_catalog_ok { + if evaluation.is_ready() { (StatusCode::OK, Json(json!({"status": "ready"}))).into_response() } else { - ( - StatusCode::SERVICE_UNAVAILABLE, - Json(json!({ - "status": "not_ready", - "postgres": pg_ok, - "redis": redis_ok, - "deletion_catalog": deletion_catalog_ok - })), - ) - .into_response() + let mut payload = json!({ + "status": "not_ready", + "postgres": pg_ok, + "redis": redis_ok, + "deletion_catalog": deletion_catalog_ok + }); + if include_reason { + payload["reason"] = json!(evaluation.reason.label()); + } + (StatusCode::SERVICE_UNAVAILABLE, Json(payload)).into_response() } } @@ -506,12 +522,17 @@ fn build_cors_layer(cors_origins: &[String]) -> CorsLayer { #[cfg(test)] mod tests { + use std::collections::VecDeque; + use std::sync::atomic::{AtomicUsize, Ordering as AtomicOrdering}; + use std::sync::{Mutex, PoisonError}; + use std::time::Duration; + use axum::{routing::get, Router}; use futures_util::SinkExt; use opentelemetry::trace::TracerProvider as _; use opentelemetry_sdk::trace::{InMemorySpanExporter, SdkTracerProvider}; use tokio::net::TcpListener; - use tokio::sync::mpsc; + use tokio::sync::{mpsc, Notify}; use tokio_tungstenite::{connect_async, tungstenite::Message}; use tower::ServiceBuilder; use tracing::Instrument as _; @@ -519,6 +540,98 @@ mod tests { use super::*; + struct ScriptedReadinessEvaluator { + evaluations: Mutex>, + } + + impl ScriptedReadinessEvaluator { + fn new(evaluations: impl IntoIterator) -> Self { + Self { + evaluations: Mutex::new(evaluations.into_iter().collect()), + } + } + + fn push(&self, evaluation: ReadinessEvaluation) { + self.evaluations + .lock() + .unwrap_or_else(PoisonError::into_inner) + .push_back(evaluation); + } + } + + #[async_trait::async_trait] + impl readiness::ReadinessEvaluator for ScriptedReadinessEvaluator { + async fn evaluate( + &self, + _db: &buzz_db::Db, + _redis_pool: &deadpool_redis::Pool, + ) -> ReadinessEvaluation { + self.evaluations + .lock() + .unwrap_or_else(PoisonError::into_inner) + .pop_front() + .expect("scripted readiness evaluation") + } + } + + struct BarrierReadinessEvaluator { + calls: AtomicUsize, + first_started: Notify, + release_first: Notify, + first: ReadinessEvaluation, + second: ReadinessEvaluation, + } + + impl BarrierReadinessEvaluator { + fn new(first: ReadinessEvaluation, second: ReadinessEvaluation) -> Self { + Self { + calls: AtomicUsize::new(0), + first_started: Notify::new(), + release_first: Notify::new(), + first, + second, + } + } + } + + #[async_trait::async_trait] + impl readiness::ReadinessEvaluator for BarrierReadinessEvaluator { + async fn evaluate( + &self, + _db: &buzz_db::Db, + _redis_pool: &deadpool_redis::Pool, + ) -> ReadinessEvaluation { + if self.calls.fetch_add(1, AtomicOrdering::SeqCst) == 0 { + self.first_started.notify_waiters(); + self.release_first.notified().await; + self.first + } else { + self.second + } + } + } + + fn readiness_evaluation( + postgres: readiness::PostgresOutcome, + redis: readiness::RedisOutcome, + deletion_catalog: readiness::DeletionCatalogOutcome, + ) -> ReadinessEvaluation { + ReadinessEvaluation::from_results( + readiness::TimedOutcome::new(postgres, Duration::from_millis(35)), + readiness::TimedOutcome::new(redis, Duration::from_millis(20)), + readiness::TimedOutcome::new(deletion_catalog, Duration::from_millis(15)), + Duration::from_millis(35), + ) + } + + fn ready_evaluation() -> ReadinessEvaluation { + readiness_evaluation( + readiness::PostgresOutcome::Success, + readiness::RedisOutcome::Success, + readiness::DeletionCatalogOutcome::Success, + ) + } + #[test] fn invite_landing_path_requires_exactly_one_nonempty_code_segment() { assert!(is_invite_landing_path("/invite/payload.mac")); @@ -594,6 +707,447 @@ mod tests { Arc::new(state) } + async fn readiness_state(evaluator: Arc) -> Arc { + let mut config = crate::config::Config::from_env().expect("default config loads"); + config.require_relay_membership = false; + config.database_url = "postgres://buzz:buzz_dev@127.0.0.1:1/buzz".to_string(); + config.redis_url = "redis://127.0.0.1:1".to_string(); + let pool = sqlx::PgPool::connect_lazy(&config.database_url).expect("lazy pg pool"); + let db = buzz_db::Db::from_pool(pool.clone()); + let redis_pool = deadpool_redis::Config::from_url(&config.redis_url) + .create_pool(Some(deadpool_redis::Runtime::Tokio1)) + .expect("redis pool"); + let pubsub = Arc::new( + buzz_pubsub::PubSubManager::new(&config.redis_url, redis_pool.clone()) + .await + .expect("pubsub manager"), + ); + let audit = buzz_audit::AuditService::new(pool.clone()); + let auth = buzz_auth::AuthService::new(config.auth.clone()); + let search = buzz_search::SearchService::new(pool.clone()); + let workflow_engine = Arc::new(buzz_workflow::WorkflowEngine::new( + db.clone(), + buzz_workflow::WorkflowConfig::default(), + )); + let media_storage = buzz_media::MediaStorage::new(&config.media).expect("media storage"); + let (mut state, _audit_shutdown) = AppState::new( + config, + db, + redis_pool, + audit, + pubsub, + auth, + search, + workflow_engine, + nostr::Keys::generate(), + media_storage, + ); + state.set_readiness_evaluator(evaluator); + Arc::new(state) + } + + async fn readiness_request(router: Router) -> (StatusCode, serde_json::Value) { + let response = router + .oneshot( + Request::get("/_readiness") + .body(Body::empty()) + .expect("readiness request"), + ) + .await + .expect("readiness response"); + let status = response.status(); + let body = axum::body::to_bytes(response.into_body(), 64 * 1024) + .await + .expect("readiness response body"); + let payload = serde_json::from_slice(&body).expect("readiness JSON"); + (status, payload) + } + + fn readiness_metric_lines(rendered: &str) -> Vec<&str> { + rendered + .lines() + .filter(|line| line.starts_with("buzz_readiness")) + .collect() + } + + fn sorted_readiness_metric_lines(rendered: &str) -> Vec { + let mut lines = readiness_metric_lines(rendered) + .into_iter() + .map(str::to_owned) + .collect::>(); + lines.sort(); + lines + } + + fn metric_value(rendered: &str, exact_prefix: &str) -> f64 { + rendered + .lines() + .find_map(|line| { + line.strip_prefix(exact_prefix) + .and_then(|value| value.strip_prefix(' ')) + .and_then(|value| value.parse().ok()) + }) + .unwrap_or_else(|| panic!("missing metric line: {exact_prefix}")) + } + + #[test] + fn production_readiness_routes_export_the_frozen_health_only_contract() { + let runtime = tokio::runtime::Builder::new_current_thread() + .enable_all() + .build() + .expect("current-thread runtime"); + let evaluator = Arc::new(ScriptedReadinessEvaluator::new(std::iter::repeat_n( + ready_evaluation(), + 4, + ))); + let (recorder, handle) = crate::metrics::readiness_test_recorder(); + + metrics::with_local_recorder(&recorder, || { + crate::metrics::describe_readiness_metrics(); + runtime.block_on(async { + let state = readiness_state(evaluator.clone()).await; + let public = build_router(state.clone()); + let health = build_health_router(state.clone()); + + for _ in 0..3 { + assert_eq!( + readiness_request(public.clone()).await, + (StatusCode::OK, json!({"status": "ready"})) + ); + } + assert!( + readiness_metric_lines(&handle.render()).is_empty(), + "public compatibility requests must emit no readiness series" + ); + + assert_eq!( + readiness_request(health.clone()).await, + (StatusCode::OK, json!({"status": "ready"})) + ); + let first_scrape = handle.render(); + + assert!(first_scrape.contains("# TYPE buzz_readiness_checks_total counter")); + assert!(first_scrape + .contains("# TYPE buzz_readiness_dependency_checks_total counter")); + assert!(first_scrape + .contains("# TYPE buzz_readiness_check_duration_seconds histogram")); + assert!(first_scrape.contains("# TYPE buzz_readiness_state gauge")); + assert_eq!( + metric_value( + &first_scrape, + "buzz_readiness_checks_total{reason=\"ready\"}" + ), + 1.0 + ); + assert_eq!( + metric_value( + &first_scrape, + "buzz_readiness_dependency_checks_total{dependency=\"postgres\",outcome=\"success\"}" + ), + 1.0 + ); + assert_eq!( + metric_value( + &first_scrape, + "buzz_readiness_state{check=\"overall\"}" + ), + 1.0 + ); + for bucket in ["2", "2.5", "+Inf"] { + assert!(first_scrape.contains(&format!( + "buzz_readiness_check_duration_seconds_bucket{{check=\"overall\",le=\"{bucket}\"}}" + ))); + } + assert!(!first_scrape.contains("result=")); + assert!(!first_scrape + .lines() + .filter(|line| line.starts_with("buzz_readiness_check_duration_seconds")) + .any(|line| line.contains("outcome="))); + + let before_public_failure = sorted_readiness_metric_lines(&first_scrape); + evaluator.push(readiness_evaluation( + readiness::PostgresOutcome::Success, + readiness::RedisOutcome::PoolTimeout, + readiness::DeletionCatalogOutcome::Success, + )); + assert_eq!( + readiness_request(public.clone()).await, + ( + StatusCode::SERVICE_UNAVAILABLE, + json!({ + "status": "not_ready", + "postgres": true, + "redis": false, + "deletion_catalog": true + }) + ) + ); + assert_eq!( + sorted_readiness_metric_lines(&handle.render()), + before_public_failure + ); + + let contract_evaluations = [ + readiness_evaluation( + readiness::PostgresOutcome::PoolTimeout, + readiness::RedisOutcome::Success, + readiness::DeletionCatalogOutcome::Success, + ), + readiness_evaluation( + readiness::PostgresOutcome::PoolError, + readiness::RedisOutcome::Success, + readiness::DeletionCatalogOutcome::Success, + ), + readiness_evaluation( + readiness::PostgresOutcome::QueryTimeout, + readiness::RedisOutcome::Success, + readiness::DeletionCatalogOutcome::Success, + ), + readiness_evaluation( + readiness::PostgresOutcome::QueryError, + readiness::RedisOutcome::Success, + readiness::DeletionCatalogOutcome::Success, + ), + readiness_evaluation( + readiness::PostgresOutcome::Success, + readiness::RedisOutcome::PoolTimeout, + readiness::DeletionCatalogOutcome::Success, + ), + readiness_evaluation( + readiness::PostgresOutcome::Success, + readiness::RedisOutcome::PoolError, + readiness::DeletionCatalogOutcome::Success, + ), + readiness_evaluation( + readiness::PostgresOutcome::Success, + readiness::RedisOutcome::Success, + readiness::DeletionCatalogOutcome::OperationTimeout, + ), + readiness_evaluation( + readiness::PostgresOutcome::Success, + readiness::RedisOutcome::Success, + readiness::DeletionCatalogOutcome::OperationError, + ), + readiness_evaluation( + readiness::PostgresOutcome::PoolTimeout, + readiness::RedisOutcome::PoolTimeout, + readiness::DeletionCatalogOutcome::OperationTimeout, + ), + readiness_evaluation( + readiness::PostgresOutcome::PoolError, + readiness::RedisOutcome::PoolError, + readiness::DeletionCatalogOutcome::Success, + ), + ]; + for evaluation in contract_evaluations { + evaluator.push(evaluation); + let (status, payload) = readiness_request(health.clone()).await; + assert_eq!(status, StatusCode::SERVICE_UNAVAILABLE); + assert_eq!(payload["reason"], json!(evaluation.reason.label())); + } + + let before_shutdown = handle.render(); + let histogram_counts_before = ["overall", "postgres", "redis", "deletion_catalog"] + .map(|check| { + metric_value( + &before_shutdown, + &format!( + "buzz_readiness_check_duration_seconds_count{{check=\"{check}\"}}" + ), + ) + }); + state.begin_shutdown(); + assert_eq!( + readiness_request(public).await, + ( + StatusCode::SERVICE_UNAVAILABLE, + json!({"status": "shutting_down"}) + ) + ); + let after_public_shutdown = handle.render(); + assert!(after_public_shutdown + .lines() + .all(|line| !line.contains("reason=\"shutting_down\""))); + + assert_eq!( + readiness_request(health).await, + ( + StatusCode::SERVICE_UNAVAILABLE, + json!({"status": "shutting_down"}) + ) + ); + let final_scrape = handle.render(); + let histogram_counts_after = ["overall", "postgres", "redis", "deletion_catalog"] + .map(|check| { + metric_value( + &final_scrape, + &format!( + "buzz_readiness_check_duration_seconds_count{{check=\"{check}\"}}" + ), + ) + }); + assert_eq!(histogram_counts_after, histogram_counts_before); + assert_eq!( + metric_value( + &final_scrape, + "buzz_readiness_checks_total{reason=\"shutting_down\"}" + ), + 1.0 + ); + assert_eq!( + metric_value( + &final_scrape, + "buzz_readiness_state{check=\"overall\"}" + ), + 0.0 + ); + assert!(!final_scrape.contains("sensitive-sql-or-url")); + + let exported_reasons = final_scrape + .lines() + .filter(|line| line.starts_with("buzz_readiness_checks_total{")) + .count(); + assert_eq!(exported_reasons, readiness::READINESS_REASON_LABELS.len()); + assert_eq!( + readiness_metric_lines(&final_scrape).len(), + readiness::READINESS_RAW_SERIES_PER_POD, + "readiness series contract must stay at or below its 99-series cap" + ); + }); + }); + } + + fn run_out_of_order_route_case( + first: ReadinessEvaluation, + second: ReadinessEvaluation, + ) -> (serde_json::Value, serde_json::Value, String) { + let runtime = tokio::runtime::Builder::new_current_thread() + .enable_all() + .build() + .expect("current-thread runtime"); + let evaluator = Arc::new(BarrierReadinessEvaluator::new(first, second)); + let (recorder, handle) = crate::metrics::readiness_test_recorder(); + + metrics::with_local_recorder(&recorder, || { + runtime.block_on(async { + let state = readiness_state(evaluator.clone()).await; + let health = build_health_router(state); + let first_started = evaluator.first_started.notified(); + let slow_first = tokio::spawn(readiness_request(health.clone())); + first_started.await; + + let (_, second_payload) = readiness_request(health).await; + evaluator.release_first.notify_one(); + let (_, first_payload) = slow_first.await.expect("slow first probe task"); + (first_payload, second_payload, handle.render()) + }) + }) + } + + #[test] + fn real_health_route_generation_fence_covers_both_completion_orders() { + let failure = readiness_evaluation( + readiness::PostgresOutcome::Success, + readiness::RedisOutcome::PoolTimeout, + readiness::DeletionCatalogOutcome::Success, + ); + + let (older_failure, newer_success, success_scrape) = + run_out_of_order_route_case(failure, ready_evaluation()); + assert_eq!(older_failure["reason"], json!("redis_pool_timeout")); + assert_eq!(newer_success, json!({"status": "ready"})); + assert_eq!( + metric_value(&success_scrape, "buzz_readiness_state{check=\"overall\"}"), + 1.0 + ); + assert_eq!( + metric_value(&success_scrape, "buzz_readiness_state{check=\"redis\"}"), + 1.0 + ); + + let (older_success, newer_failure, failure_scrape) = + run_out_of_order_route_case(ready_evaluation(), failure); + assert_eq!(older_success, json!({"status": "ready"})); + assert_eq!(newer_failure["reason"], json!("redis_pool_timeout")); + assert_eq!( + metric_value(&failure_scrape, "buzz_readiness_state{check=\"overall\"}"), + 0.0 + ); + assert_eq!( + metric_value(&failure_scrape, "buzz_readiness_state{check=\"redis\"}"), + 0.0 + ); + for scrape in [&success_scrape, &failure_scrape] { + assert_eq!( + metric_value(scrape, "buzz_readiness_checks_total{reason=\"ready\"}"), + 1.0 + ); + assert_eq!( + metric_value( + scrape, + "buzz_readiness_checks_total{reason=\"redis_pool_timeout\"}" + ), + 1.0 + ); + } + } + + #[test] + fn real_health_route_shutdown_fence_dominates_an_in_flight_success() { + let runtime = tokio::runtime::Builder::new_current_thread() + .enable_all() + .build() + .expect("current-thread runtime"); + let evaluator = Arc::new(BarrierReadinessEvaluator::new( + ready_evaluation(), + ready_evaluation(), + )); + let (recorder, handle) = crate::metrics::readiness_test_recorder(); + + metrics::with_local_recorder(&recorder, || { + runtime.block_on(async { + let state = readiness_state(evaluator.clone()).await; + let health = build_health_router(state.clone()); + let first_started = evaluator.first_started.notified(); + let in_flight = tokio::spawn(readiness_request(health)); + first_started.await; + + state.begin_shutdown(); + evaluator.release_first.notify_one(); + assert_eq!( + in_flight.await.expect("in-flight readiness task"), + ( + StatusCode::SERVICE_UNAVAILABLE, + json!({"status": "shutting_down"}) + ) + ); + + let scrape = handle.render(); + assert_eq!( + metric_value(&scrape, "buzz_readiness_state{check=\"overall\"}"), + 0.0 + ); + assert!(scrape + .lines() + .all(|line| !line.starts_with("buzz_readiness_state{check=\"postgres\"}"))); + assert_eq!( + metric_value( + &scrape, + "buzz_readiness_checks_total{reason=\"shutting_down\"}" + ), + 1.0 + ); + assert_eq!( + metric_value( + &scrape, + "buzz_readiness_dependency_checks_total{dependency=\"postgres\",outcome=\"success\"}" + ), + 1.0 + ); + }); + }); + } + /// A minimal built SPA: an index document, one hashed asset, and the /// root-level favicon Vite copies out of `public/`. fn write_bundle(dir: &std::path::Path) { diff --git a/crates/buzz-relay/src/state.rs b/crates/buzz-relay/src/state.rs index 693f6c7a9b..47665db677 100644 --- a/crates/buzz-relay/src/state.rs +++ b/crates/buzz-relay/src/state.rs @@ -713,6 +713,8 @@ pub struct AppState { pub audio_rooms: Arc, /// Set to `true` on SIGTERM — readiness probe returns 503. pub shutting_down: Arc, + /// Orders readiness gauge publication against terminal shutdown. + pub(crate) readiness: Arc, /// Process start time — used by `/_status` endpoint. pub started_at: Instant, /// Shared, community-scoped NIP-98 replay prevention. @@ -914,6 +916,7 @@ impl AppState { git_pack_cache, audio_rooms: Arc::new(AudioRoomManager::new()), shutting_down: Arc::new(AtomicBool::new(false)), + readiness: Arc::new(crate::readiness::ReadinessCoordinator::default()), started_at: Instant::now(), nip98_replay, gif_http_client, @@ -955,6 +958,23 @@ impl AppState { ) } + /// Atomically closes readiness publication before exposing shutdown to + /// the relay's other fast-path lifecycle checks. + pub fn begin_shutdown(&self) { + self.readiness.begin_shutdown(); + self.shutting_down.store(true, Ordering::Release); + } + + #[cfg(test)] + pub(crate) fn set_readiness_evaluator( + &mut self, + evaluator: Arc, + ) { + self.readiness = Arc::new(crate::readiness::ReadinessCoordinator::with_evaluator( + evaluator, + )); + } + /// Inter-relay mesh handle. `None` ⇒ mesh-off / single-instance: callers /// must no-op to today's behavior. Set once by `main.rs` after boot. pub fn mesh(&self) -> Option<&crate::mesh_boot::MeshHandle> { diff --git a/deploy/charts/buzz/README.md b/deploy/charts/buzz/README.md index 8a4b0c6d66..b022dd8a27 100644 --- a/deploy/charts/buzz/README.md +++ b/deploy/charts/buzz/README.md @@ -115,6 +115,27 @@ disables that probe through `relay.extraEnv`, `/_readiness` does not test object storage; configuration is still parsed strictly, but reachability and addressing errors surface on the first storage operation. +### Readiness telemetry contract + +Only requests served by the private health listener (`BUZZ_HEALTH_PORT`) emit +rollout readiness telemetry. The compatibility `/_readiness` route on the public +app listener returns health but does not change these metrics. + +| Metric | Type | Labels | +|--------|------|--------| +| `buzz_readiness_checks_total` | counter | `reason` from the closed readiness-reason set | +| `buzz_readiness_dependency_checks_total` | counter | `dependency`, typed bounded `outcome` | +| `buzz_readiness_check_duration_seconds` | histogram | `check` only | +| `buzz_readiness_state` | gauge | `check` only; latest publishable generation | + +The schema has a ceiling of 99 raw Prometheus series per pod: 12 overall +reasons, 11 valid dependency/outcome pairs, 72 histogram series, and 4 gauges. +Do not add pod, ReplicaSet, version, rollout, error text, SQL, URL, tenant, +user, community, pubkey, header, query, or other request-controlled labels. +Shutdown without dependency evaluation increments only +`buzz_readiness_checks_total{reason="shutting_down"}` and sets the overall +state to zero; it does not fabricate dependency failures or latency samples. + ## Relay Pod extensions The chart exposes narrow extension points for init containers, volumes, relay From d4420eb47f9d48bd81358f928dfe6dee6804ac13 Mon Sep 17 00:00:00 2001 From: Will Pfleger Date: Tue, 1 Sep 2026 19:05:39 -0400 Subject: [PATCH 3/5] docs(nip-fi): rewrite NIP-FI as stateless OSS Buzz spec v2 (#7214) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rewrites `docs/nips/NIP-FI.md` as a stateless spec and deletes the five companion profiles (NIP-FI-CONF, NIP-FI-DELEG, NIP-FI-EDGE, NIP-FI-LIFECYCLE, NIP-FI-MODEL) that presuppose relay-side state. ## What changes ### `docs/nips/NIP-FI.md` — full rewrite The relay-side authority engine (bindings, receipts, lifecycle, invalidation, leases, delegation, enrollment modes, TOFU) is removed. The spec settles exactly five things: 1. **Assertion contract**: JWT claims (`iss`, `sub`, `nostr_pubkey`, `aud` (required), `iat`/`exp`, finite TTL bounds), npub binding semantics, required NIP-42 proof-of-key pairing. Evidence rules (assertion-key-mismatch → `authorization_denied`, freshness) preserved from v1. 2. **Verification**: offline against configured per-issuer JWKS snapshots; multi-issuer; fail-closed on missing/expired/unverifiable. Cites `crates/buzz-auth/src/nip_fi/` (PR 3 / `70895b355`) as implementing the assertion-verification procedure. The `require_attested_key` flag in `IssuerPolicy` is the enforcement primitive for the unconditional `nostr_pubkey` requirement; conformance requires forcing it true for every issuer — follow-on code outside this PR. 3. **Session policy**: required finite `max_connection_lifetime_seconds` knob with no permissive default; re-auth on every reconnect; no in-band renewal. 4. **Admin disconnect API**: session-only normative text with a dedicated `VerifyCommandJwt` procedure — distinct `typ: nip-fi-command+jwt`, signed `method`/`path` binding, normative `maximum_command_age` bound (0 < value ≤ 60 s), atomic `(iss, jti)` reservation as the final admission step after all pure checks. Non-normative note documents cumulative residual access: `max(0, min(exp, iat + maximum_assertion_age) - now)` assuming issuance stops; states that previously issued assertions remain valid for reconnection until their authority expires; states that continued issuance extends access with no protocol-level bound. Non-normative note documents the session-only vs deny-until-TTL trade-off. This verifier and endpoint are follow-on code. 5. **Deletions**: bindings, enrollment, lifecycle, receipts, leases, invalidation, delegation gone. "Out of scope" section names every issuer and deployment concern explicitly. ### Companion profiles deleted NIP-FI-CONF, NIP-FI-DELEG, NIP-FI-EDGE, NIP-FI-LIFECYCLE, NIP-FI-MODEL — all presuppose relay-side binding/lifecycle state. ## What does NOT change The entire `crates/buzz-auth/src/nip_fi/` Rust crate (PR 3 / `70895b355`) implements the assertion-verification procedure described in this spec. No Rust code changes in this PR. The `require_attested_key` enforcement integration and the `VerifyCommandJwt` / disconnect endpoint implementation are explicit follow-on code work. ## Follow-on work (separate PRs after spec merges) - `require_attested_key` forced-true startup enforcement - `VerifyCommandJwt` implementation and disconnect endpoint - Down-migration dropping migrations 0041/0042 (0042 before 0041 per FK order) - `schema.sql` NIP-FI sections removed - `buzz-db` migration test blocks cleaned up --------- Signed-off-by: Will Pfleger Co-authored-by: Duncan --- docs/nips/NIP-FI-CONF.md | 375 ------------- docs/nips/NIP-FI-DELEG.md | 186 ------- docs/nips/NIP-FI-EDGE.md | 417 -------------- docs/nips/NIP-FI-LIFECYCLE.md | 269 --------- docs/nips/NIP-FI-MODEL.md | 96 ---- docs/nips/NIP-FI.md | 995 ++++++++++++++++------------------ 6 files changed, 465 insertions(+), 1873 deletions(-) delete mode 100644 docs/nips/NIP-FI-CONF.md delete mode 100644 docs/nips/NIP-FI-DELEG.md delete mode 100644 docs/nips/NIP-FI-EDGE.md delete mode 100644 docs/nips/NIP-FI-LIFECYCLE.md delete mode 100644 docs/nips/NIP-FI-MODEL.md diff --git a/docs/nips/NIP-FI-CONF.md b/docs/nips/NIP-FI-CONF.md deleted file mode 100644 index c6ba387977..0000000000 --- a/docs/nips/NIP-FI-CONF.md +++ /dev/null @@ -1,375 +0,0 @@ -NIP-FI-CONF -=========== - -Conformance evidence profile ----------------------------- - -`draft` `optional` - -**Dependencies**: NIP-FI core. Applies additionally to any claimed -NIP-FI-EDGE, NIP-FI-LIFECYCLE, and NIP-FI-DELEG profile. - -The key words "MUST", "MUST NOT", "REQUIRED", "SHOULD", "SHOULD NOT", and -"MAY" in this document are to be interpreted as described in BCP 14 (RFC 2119 -and RFC 8174) when, and only when, they appear in all capitals. - -## Abstract - -NIP-FI core and its profiles state required behavior. This profile states what -counts as evidence that an implementation has it: the claim unit, the evidence -rules, the complete denial-fixture enumeration, mutation adequacy, and the -interoperability exit test. - -This profile is separately claimable and is never advertised in discovery: -conformance is a property of a reviewed revision, not a wire feature. It -defines no wire behavior, denial mapping, invariant, or admission rule; where -it names one, NIP-FI core or the owning profile is normative. - -## Claim unit - -A conformance claim names exactly one immutable tuple: - -```text -(implementation revision, - adapter revision, - build artifact digest, - deployment revision, - governing document revision, - exit fixture digest, - claimed profiles, - assertion_policy_id, - transport_contract_id, - enrollment mode) -``` - -Changing any element creates a new claim. Results from one tuple MUST NOT be -carried into another. A report contains every applicable oracle from core and -every claimed profile exactly once, with status `pass` or `not-applicable` -only, except that `FI-CONF-INTEROP-EXIT` alone may instead carry `deferred` -under the condition in **Interoperability exit test**. Blank, skipped, -expected-failure, and not-run results cannot support a claim -(`FI-CONF-CLAIM-COMPLETE`). - -Enrollment mode is part of the claim unit and is private: it is recorded in -the access-controlled report, never in discovery or any public artifact. - -## Evidence rules - -Each passing oracle records the claim tuple, a stable test identifier and -adapter entry point, the command with start time, end time, exit status, and -any random seed, the synthetic input or a privacy-safe digest of it, the -before-and-after authoritative state relevant to the oracle, the expected and -observed outcomes, and artifact locations with SHA-256 digests. Stateful -oracles use an isolated database or namespace and inspect committed state -rather than inferring it from a response. Concurrency oracles record every -contender and the single serialized outcome. Time-boundary oracles use a -controlled clock. - -Adapters MUST drive public or production-equivalent entry points. A storage -helper MAY inspect state or inject a dependency outage; it MUST NOT replace the -operation under test. Calling an internal authorization function without -traversing the protected ingress does not satisfy ingress coverage. - -None of the following satisfies any oracle: searching source, documentation, -schemas, or binaries for a token; asserting that a route calls a named -function; recording a test name without its execution result; using a mock to -prove a deployed network boundary; citing a check from another revision; or -marking an oracle passed because the feature is configured. - -`FI-TRACE-TOFU-THEFT` takes an access-controlled **configuration** witness -only; under the private-posture rule no discovery witness for enrollment mode -can exist. Discovery invariance is proved separately by -`FI-TRACE-DISCOVERY-PRIVATE`. - -Requirements marked `[deployment artifact: ...]` in core or a profile are -evidenced by the named access-controlled review record at the claimed -deployment revision, not by a behavioral oracle. A claim listing an artifact -without the record is incomplete. - -Reports and artifacts hold private deployment detail and MUST remain access -controlled. They MUST NOT enter public reports, examples, discovery, or -protocol output, and MUST NOT contain raw assertions, secrets, or unredacted -`iss`, `sub`, or claim values. The shared exit fixture is exempt: its values -are synthetic by construction and name no real principal, issuer, or key. - -## Denial fixtures - -`FI-TRACE-DENIAL-ORACLE` requires one fixture per **private condition**, not -one per public class; a per-class suite compares a class against itself. The -enumeration below is the required fixture set (`FI-CONF-DENIAL-FIXTURES`). The -public-class column restates NIP-FI core, which owns the mapping and the bytes. - -| # | Private condition | Public class | Defined by | -|---|---|---|---| -| 1 | assertion, proof, or delegation evidence absent | `missing_evidence` | core | -| 2 | edge provenance absent or incomplete on an edge-required route (assertion may be present) | `missing_evidence` | NIP-FI-EDGE | -| 3 | evidence present but rejected: signature, key selection, issuer, audience, time, size, ambiguity, token class, body binding, or edge provenance (present but rejected) | `evidence_rejected` | core, NIP-FI-EDGE | -| 4 | replayed evidence — committed replay identity already claimed | `authorization_denied` | core, NIP-FI-EDGE | -| 5 | `key_mismatch` — asserted key is not the proven actor | `authorization_denied` | core | -| 6 | `attestation_required` — attested-key enrollment without a matching key claim | `authorization_denied` | core | -| 7 | `binding_conflict` — either side of the active relation is taken | `authorization_denied` | core | -| 8 | `pair_retired` | `authorization_denied` | core | -| 9 | `key_revoked` | `authorization_denied` | core | -| 10 | `policy_denied` — local operation policy | `authorization_denied` | core | -| 11 | `binding_required` — enrollment policy creates no binding at this request: provisioned mode with no binding, or any unrecognized policy value | `authorization_denied` | core | -| 12 | `identity_disabled` | `authorization_denied` | NIP-FI-LIFECYCLE | -| 13 | `explicit_replacement_required` — pending lineage | `authorization_denied` | NIP-FI-LIFECYCLE | -| 14 | `binding_expired` — administrative expiry | `authorization_denied` | NIP-FI-LIFECYCLE | -| 15 | `delegation_not_current` — owner or relationship no longer current | `authorization_denied` | NIP-FI-DELEG | -| 16 | `dependency_unreadable` | `authorization_unavailable` | core | - -Private-condition names are fixture identifiers, not wire values; a deployment -MAY use other internal reason codes if every enumerated condition has a -fixture. Rows for an unclaimed profile are `not-applicable` with absence -evidence. A profile that introduces a private condition MUST add its row; an -unenumerated condition escapes this oracle entirely. - -**Enumeration agreement.** `policy_denied` and `dependency_unreadable` are the -*prose-only allowlist*: core conditions that core states in prose and does not -name symbolically. The suite MUST check mechanically at the claimed head, by -symbol and never by row number, and every check MUST be green on the unmutated -documents before any mutant is scored: - -1. every symbol core denies by name has a row here attributed to core with the - same public class; -2. the set of symbols in core-attributed rows equals core's symbolic denial set - together with the allowlist, exactly, and the allowlist is disjoint from - that set; and -3. for each claimed profile that owns a private-denial-condition table, the - set of `(identifier, public class)` pairs in that table equals the set of - pairs attributed to that profile here, exactly; a row with multiple owners - contributes its pair to each. - -If a later core names an allowlisted symbol, check 2's disjointness fails -until the allowlist entry is deleted, and check 1 validates the promoted -symbol's class. - -**Anonymity comparison.** Every `authorization_denied` row is in the -private-state anonymity set. Between two private conditions on one -implementation, every response byte as transmitted MUST agree — transfer -framing included — except values a server cannot hold constant across two -instants, such as `Date`. This is wider than the interoperability object -below: within one implementation, any byte that varies by private condition is -a disclosure, whatever field it sits in. - -**Interoperability compared object.** Between two implementations, comparison -is over what core pins and nothing more. Over Nostr: the complete relay message -excluding only the event or subscription identifier echoed from the request, -as compact JSON with no insignificant whitespace per NIP-01. Over HTTP: the -status code; the content per RFC 9110 Section 6.4, after transfer decoding with -chunk framing and trailers excluded; and the exact values of only the header -fields core's denial table names, field names matched case-insensitively per -RFC 9110 Section 5.1. `Content-Length` is not pinned. Header order and unnamed -fields are outside the object, and their values MUST NOT depend on the private -condition. A field core names that an implementation cannot hold constant MUST -be reported with the reason, and its value MUST be independent of the private -condition. If core later pins another field, it joins with no edit here. - -**Run discipline.** The oracle runs a fixed positive iteration count on a -pinned isolated runner at the exact claimed head. Before the run the operator -records the environment, public-response corpus, bounds, sampling method, -statistical rule, noise treatment, and acceptance threshold. A breach fails the -gate, MUST NOT trigger an automatic retry, and is retained and investigated -before a separately authorized rerun. - -`authorization_unavailable` is observably distinct from `authorization_denied`. -This is accepted residual: it discloses no per-principal state, and collapsing -it would make fail-closed behavior undiagnosable. - -**Negative control.** The suite MUST include an implementation deliberately -patched to vary its denial response by private condition, and it MUST fail -this oracle. - -## Mutation adequacy - -An oracle that cannot fail is untested text that reads as tested. The -denominator is the **listed oracle**: every table row whose first cell names -exactly one complete literal oracle identifier, in NIP-FI core, in each claimed -normative profile, and in this document when CONF is claimed — selected by -that cell, not by section title. It is not the set of normative sentences, RFC -2119 keywords, or invariant labels, none of which two readers enumerate alike. - -For each listed oracle the suite MUST retain at least one **mutant**: an -implementation variant that violates a requirement that oracle governs, -together with that oracle's failing output (`FI-CONF-MUTATION`). Evidence is -the exact patch identity, the oracle identifier, and the retained failure -output at the claimed head. For this document's own oracles the implementation -under test includes the conformance suite and its report; a mutant is a single -variant of the suite or report that the entry's own oracle rejects. - -While `FI-CONF-INTEROP-EXIT` is validly deferred it remains in claim -completeness but is excluded from this section's mutation and global-control -obligations, since its failing output cannot exist without the run. Both -obligations attach with the run and MUST be discharged before either -implementation's interoperable conformance claim is accepted. No other -oracle's obligation under this section is deferrable. - -Normative prose outside the oracle tables remains binding but is not a second -denominator. Prose that no listed oracle can detect is untestable text: add the -oracle that detects it, or delete it. - -1. **One at a time.** Mutants are applied singly against an otherwise - unmodified implementation, so layered defenses cannot mask each other. -2. **Attribution.** The kill MUST come from the entry's own oracle. A mutant - killed only by another oracle establishes coverage for neither. -3. **One entry per mutant.** A mutant satisfies only the entry it was selected - for, even when it also kills other oracles. -4. **Reachability.** The suite MUST witness that a fixture reaches the mutated - decision, not merely the enclosing operation. -5. **Survivors are recorded.** A mutant its named oracle fails to kill is a - defect in the specification or the suite. It is recorded with that - disposition and MUST NOT be waived or replaced by an easier mutant. - -Two global controls bound the suite. A deny-everything implementation MUST -fail every positive oracle; an allow-everything implementation MUST fail every -negative oracle. Neither substitutes for per-entry mutants. - -## Interoperability exit test - -A claim of core conformance requires evidence that the documents alone are -sufficient to build against (`FI-CONF-INTEROP-EXIT`). Two implementations that -have not shared code and have not consulted a common reference implementation -each produce, from NIP-FI core and any claimed profile documents alone: - -- one valid `client-attached` request, over WebSocket upgrade and over HTTP, - compared over its signing inputs as defined below; and -- one byte-exact public denial response for each of the four public classes, - on each transport where the class can be decided, compared over the - interoperability compared object under **Denial fixtures**. - -Independence is a claim about code, not inputs: two implementations given -different issuers, keys, or clocks cannot produce equal bytes. The run is -therefore parameterized by a **shared exit fixture** that both sides load and -neither side authors: - -- one issuer identity and one JWK set, including the private key needed to - mint assertions and the `kid` selecting it; -- one assertion per denial class and one for the valid request, each as - complete pre-signature protected-header and claim-set JSON values — - including `alg`, `typ`, `kid`, every member the policy allows, and fixed - `iss`, `sub`, `aud`, `nostr_pubkey`, `client_id`, `iat`, `exp`, and token - class; -- one Nostr secret key for the proof, with the complete unsigned event fields - for each transport — the NIP-98 event over HTTP and the NIP-42 event with - its challenge and relay values over the WebSocket upgrade — including - `created_at`; -- one frozen evaluation instant, and the skew and lifetime bounds in force; - and -- the domain, target resource, operation, and enrollment policy for each case. - -The canonical fixture is authored by this document's editors, not by any -claiming implementation, and MUST be published as a single file at -`docs/nips/fixtures/nip-fi-conf-exit.json` in the same repository as these -documents, with its SHA-256 digest, before any `FI-CONF-INTEROP-EXIT` run. -Both sides MUST load that file, MUST verify the digest before the run, and -MUST record the digest with the evidence; a run against any other fixture -instance is not `FI-CONF-INTEROP-EXIT` evidence. While the canonical fixture -is unpublished, the claim tuple's exit fixture digest records the reserved -value `pending-canonical-fixture`, valid only in a claim whose -`FI-CONF-INTEROP-EXIT` result is `deferred`. Publication changes the element -and therefore creates a new claim. - -**Request compared object.** Signature octets are excluded, because conforming -implementations need not agree on them (randomized `ES256` and fresh-aux -BIP-340 do not) and no document here pins JWS or JSON member order. The -compared object is the **signing inputs**: for each transport's Nostr proof, -the NIP-01 serialization the event id is taken over, compared against its own -transport's serialization; for the assertion, the decoded protected header and -claim set compared as JSON values with member order excluded. Every value the -compared object depends on MUST be pinned in the fixture. - -The exchanged artifact per case is the complete request and response frame on -each transport — for HTTP the request line, headers, and body and the response -status, headers, and body; for Nostr the complete client and relay messages — -so that a mismatch can be explained from fields outside the compared object. - -The test passes when outputs compare equal over their compared objects and -each implementation accepts the other's valid request and reproduces the -other's denials. Exit evidence includes the exchanged artifacts and each -implementation's statement of independence. A divergence traced to an -underspecified value is a defect in the specification, not in either -implementation, and is fixed there. - -**Negative control.** One implementation is patched to emit a denial that -differs from the other only outside the compared object — a header core does -not name, or reordered fields — and the run MUST still pass. A run that fails -this control is comparing more than core pins; the exit test is then the -defect. The control is retained with the evidence. - -`FI-CONF-INTEROP-EXIT` is REQUIRED only once a second implementation meeting -the independence conditions exists. Until then a conformance claim MUST record -it as deferred with the machine-readable reason -`no-independent-implementation`. A deferred exit test MUST be run and passed -before the second implementation's conformance claim is accepted, and the -first implementation's claim MUST be re-evidenced against that run. - -## Applicability - -`not-applicable` requires a machine-readable reason and behavioral proof that -the surface is absent: - -- edge oracles only when no trusted-edge profile is accepted, none is - advertised, and executable cases reject every trusted-edge evidence shape; -- snapshot-rotation oracles only when no local key or status snapshot source - is configured and executable evidence proves the absence; -- `FI-TRACE-TOFU-THEFT` only when TOFU is neither configurable nor configured - and executable first-use cases deny; -- `FI-TRACE-CURRENT-STATUS-STALE` and `FI-TRACE-CURRENT-STATUS-REVOKED` only - when every configured assertion policy declares freshness class - `offline-jwt` and executable cases prove a presented witness is never - consulted; -- `FI-TRACE-CAPABILITY-REVOCATION` only when no external capability - projection requiring a declared revocation bound is configured, and - executable evidence proves no assertion capability or local-policy value - claims such a bound; -- lifecycle and delegation oracles only when the profile is unclaimed, - disabled, and denied on every ingress; and -- every other oracle is required for an enforcing deployment. - -An implementation that supports an optional surface runs its oracles even when -one deployed domain does not activate it. - -## Release gate - -Before NIP-FI enforcement or discovery is enabled, reviewers verify, at one -reviewed revision, that: - -- one immutable claim tuple passes every applicable oracle other than a - validly deferred `FI-CONF-INTEROP-EXIT`; -- if the canonical fixture was published before the review, the tuple's exit - fixture digest is not `pending-canonical-fixture`; -- the protected-ingress inventory has no uncovered or competing authority; -- every listed oracle, other than a validly deferred `FI-CONF-INTEROP-EXIT`, - has a killed, attributed, reachable mutant and every survivor is recorded; -- the denial-fixture enumeration is complete for the claimed profiles and its - negative control fails as required; -- the interoperability exit test has passed against an independent - implementation, or is recorded as deferred because none exists; -- every named deployment artifact exists at the claimed deployment revision; - and -- public and operational sinks pass privacy-canary inspection. - -Documentation review, source review, and static scans are review inputs. They -close no item in this gate. - -## Behavioral oracles - -| ID | Required outcome | -|---|---| -| `FI-CONF-CLAIM-COMPLETE` | A report missing an applicable oracle, duplicating one, carrying a result from another claim tuple, claiming a status other than `pass`/`not-applicable` — or `deferred` on any oracle other than `FI-CONF-INTEROP-EXIT` — or omitting mutant evidence for any oracle other than a deferred `FI-CONF-INTEROP-EXIT`, or recording the exit fixture digest `pending-canonical-fixture` with any `FI-CONF-INTEROP-EXIT` result other than `deferred`, is rejected. | -| `FI-CONF-DENIAL-FIXTURES` | Every enumerated private condition has a fixture; core and each claimed profile pass exact identifier/class/owner enumeration agreement; anonymity-set responses compare byte-identical; the distinguishing negative control fails. | -| `FI-CONF-MUTATION` | Every listed oracle — except `FI-CONF-INTEROP-EXIT` while validly deferred, per **Mutation adequacy** — has a singly-applied, attributed, reachability-witnessed mutant killed by that entry's own oracle; the deny-everything and allow-everything global controls fail every oracle **Mutation adequacy** requires of them, with retained evidence; survivors are recorded, not waived. | -| `FI-CONF-INTEROP-EXIT` | Two independent implementations produce, from the documents alone, valid requests equal over the request compared object and per-class denials equal over the denial compared object, and accept each other's output. | - -## Security considerations - -Conformance evidence is a privileged artifact: it enumerates private denial -conditions, enrollment posture, and deployment topology that the protocol -deliberately keeps off the wire. Publishing a report, a fixture corpus, or a -mutant catalogue would disclose exactly what `FI-INV-13` and -`FI-TRACE-DISCOVERY-PRIVATE` protect. - -A passing suite bounds the behaviors it exercises and nothing else. Mutation -adequacy raises the cost of a masked defect; it does not prove absence of -defects, and a claim that cites this profile as proof of security rather than -of tested behavior is misusing it. diff --git a/docs/nips/NIP-FI-DELEG.md b/docs/nips/NIP-FI-DELEG.md deleted file mode 100644 index 9b9134c2b3..0000000000 --- a/docs/nips/NIP-FI-DELEG.md +++ /dev/null @@ -1,186 +0,0 @@ -NIP-FI-DELEG -============ - -Delegated agent authorization profile --------------------------------------- - -`draft` `optional` `relay` - -**Protocol dependency**: NIP-FI core. - -The key words "MUST", "MUST NOT", "REQUIRED", "SHOULD", "SHOULD NOT", and -"MAY" in this document are to be interpreted as described in BCP 14 (RFC 2119 -and RFC 8174) when, and only when, they appear in all capitals. - -## Scope - -This profile authorizes a delegate key from separately validated delegation -evidence rooted in a currently eligible NIP-FI owner binding. The delegate -proves its own key. It does not present a federated assertion and never receives -or inherits the owner's binding. Because a trusted edge inserts assertion and -provenance fields on every request it forwards, and `FI-DELEG-PATH-SEPARATION` -denies any such field on a delegated request, delegated requests cannot traverse -a route that requires edge provenance; they use ingress on which NIP-FI-EDGE is -not required. - -This profile defines the normalized delegation result and its additional -preparation, final-admission, and lease witnesses. It does not define a wire -format for creating delegation relationships; NIP-OA or another protocol may -supply the evidence if it satisfies this contract. - -## Delegation evidence - -A validator returns this closed result: - -```text -DelegationEvidence = ( - domain, - owner_key, - delegate_key, - relationship_id, - relationship_revision, - audience, - operations, - conditions, - resource_or_target, - not_before?, - mandatory_expiry -) -``` - -`relationship_id` and `relationship_revision` are deployment-local dependency -identifiers. All other fields are interoperability-critical in meaning even -when their concrete encoding belongs to the supplying delegation protocol. - -The evidence authenticates every field, has one unambiguous owner and delegate, -matches the server-owned domain and exact request or target, and has a finite -expiry satisfying `now < mandatory_expiry`; equality at an expiry is expired. -Optional `not_before` satisfies `not_before <= now + skew`, using the -configured delegated `skew`; arithmetic is overflow-safe. A missing -configured `skew` denies. The proven actor equals `delegate_key`. -[FI-DELEG-EVIDENCE-CLOSED] - -A delegated request carries fresh request-appropriate Nostr proof and no -`Nostr-Federated-Identity` or profile provenance field. Mixed direct and -delegated evidence denies rather than selecting a path. [FI-DELEG-PATH-SEPARATION] - -## Private denial conditions - -This profile defines exactly this private condition identifier and owning public -class for NIP-FI-CONF enumeration agreement: - -| Private condition identifier | Public class | -|---|---| -| `delegation_not_current` | `authorization_denied` | - -The identifier is a fixture name, not a wire value. Adding, removing, renaming, -or reclassifying it requires the same change in NIP-FI-CONF's denial-fixture -table. - -## Preparation - -Preparation resolves the exact server-owned domain, target context, operation, -resource, and proven delegate actor before validating delegation evidence. It -then atomically reads: - -- the active owner binding and exact binding version; -- every owner tombstone, key-revocation, administrative, and profile lifecycle - gate applicable to that binding; -- the exact relationship identifier and revision; -- current local policy and resource versions; and -- every invalidation dependency and deadline. - -The owner binding is current and authorization-eligible at preparation. A -cached owner lease is not authority. The requested capability is the -intersection of the delegation's operation, audience, conditions, and target -with current local policy; an unsupported operation or empty intersection -denies. [FI-DELEG-OWNER-CURRENT] - -Preparation remains read-only under `FI-INV-08`. It cannot create or change an -owner or delegate binding, identity, provenance, lifecycle fact, relationship, -last-seen value, replay claim, receipt, lease, or application effect. -[FI-DELEG-NO-BINDING] - -## Final admission - -Core final admission additionally requires: - -1. the exact delegation evidence and delegate proof remain live; -2. domain, actor, target, audience, operation, resource, and relationship match - the prepared value; -3. the exact current owner binding and binding version remain eligible; -4. relationship identity and revision remain current; -5. current capability intersection equals the prepared intersection; and -6. changed dependencies are reread and the complete delegated decision is - recomputed before atomic commit. - -Any mismatch, expiry, owner retirement, owner key revocation, owner binding -version change, relationship change, unreadable dependency, or unsupported -capability denies. Rotation makes the former owner key non-current; its -relationships do not transfer to the new key. [FI-DELEG-OWNER-CURRENT] - -The delegated path creates no owner or delegate binding and cannot consume an -enrollment opportunity. Its receipt identifies the delegate actor and exact -owner-binding and relationship dependencies without publishing identity -material. [FI-DELEG-NO-BINDING] - -## Delegated leases - -A deployment configures a positive finite delegated maximum and a non-negative -finite delegated `skew`. The lease deadline is no later than the minimum of: - -- delegation expiry; -- delegate proof or connection bound; -- owner binding administrative bound, when applicable; -- current relationship bound; -- local policy bound; -- the lease issue instant plus the configured delegated maximum; and -- any stronger owner-assertion bound the deployment requires. - -Missing finite configuration denies. Equality is expired and arithmetic is -overflow-safe. [FI-DELEG-LEASE-BOUND] - -Before each protected use, the service checks the delegate actor, owner binding -and version, relationship and revision, capability intersection, target, -resource, local policy, deadline, and invalidation state. It closes or rejects -the lease within the deployment's tested revocation-detection bound after any -owner or relationship dependency becomes ineligible. The claimed bound is no -smaller than measured worst-case detection plus enforcement delay. -[FI-DELEG-INVALIDATION-BOUND] - -Owner retirement, revocation, rotation, disablement under NIP-FI-LIFECYCLE, or -binding replacement invalidates dependent delegates on the same effective -schedule as owner authority. A delegate lease never authorizes another delegate -or owner key on the same connection. [FI-DELEG-OWNER-CURRENT] - -## Discovery - -A relay claiming this profile MAY add `"delegation": true` to the NIP-11 -`federated_identity` object only when owner-current resolution, the positive -finite maximum, uniform final admission, and all profile oracles are active. It -does not advertise relationship IDs, owner keys, private delegation protocol -names, or policy detail. [FI-DELEG-DISCOVERY] - -## Behavioral oracles - -| ID | Required outcome | -|---|---| -| `FI-DELEG-EVIDENCE-CLOSED` | Valid closed evidence passes; unauthenticated, ambiguous, wrong-domain/actor/target/audience, not-yet-valid, and expiry-equality variants deny. | -| `FI-DELEG-PATH-SEPARATION` | Delegation plus any direct assertion/provenance field denies; neither path falls back to the other. | -| `FI-DELEG-OWNER-CURRENT` | Exact current owner succeeds; retirement, revocation, rotation, replacement, stale owner version, stale relationship, and unreadable owner state deny without inheritance. | -| `FI-DELEG-NO-BINDING` | Successful, denied, and concurrent delegated requests create or change no owner/delegate binding or lifecycle state. | -| `FI-DELEG-LEASE-BOUND` | Every authority bound and equality boundary closes the lease; absent finite maximum denies. | -| `FI-DELEG-INVALIDATION-BOUND` | Measured owner/relationship revocation closes prepared evidence and live leases within the claimed detection bound. | -| `FI-DELEG-DISCOVERY` | Discovery is false/absent until the complete active profile passes; public output contains no relationship or owner detail. | - -NIP-FI-CONF defines evidence packaging and mutation adequacy. Each uppercase -requirement above names the oracle that detects its violation. - -## Security considerations - -Delegation expands authority only by intersection and never by copying owner -capabilities. A stolen delegation still requires the delegate key. A stolen -delegate key is bounded by the relationship and finite lease. Owner rotation -cannot silently transfer delegation because the exact owner key and binding -version are dependencies. Implementations should invalidate by dependency index -rather than wait for incidental delegate traffic. diff --git a/docs/nips/NIP-FI-EDGE.md b/docs/nips/NIP-FI-EDGE.md deleted file mode 100644 index 1f54d36e37..0000000000 --- a/docs/nips/NIP-FI-EDGE.md +++ /dev/null @@ -1,417 +0,0 @@ -# NIP-FI-EDGE: Trusted Edge Profile - -`draft` `optional` - -## Scope - -This profile lets a trusted enterprise edge deliver federated assertion evidence to -a NIP-FI verifier. It defines two constructions: - -- `trusted-proxy-hmac-v2`, a portable request-bound HMAC envelope; and -- a private authenticated-edge assertion adapter, for platforms that provide an - equivalent closed trust boundary without the stock envelope. - -NIP-FI-EDGE is optional. A deployment can implement NIP-FI core using only -`client-attached`. Claiming this profile does not weaken core assertion validation, -independent Nostr proof, binding, lifecycle, policy, final-admission, or lease rules. -The key words **MUST**, **MUST NOT**, **REQUIRED**, **SHALL**, **SHALL NOT**, -**SHOULD**, **SHOULD NOT**, **RECOMMENDED**, **NOT RECOMMENDED**, **MAY**, and -**OPTIONAL** are to be interpreted as described in BCP 14 when, and only when, -they appear in all capitals. - -Every identifier this document serializes on the wire — header names, the -profile identifier, provenance envelope fields, and proof transport codes — is -interoperability-critical. `transport_contract_id` remains deployment-local as -core classifies it: its value is opaque outside a deployment, while the -canonical contract semantics this profile contributes to it are normative and -fixed here. Local adapter revision identifiers and key identifiers are -deployment-local and MUST NOT appear in public discovery. - -## Common trusted-edge requirements - -Server-owned listener, route, and authorization-domain configuration selects exactly -one edge profile before protected traffic is accepted. Request evidence cannot -select, negotiate, or downgrade that profile. Missing, repeated, comma-combined, -malformed, oversized, mixed-profile, or profile-inconsistent evidence denies without -fallback to `client-attached` or another edge profile. - -Every trusted edge MUST: - -1. strip every inbound copy of each `Nostr-Federated-Identity`, - `Nostr-Federated-Identity-Provenance`, and `Nostr-Federated-Identity-Client-Peer` - field, and of every other edge profile's assertion, identity, capability, - provenance, and client-peer field, before inserting its own fields. A trusted - edge MUST NOT remove or modify the `Authorization` field, which remains reserved - for the independent NIP-98 proof and MUST reach final admission unmodified; -2. cryptographically authenticate the immediate edge to the accepting origin and - isolate the origin from direct or alternate ingress; -3. integrity-protect every request component used by authorization other than - an independent Nostr proof, which is protected by its own signature; -4. apply a positive finite provenance deadline that is included in final admission - and every resulting lease; -5. validate a closed upstream identity and authorization claim set and produce the - same normalized assertion result required by core; -6. preserve the server-resolved domain, operation, resource, method, authority, - path/query, body semantics, proof transport, and Nostr actor key through final - admission; and -7. keep assertions, credentials, signatures, MACs, raw client addresses, and private - claims out of URLs, public protocol output, logs, metrics, and traces. - -Header presence, source address, private-network location, hostname, or reachability -alone is not provenance. Accepting unsigned identity or capability headers, or -accepting signed headers without authenticating and isolating the immediate caller, -is nonconformant. A trusted edge that strips, rewrites, or reorders the -`Authorization` field, or that admits a proof-transport-`0x02` request whose -`Authorization` field did not arrive at the verifier byte-identical to the -client-sent value, is nonconformant. - -An adapter's reviewed contract MUST identify its accepting origins, direct-origin -controls, field-stripping point, immediate-caller authentication, protected request -components, upstream assertion and policy validation, freshness bounds, independent -Nostr-proof path, compromise impact, and conformance evidence. It MUST deny when any -part of this boundary is absent or unreadable. - -### Authenticated-edge assertion adapters - -A deployment MAY install a private authenticated-edge adapter instead of HMAC-v2. -The adapter MUST satisfy all common requirements and demonstrate together: -origin isolation, cryptographically authenticated immediate caller, inbound-field -stripping, integrity of the complete authorization-relevant request, bounded -assertion and policy freshness, no direct-origin fallback, and the core final- -admission path with independent Nostr proof. - -The adapter maps only its closed, validated claim set into the normalized result. -An opaque edge token is acceptable only inside this complete contract; opacity does -not make an unchecked header authoritative. Vendor names, issuer details, caller -identities, private field names, capability semantics, and adapter identifiers MUST -NOT appear in NIP-11 or portable examples. - -## `trusted-proxy-hmac-v2` - -The stock profile identifier is `trusted-proxy-hmac-v2`. Core computes the -`transport_contract_id` from a canonical contract that includes this profile's exact -wire format, protected components, replay rules, deadline rules, configured code -meanings, and adapter semantics. Changing any of those inputs produces a different -contract identity; the profile identifier itself remains stable. The proxy strips -all inbound assertion, provenance, and client-peer fields and inserts exactly one of -each: - -```text -Nostr-Federated-Identity: Bearer -Nostr-Federated-Identity-Provenance: v2... -Nostr-Federated-Identity-Client-Peer: -``` - -The assertion field follows core's compact-JWS and size rules. `timestamp` is -canonical unsigned decimal without leading zeroes, except zero is `0`. `nonce` and -`mac` are canonical unpadded base64url. Padding, the standard base64 alphabet, -ignored whitespace, or another encoding denies. The proxy generates a fresh nonce -containing at least 128 bits from a cryptographically secure random source. The -decoded MAC is exactly 32 octets. Finite field and decoded-nonce maxima are applied -before decoding, replay lookup, hashing, or allocation. - -`client-peer` is at most 64 ASCII octets. IPv4 uses dotted decimal with no leading -zeroes. IPv6 uses lowercase RFC 5952 text. The edge converts an observed IPv4-mapped -IPv6 address to canonical IPv4 before constructing the field; a textual mapped IPv6 -field is noncanonical. Empty, repeated, comma-combined, whitespace-padded, non-IP, -or noncanonical values deny. After verification, the verifier MAY retain only a -domain-separated keyed digest of this value in bounded private state. - -The profile uses HMAC-SHA-256 with a deployment secret containing at least 256 bits. -Let `LP(x) = uint64be(len(x)) || x`, where length is in octets. The literal prefix is -14 ASCII octets and is not length-prefixed. The pre-MAC input is exactly: - -```text -"NIP-FI-PROXY-2" || -LP(timestamp_u64be) || LP(nonce_bytes) || LP(SHA256(jwt_ascii)) || -LP(authorization_domain_id) || -LP(method_ascii) || LP(authority_ascii) || LP(path_and_query_ascii) || -LP(SHA256(payload_octets)) || LP(proof_transport_octet) || LP(client_peer_ascii) -``` - -`mac = HMAC-SHA-256(secret, pre_mac_input)`. The transmitted `mac` is canonical -unpadded base64url of the raw 32-octet result. The verifier compares it in constant -time. - -### Canonical components - -- **Timestamp:** Parse canonical decimal into an unsigned 64-bit integer, rejecting - overflow, then serialize it as exactly eight-byte big-endian. Freshness checks are - separate from serialization. -- **Nonce:** Decode the exact canonical base64url field before serialization. -- **Assertion:** Hash the exact ASCII compact-JWS octets after the one space in - `Bearer `. No whitespace, Unicode, JSON, or base64 normalization is allowed. -- **Authorization domain:** Configuration contains a canonical lowercase, - hyphenated RFC 9562 UUID named `authorization_domain_uuid`. Parse its 32 displayed - hexadecimal digits into the exact 16 UUID octets in display/network order. For - example, `00112233-4455-6677-8899-aabbccddeeff` becomes - `00112233445566778899aabbccddeeff`. UTF-8 UUID text, hashing, truncation, - namespace derivation, mixed-endian GUID encoding, uppercase, and unhyphenated - configuration are forbidden. The UUID is generated once, is immutable for the - domain's lifetime, and is shared through authenticated proxy/verifier - configuration. Duplicate UUIDs among active domains MUST fail startup. -- **Method:** Use the exact uppercase ASCII method token after trusted route - resolution. Lowercase or noncanonical input denies; the verifier does not repair it. -- **Authority:** Use server-configured lowercase ASCII host plus explicit effective - decimal port. IPv6 uses brackets and RFC 5952. Userinfo, a trailing dot, an omitted - port, percent encoding, or an authority derived solely from `Host`, `Forwarded`, or - `X-Forwarded-Host` denies. -- **Path and query:** Use the exact post-rewrite ASCII origin-form. Empty path becomes - `/`; a present query includes `?`. Percent octets and hex case, an empty query, - repeated names, and parameter order are preserved. No decoding, sorting, - dot-segment removal, or re-encoding may occur after the edge snapshot. An - unaccounted rewrite denies. -- **Payload:** Hash the complete HTTP payload octets after transfer-coding removal and - before content-coding decompression. These are exactly the octets forwarded by the - edge and exposed to verification. HTTP framing, chunk delimiters, and trailers are - excluded; `Content-Encoding` is not decoded. A WebSocket upgrade uses the empty - payload. Substitution of the protected octets after the snapshot denies. -- **Proof transport:** Serialize exactly one assigned octet from the registry below. -- **Client peer:** Serialize the exact canonical ASCII field value. - -No authorization decision, target, resource, capability, or effect selector -derives from any request or connection component outside the protected pre-MAC -components, except an independent Nostr proof validated on its own signature, -such as the NIP-98 event in `Authorization` or the NIP-42 event after connect, -which the MAC does not protect; body interpretation follows the server-resolved -body semantics, never unprotected transport metadata such as `Content-Type` or -`Content-Encoding`. - -### Freshness, replay, and key rotation - -The deployment configures a positive finite `maximum_provenance_age` and a -non-negative finite `future_skew`. Evidence is live exactly when, using overflow-safe -comparisons: - -```text -timestamp <= now + future_skew -now < timestamp + maximum_provenance_age -``` - -Equality at the age bound is expired. A direct lease deadline is no later than -`timestamp + maximum_provenance_age` and every core assertion, proof, policy, and -state deadline. - -Absent, malformed, stale, future-dated, wrong-key, or mismatched provenance denies. -On a route that requires edge provenance, absent or incomplete provenance — -including provenance that omits the proxy-authenticated end-client peer — maps -to the `missing_evidence` public class, regardless of whether an assertion is -present. Provenance that is present and complete but fails verification maps to -`evidence_rejected`. -A v1 envelope denies. A verifier MAY try only a configured finite set of active -secrets. Rotation does not change nonce identity: replay uniqueness is scoped to -`(authorization_domain_id, trusted-proxy-hmac-v2, nonce)` and is independent of the -secret that verifies the MAC. A committed nonce is retained through at least -`timestamp + maximum_provenance_age`. - -Preparation consumes neither nonce nor Nostr-proof replay identity. Final admission -atomically consumes both with any enrollment, receipt, and authorization decision. -A failed or rolled-back admission consumes neither. Two concurrent admissions with -the same nonce commit at most one authorization. The proxy-to-verifier hop still -requires confidentiality and integrity. - -## Proof-transport code registry - -| Code | Meaning and allocation policy | -|---|---| -| `0x00` | Invalid; MUST deny. | -| `0x01` | NIP-42 connection proof. | -| `0x02` | NIP-98 HTTP proof. | -| `0x03` | Git smart-HTTP session proof: the proxy verifies a session-scoped Nostr authorization for a Git smart-HTTP request before forwarding. Reserved; allocation completes on publication of its transport contract (see below). | -| `0x04` | Blossom media proof: the proxy verifies a Blossom media-HTTP authorization event for the request before forwarding. Reserved; allocation completes on publication of its transport contract (see below). | -| `0x05`–`0x7f` | Unassigned; allocation requires a published stable specification. | -| `0x80`–`0xfe` | Private use under an explicit shared proxy/verifier contract only. | -| `0xff` | Reserved for a future extended encoding; invalid in HMAC-v2. | - -An allocation MUST define exact proof validation, request binding, freshness, replay -identity and window, and conformance vectors. Assigned semantics never change; an -incompatible meaning receives a new code. Unknown, unconfigured, or private-use -codes without the same configured contract at proxy and verifier deny. Private-use -codes MUST NOT be advertised as portable NIP-FI-EDGE interoperability. - -Codes `0x03` and `0x04` are reserved to fix their meanings and prevent -reassignment; their transport contracts are not yet published, so their -allocations are not complete. Until the contract for such a code is published, -the code is valid only under an explicit shared proxy/verifier contract, -exactly as for private use, and MUST NOT be presented as portable NIP-FI-EDGE -interoperability. - -## Bounded payload acquisition - -Every protected `(authorization_domain_id, route, proof_transport_code)` tuple MUST -configure a finite `maximum_payload_octets` and finite per-request -`maximum_spool_octets >= maximum_payload_octets`. Zero is allowed only for a route -that requires an empty payload. Proxy and verifier configuration MUST agree and is -part of the transport contract. - -If trusted `Content-Length` exceeds the route limit, the edge denies before reading, -hashing, JWT verification, replay lookup, or authoritative mutation. For absent, -unknown, or streamed length, acquisition uses a bounded counter and spool and stops -on octet `limit + 1`. Incremental SHA-256 is allowed, but no digest or prefix can -authorize until EOF proves completeness. - -Spooling uses memory or access-controlled temporary storage with finite per-request -and aggregate quotas, cleanup on every outcome, no public or log output, and no reuse -across requests. Quota exhaustion fails closed and creates no nonce claim, proof -claim, receipt, lease, or application mutation. At or below the limit, the exact -captured payload is replayed unchanged. HMAC verification and core final admission -complete before application effects. Forwarding to a rollback-safe private spool is -not an application effect; forwarding to a parser, decoder, handler, or origin that -can act is. - -A content decoder, multipart parser, Git/Blossom handler, framework, or intermediary -that cannot expose and replay the exact stage defined above before effects cannot -claim HMAC-v2 for that route. It MUST use core `client-attached` or another specified -edge profile, never a partial-body MAC. - -## Normative HMAC-v2 vectors - -All vector integers and lengths are big-endian. Common values are: - -```text -secret_hex = 000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f -nonce_hex = 000102030405060708090a0b0c0d0e0f -nonce_base64url = AAECAwQFBgcICQoLDA0ODw -authorization_domain_uuid = 00112233-4455-6677-8899-aabbccddeeff -authorization_domain_id_hex = 00112233445566778899aabbccddeeff -jwt_ascii = eyJhbGciOiJFUzI1NiIsInR5cCI6Im5pcC1maStqd3QifQ.eyJpc3MiOiJodHRwczovL2lkLmV4YW1wbGUiLCJzdWIiOiIxMjMifQ.c2ln -assertion_digest_hex = 6103b52a52730bc065d65673247603a63c9810488c90d0ada3d8d227eee5285f -``` - -The fixture JWT represents a separately minted `nip-fi+jwt` assertion and is opaque -test input; its deliberately synthetic signature is not an assertion-validation -vector. Implementations MUST reproduce each field, complete pre-MAC input, -diagnostic input digest, raw MAC, and wire MAC exactly -(`FI-TRACE-EDGE-VECTORS`). - -### Vector 1: HTTP / NIP-98 / non-empty payload - -```text -timestamp_decimal = 1700000000 -timestamp_u64be_hex = 000000006553f100 -method_ascii = POST -authority_ascii = api.example:443 -path_and_query_ascii = /upload?part=1&part=2&x=%2F -payload_hex = 68656c6c6f0a -body_digest_hex = 5891b5b522d5df086d0ff0b110fbd9d21bb4fc7163af34d08286a2e846f6be03 -proof_transport_hex = 02 -client_peer_ascii = 203.0.113.9 -pre_mac_input_hex = 4e49502d46492d50524f58592d320000000000000008000000006553f1000000000000000010000102030405060708090a0b0c0d0e0f00000000000000206103b52a52730bc065d65673247603a63c9810488c90d0ada3d8d227eee5285f000000000000001000112233445566778899aabbccddeeff0000000000000004504f5354000000000000000f6170692e6578616d706c653a343433000000000000001b2f75706c6f61643f706172743d3126706172743d3226783d25324600000000000000205891b5b522d5df086d0ff0b110fbd9d21bb4fc7163af34d08286a2e846f6be03000000000000000102000000000000000b3230332e302e3131332e39 -pre_mac_input_sha256 = df2870230d2170595dccd17d9e61a82282d8cd8b978ac18bff07419ed59091d5 -mac_hex = 761d3ecbf609f0f558b4a02a1a18a25070f3dbe89fce9cac59a80bce4436ade5 -mac_base64url = dh0-y_YJ8PVYtKAqGhiiUHDz2-ifzpysWagLzkQ2reU -provenance = v2.1700000000.AAECAwQFBgcICQoLDA0ODw.dh0-y_YJ8PVYtKAqGhiiUHDz2-ifzpysWagLzkQ2reU -``` - -### Vector 2: WebSocket / NIP-42 / empty payload / mapped peer - -The edge observed `::ffff:192.0.2.128` and emitted canonical `192.0.2.128`. - -```text -timestamp_decimal = 1 -timestamp_u64be_hex = 0000000000000001 -method_ascii = GET -authority_ascii = relay.example:443 -path_and_query_ascii = / -payload_hex = -body_digest_hex = e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 -proof_transport_hex = 01 -client_peer_ascii = 192.0.2.128 -pre_mac_input_hex = 4e49502d46492d50524f58592d32000000000000000800000000000000010000000000000010000102030405060708090a0b0c0d0e0f00000000000000206103b52a52730bc065d65673247603a63c9810488c90d0ada3d8d227eee5285f000000000000001000112233445566778899aabbccddeeff0000000000000003474554000000000000001172656c61792e6578616d706c653a34343300000000000000012f0000000000000020e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855000000000000000101000000000000000b3139322e302e322e313238 -pre_mac_input_sha256 = 67564d241499491b3ea53b31d6111fbc9efac37a294f6ce591519e4bf21b53e9 -mac_hex = f71a179a018637a0582cf3de39ccb7b976216c18ada312127d4c983c14af4b20 -mac_base64url = 9xoXmgGGN6BYLPPeOcy3uXYhbBitoxISfUyYPBSvSyA -``` - -### Vector 3: IPv6 authority and path/query byte preservation - -```text -timestamp_decimal = 1700000000 -timestamp_u64be_hex = 000000006553f100 -method_ascii = GET -authority_ascii = [2001:db8::1]:443 -path_and_query_ascii = /a%2Fb?b=2&a=1&a=0 -payload_hex = -body_digest_hex = e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 -proof_transport_hex = 02 -client_peer_ascii = 2001:db8::2 -pre_mac_input_hex = 4e49502d46492d50524f58592d320000000000000008000000006553f1000000000000000010000102030405060708090a0b0c0d0e0f00000000000000206103b52a52730bc065d65673247603a63c9810488c90d0ada3d8d227eee5285f000000000000001000112233445566778899aabbccddeeff000000000000000347455400000000000000115b323030313a6462383a3a315d3a34343300000000000000122f61253246623f623d3226613d3126613d300000000000000020e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855000000000000000102000000000000000b323030313a6462383a3a32 -pre_mac_input_sha256 = 8a93a29c4ac30b0f2551d346d0636040b639bb1f109d287e93ce44ddaed73e33 -mac_hex = df2936f81d752f3d6bac2a36d3381c38db2c9abc3570236cb121274ad34a6161 -mac_base64url = 3yk2-B11Lz1rrCo20zgcONssmrw1cCNssSEnStNKYWE -``` - -### Serialization and negative matrix - -The following timestamp values MUST serialize as shown before freshness evaluation: - -| Decimal | `uint64be` hex | -|---:|---| -| `0` | `0000000000000000` | -| `1` | `0000000000000001` | -| `255` | `00000000000000ff` | -| `256` | `0000000000000100` | -| `18446744073709551615` | `ffffffffffffffff` | - -`00`, `01`, `+1`, surrounding whitespace, negative values, and -`18446744073709551616` deny before MAC comparison. The maximum value above is an -encoding vector; ordinary freshness policy will reject it. - -Every implementation MUST run these normative negative cases: - -| Class | Required cases and result | -|---|---| -| Envelope | Absent/repeated/comma-combined fields, `v1`, missing/extra component, padding, alternate alphabet, nonce below 16 octets or above configured max, and MAC lengths 31 or 33 all deny. | -| Domain | Uppercase/nonhyphenated UUID config fails configuration; mixed-endian UUID bytes or any one-bit domain transplant fails the baseline MAC; duplicate active UUID fails startup. | -| Request | Mutating assertion, method, authority, path/query, body, proof code, or peer while retaining Vector 1's MAC denies. | -| Metadata | Mutating `Content-Type` or `Content-Encoding` in flight changes no authorization decision, target, capability, or effect selector; a request whose server-resolved body semantics no longer hold denies. | -| Path | `%2F`→`%2f`, decoding to `/`, reordering repeated query values, or adding/removing an empty `?` fails the baseline MAC. | -| Authority | Unbracketed or non-RFC-5952 IPv6, uppercase host, trailing dot, or missing port denies before MAC comparison. | -| Peer | Textual `::ffff:192.0.2.128`, padded IPv4, uppercase/noncanonical IPv6, or whitespace denies before MAC comparison. | -| Proof | `0x00`, `0xff`, unknown stock code, or private code without a shared configured contract denies. | -| Body | Known and unknown lengths `0`, `limit-1`, and `limit` may proceed only after EOF; `limit+1`, disconnect before EOF, aggregate-quota exhaustion, or any post-snapshot substitution of the protected octets denies with no replay or authoritative mutation. | -| Replay | Concurrent final admissions of one valid envelope commit at most one; preparation and failed final admission consume none; secret rotation does not create a new nonce namespace. | -| Fallback | Direct ingress, mixed evidence, and failed HMAC never retry as `client-attached` or another adapter. | - -## Discovery and conformance - -A relay that completely implements the stock profile MAY add exactly -`"edge_transports": ["trusted-proxy-hmac-v2"]` inside the top-level NIP-11 -`federated_identity` object. `edge_transports` is an array of unique ASCII string -profile identifiers in ascending bytewise order; this document assigns only the -single value shown. A relay that does not completely implement the stock profile -MUST omit the member. It MUST NOT advertise private adapters, keys, domains, field -names, or code contracts. No request may select behavior from this discovery -member; server-owned configuration selects the edge profile. Claiming FI-EDGE -requires every configured edge profile to pass the applicable core conformance suite -and these profile traces: - -| Trace | Required oracle | -|---|---| -| `FI-TRACE-EDGE-VECTORS` | Reproduce all three normative vectors field-for-field, including each complete pre-MAC input, diagnostic input digest, raw MAC, and wire MAC; reproduce all five timestamp serialization rows; every listed serialization and negative-matrix case produces its required denial or configuration failure. | -| `FI-TRACE-PROXY-SPOOF` | Direct ingress, unsigned/header-only identity, unauthenticated caller, or invalid provenance denies without fallback. | -| `FI-TRACE-PROXY-REPLAY` | Two HMAC-v2 final admissions using one nonce commit at most one; preparation consumes neither. A private adapter proves its declared replay semantics. | -| `FI-TRACE-PROXY-CROSS-REQUEST` | Each protected component mutation denies. HMAC-v2 covers assertion, domain, method, authority, path/query, complete body, proof transport, and peer. On a `0x02` route the `Authorization` bytes at final admission equal the client-sent bytes, witnessed at both points; an edge that substitutes a valid proof from the same actor fails the witness. | -| `FI-TRACE-EDGE-BODY-BOUNDS` | Known and streamed boundary cases prove bounded work/storage, EOF completeness, cleanup, and no pre-authorization effect. | -| `FI-TRACE-EDGE-KEY-ROTATION` | A finite active-key set accepts an intended overlap without allowing nonce reuse or an unknown key. | - -The conformance record binds the exact implementation, adapter, deployment, -assertion policy, transport contract, configured code meanings, and vector revision. -Two HMAC-v2 implementations interoperate only when they reproduce all valid vector -bytes exactly, reject every negative, agree on UUID and code configuration, and -preserve atomic replay and bounded complete-body behavior. - -## Security considerations - -HMAC-v2 limits header spoofing, replay, and cross-request transplantation only when -its secret remains confidential, the edge snapshots the final routed request, the -origin authenticates that edge, and final admission atomically consumes replay state. -It does not replace TLS or independent Nostr proof. A compromised edge or shared -secret can forge federated evidence within its configured domains; use distinct -secrets and UUIDs to limit blast radius. - -Authenticated-edge adapters intentionally shift more proof to deployment controls. -A hostname, private network, or opaque token is not an equivalent construction unless -the complete boundary obligations above are demonstrated. Body buffering and replay -state are attacker-controlled resource surfaces, so all field, payload, spool, -aggregate, key-set, and retention bounds fail closed. diff --git a/docs/nips/NIP-FI-LIFECYCLE.md b/docs/nips/NIP-FI-LIFECYCLE.md deleted file mode 100644 index fdd08d4dfb..0000000000 --- a/docs/nips/NIP-FI-LIFECYCLE.md +++ /dev/null @@ -1,269 +0,0 @@ -# NIP-FI-LIFECYCLE: Binding Lifecycle Profile - -`draft` `optional` - -## Abstract - -This profile extends NIP-FI with provisioned enrollment, identity disablement, -re-enablement, and an administrative binding-expiry gate. It is for deployments -whose binding changes require separately authorized operator or enterprise -workflows. It does not change NIP-FI assertion validation, Nostr proof, final -admission, or public denial semantics. - -The key words **MUST**, **MUST NOT**, **REQUIRED**, **SHALL**, **SHALL NOT**, -**SHOULD**, **SHOULD NOT**, **RECOMMENDED**, **MAY**, and **OPTIONAL** in this -document are to be interpreted as described in BCP 14 when, and only when, they -appear in all capitals as shown here. - -## Dependencies and claim - -An implementation of this profile implements NIP-FI Core and advertises only -the boolean `"lifecycle": true` inside its NIP-11 `federated_identity` object. -This boolean claims support for this profile; it deliberately reveals neither an -enrollment mode nor lifecycle state. For a fixed set of claimed profiles, the -complete discovery output MUST be byte-identical whether enrollment is -attested-key, TOFU, or provisioned and whether lifecycle facts exist. A server -MUST NOT advertise the claim until every protected ingress in the advertised -authorization domain applies this profile through the same final-admission -authority (`FI-LC-CLAIM`). - -This profile contributes lifecycle dependencies and deadlines to the core -prepared decision and lease. They compose with core dependencies by set union; -the earliest applicable deadline wins. This profile cannot weaken, replace, or -bypass a core check. - -## Additional state - -For authorization domain `D`, this profile adds: - -```text -X_D : set of disabled identities -Q_D : identity -> pending lineage - -PendingLineage = ( - identity, - old_key, - old_binding_version -) -``` - -It also permits a core binding to carry `binding_not_after`, an optional -administrative deadline. The pending lineage names one exact retired pair and -binding version. There is at most one pending lineage per identity. - -`binding_not_after` bounds the grant represented by **one binding**; it is not a -bound on the identity. Rotation continues the same grant: the replacement binding -preserves the carried bound, and only expiry authority changes it. Retirement, -revocation, and disablement end the existing grant. Re-enablement, provisioning, -and ordinary enrollment establish a *new* grant that carries no prior bound — so -retirement of a bound pair followed by ordinary enrollment under `attested-key` -or `tofu` policy yields an unbounded binding, and that is conformant. A deadline that must survive the end -of a grant — an identity-scoped access bound — belongs in the capability -projection of authoritative local-policy state, which core already requires for -any projection whose removal must close authority within a declared bound and -which is reread at preparation, final admission, and every protected lease use. -`binding_not_after` is not that mechanism and cannot substitute for it: it does -not survive the end of the grant that carries it, so a deployment relying on it -for identity-scoped expiry cannot claim a revocation bound for that expiry -(`FI-LC-ADMIN-EXPIRY`). - -A binding carrying a reached bound is **active** for every core relation and -eligibility test in this profile and in core, including the core partial -bijection and `TargetEligible` below. It is ineligible for authorization, not -absent from the binding relation. - -`X_D`, `Q_D`, and `binding_not_after` are deployment-local state. Their versions -are revalidation dependencies, not contract identities. A change invalidates a -prepared decision and every dependent lease unless complete final-admission -recomputation produces the required current result. - -Ordinary authorization MUST deny when its identity is disabled, when pending -lineage exists for that identity, or when `now >= binding_not_after`; it MUST -NOT clear, consume, or alter any of those facts (`FI-LC-ORDINARY-GATES`). An -absent `binding_not_after` has no administrative expiry. Assertion `exp`, -`iat`, refresh, or maximum age never creates, renews, extends, or clears it. -Time passage alone creates no tombstone, lineage, or history. - -## Private denial conditions - -This profile defines exactly these private condition identifiers and owning -public classes for NIP-FI-CONF enumeration agreement: - -| Private condition identifier | Public class | -|---|---| -| `identity_disabled` | `authorization_denied` | -| `explicit_replacement_required` | `authorization_denied` | -| `binding_expired` | `authorization_denied` | - -The identifiers are fixture names, not wire values. Adding, removing, renaming, -or reclassifying one requires the same change in NIP-FI-CONF's denial-fixture -table. - -## Common transition contract - -Each transition below requires privileged authority distinct from an ordinary -federated assertion and Nostr proof. That authority MUST be bound to the exact -`D`, transition name, identity, request, old binding version when present, and -target key when present (`FI-LC-AUTHORITY`). The deployment defines how that -authority is obtained; role names, approval count, and operator APIs are out of -scope. - -A transition MUST, in one atomic commit: - -1. validate that privileged authority and fresh target-key evidence; -2. read and recheck the applicable core binding relation, retired pairs, - revoked keys, `X_D`, `Q_D`, policy, and dependency versions; -3. apply exactly the state changes specified below; -4. append immutable lifecycle history identifying the transition and versions; - and -5. advance lifecycle state so dependent prepared decisions and leases cannot - authorize after commit. - -A stale precondition, denied transition, unreadable dependency, or failed commit -MUST leave all authoritative state unchanged (`FI-LC-ATOMIC`). Lease -invalidation MAY be delivered asynchronously, but authorization use after the -commit MUST recheck the advanced dependency before allowing an operation. - -Every transition that creates a binding MUST state whether it **continues** an -existing grant, and therefore preserves that grant's administrative bound, or -**establishes** a new grant carrying no prior bound. The two cases partition the -binding-creating transitions with no remainder: core rotation continues, and -provisioning, re-enablement, and ordinary enrollment establish. A profile that -adds a binding-creating transition without this declaration cannot claim -conformance (`FI-LC-CLAIM`). - -`TargetEligible(i, k, allow_disabled)` means that `k` is not revoked, `(i, k)` -is not retired, neither `i` nor `k` has an active binding, and `i` is not -disabled unless `allow_disabled` is true. Every new target key requires fresh, -request-bound Nostr proof by that key. If domain policy requires issuer key -attestation, the transition also requires a current assertion for `i` whose key -claim equals `k`. Supplied stale, absent, wrong-identity, or mismatched required -attestation denies; it is never ignored as optional evidence -(`FI-LC-TARGET-PROOF`). - -A replacement binding records `attested-key` provenance only when current -matching issuer attestation was validated; otherwise it records `provisioned`. -TOFU provenance can arise only from the core ordinary first-use extension and -is never inherited by a replacement. - -## Privileged transitions - -### Provision binding - -```text -ProvisionBinding(i, k): - require domain enrollment policy = provisioned - require TargetEligible(i, k, false) - require Q_D(i) is absent - require fresh target-key evidence - create Binding(i, k, new_version, provisioned) -``` - -The transition creates no authorization lease. Later use requires a current -assertion, fresh Nostr proof, and ordinary final admission. Ordinary -request-time authorization under `provisioned` policy MUST NOT create a binding -(`FI-LC-PROVISION`). - -### Disable identity - -```text -DisableIdentity(i): - add i to X_D - if Binding(i, k, old_version) exists: - remove Binding(i, k, old_version) - add (i, k) to the core retired-pair set - set Q_D(i) = (i, k, old_version) -``` - -Applying an authorized disablement repeatedly is idempotent. It MUST NOT erase -or replace existing lineage. If `i` has no active binding, disablement creates -no lineage (`FI-LC-DISABLE`). - -### Re-enable identity - -```text -ReenableIdentity(i, expected_lineage?, k_new): - require i is in X_D - require Q_D(i) is absent when expected_lineage is absent, - otherwise require Q_D(i) = expected_lineage - require TargetEligible(i, k_new, true) - require fresh target-key evidence - remove i from X_D - consume expected_lineage when present - create Binding(i, k_new, new_version, ReplacementProvenance(evidence)) -``` - -Clearing disabled state and creating the target binding are inseparable. There -is no clear-only transition: it would permit a later ordinary enrollment to -capture the identity. An operator that intends to provision later leaves the -identity disabled until the target and fresh proof are available -(`FI-LC-REENABLE`). - -### Set administrative expiry - -```text -SetAdministrativeExpiry(i, k, old_version, binding_not_after?): - require exact current Binding(i, k, old_version) - require separate privileged expiry authority - replace it with Binding(i, k, new_version, - same_provenance, binding_not_after?) -``` - -This transition changes neither side of the pair nor its provenance. Setting, -replacing, or clearing the bound advances the binding version. At equality the -binding is ineligible but remains durable and occupies both sides of the core -partial bijection. This transition is the only expiry authority: no other -transition in this profile or in core sets, replaces, or clears the bound, and -core rotation carries it onto the replacement binding unchanged. Only this or -another applicable privileged transition can restore access; ordinary -authorization cannot renew the bound (`FI-LC-ADMIN-EXPIRY`). - -## One-shot lineage and concurrency - -Consumption of `Q_D` and creation of its replacement binding MUST be one -compare-and-commit operation over the exact pending lineage. Of two concurrent -re-enablings presenting the same lineage, at most one can commit. The loser -observes changed state and denies without creating a binding, consuming another -lineage, or changing history (`FI-LC-QD-ONCE`). - -A lifecycle transition racing ordinary final admission is ordered by the same -authoritative state transaction or dependency check. If the lifecycle commit -wins, the ordinary operation denies; if final admission wins first, the -lifecycle transition still invalidates subsequent lease use. No ordering -permits authority from a disabled identity, consumed lineage, or expired -binding after the corresponding state change is observed. - -## Behavioral oracles - -Each oracle is normative. A conforming implementation produces the stated -result at final admission and retains no partial authoritative mutation from a -denied case. - -| ID | Setup and required result | -|---|---| -| `FI-LC-CLAIM` | For a fixed profile set, compare complete discovery bytes across attested-key, TOFU, and provisioned configurations and across lifecycle states: they are identical. If one protected ingress omits lifecycle gates or uses a different lifecycle lineage, the domain cannot advertise the profile and the uncovered ingress fails closed. If any claimed profile defines a binding-creating transition that declares neither grant continuation nor grant establishment, the domain cannot advertise that profile. | -| `FI-LC-ORDINARY-GATES` | Fresh assertion and proof for a disabled identity, an identity with pending lineage, and a binding at administrative-expiry equality each deny without changing lifecycle state. | -| `FI-LC-AUTHORITY` | An ordinary assertion plus valid Nostr proof, but no transition-specific authority, cannot perform any transition; mutation of any authority-bound field denies. | -| `FI-LC-ATOMIC` | Inject failure at each transition write boundary; no binding, tombstone, disabled fact, lineage, history entry, or dependency version is partially committed. | -| `FI-LC-TARGET-PROOF` | Missing, stale, wrong-key, wrong-request, or mismatched required attestation for a new target denies without mutation. | -| `FI-LC-PROVISION` | Ordinary first use in provisioned mode denies; authorized provisioning creates one binding and no lease; later current ordinary admission may use it. | -| `FI-LC-DISABLE` | Disabling an active identity atomically disables it, retires its exact pair, records exact lineage, and closes subsequent lease use; replay is idempotent and preserves lineage. | -| `FI-LC-REENABLE` | Re-enablement creates an eligible proven binding in the same commit that clears disabled state; absent or wrong expected lineage and a clear-only attempt deny. | -| `FI-LC-ADMIN-EXPIRY` | Before the bound the binding may authorize; at equality it denies while still occupying the relation, so a target eligibility test for either side of that pair fails. Rotating the expired binding to a new key carries the bound: the replacement denies at the same instant. No non-expiry transition clears it. Only an authorized version-checked update by the expiry authority changes the bound. Conversely, retirement, revocation, or re-enablement of the bound pair followed by an authorized or ordinary new grant produces an unbounded binding, which is the required result and not an escape. | -| `FI-LC-QD-ONCE` | Two concurrent re-enablings consume the same `Q_D` lineage; exactly one commits and the loser leaves every authoritative store unchanged. | -| `FI-LC-RACE` | Race each transition against prepared ordinary admission and lease use; no operation authorizes after observing the advanced lifecycle or binding dependency. | - -## Security considerations - -Privileged authority compromise can provision or replace enterprise bindings; -deployments should apply controls proportionate to that authority. This profile -makes the authority request-bound and transitions atomic, but does not define -approval UX or key custody. - -Disabled identities, retired pairs, revoked keys, and pending lineage serve -different purposes. Re-enablement removes only the exact disabled fact and -optional exact lineage named by its transition. No transition in this profile -removes a core revoked-key or retired-pair fact. - -Administrative expiry is local policy, not upstream revocation freshness. It -cannot extend an assertion, status witness, Nostr proof, or lease deadline. diff --git a/docs/nips/NIP-FI-MODEL.md b/docs/nips/NIP-FI-MODEL.md deleted file mode 100644 index 821477755c..0000000000 --- a/docs/nips/NIP-FI-MODEL.md +++ /dev/null @@ -1,96 +0,0 @@ -NIP-FI-MODEL -============ - -Composed authorization model (non-normative) --------------------------------------------- - -This companion is explanatory. It defines no requirement, invariant, wire -value, denial mapping, or conformance claim. Normative requirements live in -[NIP-FI](NIP-FI.md) and the claimed profile documents. In particular, -`FI-INV-01` through `FI-INV-16` are defined only by NIP-FI core. - -## State sketch - -One useful implementation model keeps these authoritative relations per domain: - -```text -B_D : active identity-to-key relation -T_D : retired identity/key pairs -Y_D : revoked keys -H_D : immutable lifecycle history -V_D : binding and lifecycle versions -``` - -NIP-FI-LIFECYCLE adds disabled identities and pending replacement lineage. -NIP-FI-DELEG adds relationship state but no delegate binding. NIP-FI-EDGE adds -transport-provenance and replay witnesses. Implementations may use different -storage as long as their observable behavior satisfies the owning normative -documents. - -## Composed direct decision - -The core decision can be read as this equation: - -```text -validated issuer-qualified identity -+ fresh request/connection-bound Nostr proof -+ current durable partial-bijection state -+ current local policy -+ atomic final admission -= authority for exactly the proven key and operation -``` - -Preparation gathers immutable evidence and snapshots every dependency without -mutation. Final admission compares exact context and stable contract identities, -checks all deadlines, revalidates changed snapshots, recomputes from current -binding and policy state, then commits replay claims, optional enrollment, and a -receipt atomically. The special concurrent-enrollment normalization is narrow: -an `enroll(i,k)` proposal may become the same eligible `existing(i,k)` result; -a different winner is not equivalent. - -## Profile composition - -Profiles contribute witnesses, never alternate final authority: - -```text -core witnesses -∪ EDGE provenance/replay witnesses -∪ LIFECYCLE eligibility/lineage witnesses -∪ DELEG owner/relationship witnesses -``` - -The lease deadline is the minimum of every bound in the resulting set. A missing -or unreadable required witness denies. A profile cannot remove a core witness, -extend a core deadline, replace the proven actor, or create a second admission -lineage. - -For direct authorization, the path dependency is the normalized assertion and -its current snapshot/status witnesses. For delegated authorization, it is the -exact eligible owner binding plus relationship evidence; direct assertion fields -are absent. Both paths share context resolution, Nostr-proof validation, local -policy, read-only preparation, and atomic final admission. - -## Lifecycle intuition - -Bindings are durable; leases are ephemeral. Retirement makes one exact pair -permanently ineligible for ordinary recreation. Revocation makes a key -ineligible throughout the domain. Rotation retires the old pair and creates a -new binding version but does not globally revoke the old key. Extended lifecycle -operations may add disabled identity and one-shot pending-lineage state, as -specified by NIP-FI-LIFECYCLE. - -## Privacy intuition - -Private reasons collapse to fixed public bytes. In particular, binding -conflicts, tombstones, lifecycle gates, key mismatch, enrollment requirements, -and local-policy decisions are indistinguishable. Operational diagnostics may -retain bounded private reason codes, but such records are not protocol objects -and never become authorization witnesses. - -## Reading order - -1. NIP-FI for core state, wire behavior, invariants, and direct admission. -2. NIP-FI-EDGE for a trusted-enterprise edge. -3. NIP-FI-LIFECYCLE for provisioning, disablement, and re-enablement. -4. NIP-FI-DELEG for delegated agents. -5. NIP-FI-CONF for claim and evidence rules. diff --git a/docs/nips/NIP-FI.md b/docs/nips/NIP-FI.md index b5b35ca5dd..845a150a4e 100644 --- a/docs/nips/NIP-FI.md +++ b/docs/nips/NIP-FI.md @@ -1,14 +1,12 @@ NIP-FI ====== -Federated identity authorization — core ----------------------------------------- +Federated identity authorization — stateless core +--------------------------------------------------- `draft` `optional` `relay` -**Protocol dependencies**: NIP-01 and either NIP-42 or NIP-98. Optional -profiles are defined by NIP-FI-EDGE, NIP-FI-LIFECYCLE, NIP-FI-DELEG, and -NIP-FI-CONF. +**Protocol dependencies**: NIP-01, NIP-42. The key words "MUST", "MUST NOT", "REQUIRED", "SHOULD", "SHOULD NOT", and "MAY" in this document are to be interpreted as described in BCP 14 (RFC 2119 @@ -16,485 +14,457 @@ and RFC 8174) when, and only when, they appear in all capitals. ## Abstract -NIP-FI authorizes a Nostr key only when four independent facts agree: a valid -issuer-qualified identity assertion, fresh proof of that Nostr key, current -identity-to-key binding state, and current local policy for the exact operation. -The identity provider never signs Nostr events, and an assertion never replaces -Nostr proof. +NIP-FI authorizes a Nostr key when two independent facts agree: a valid +issuer-qualified identity assertion that names the key, and fresh NIP-42 proof +of possession of that key. No relay-side identity state is required. The +relay verifies the assertion offline against configured per-issuer JWKS +snapshots; every identity decision beyond key verification is the assertion +issuer's responsibility. -Bindings outlive individual assertions. Assertions and authorization leases do -not outlive their evidence. This core defines the portable client-attached -assertion transport, direct enrollment, atomic final admission, bounded -sessions, privacy-preserving denial responses, and the smallest useful binding -lifecycle. Companion profiles add trusted edges, extended lifecycle operations, -and delegation without changing the core admission rule. +This NIP defines the assertion contract, the offline verification procedure, +session lifetime policy, and an authenticated issuer→relay disconnect API. +Enrollment, rotation, revocation decisions, identity↔key registry, one-identity +one-key enforcement, audit, and directory integration are issuer concerns outside +this spec. -This NIP does not define an identity provider, database schema, operator API, -public identity projection, application membership policy, or user interface. +## Terms -## Terms and identifier classes - -- **domain** (`D`): an authorization boundary selected only by authenticated - server routing and configuration. - **identity** (`i`): the exact tuple `(iss, sub)` returned by assertion - validation. Email, display name, employee number, and a bare `sub` are not - identities. -- **target context** (`R_t`): the server-resolved method, authority, path and - query, body semantics, transport, operation, and resource. -- **actor** (`k`): the 32-byte public key returned by Nostr-proof validation. -- **request context** (`R`): `R_t` sealed with `k`. -- **binding**: a durable, versioned association `(D, i, k)` with immutable - provenance `attested-key`, `tofu`, or `provisioned`. -- **retired pair**: a durable denial fact for an exact `(D, i, k)`. -- **revoked key**: a durable denial fact for `(D, k)`. -- **prepared authorization**: immutable, read-only evidence and witnesses for a - possible admission. -- **committed authorization**: authority returned only after final revalidation - and atomic commit. -- **lease**: a cached committed decision for one actor and bounded operation - set. A lease is not a binding. + validation. Email, display name, opaque user ID, and a bare `sub` are not + identities. Equal `sub` values under different `iss` values are distinct + identities. [FI-TRACE-CROSS-DOMAIN-COLLISION] +- **actor** (`k`): the 32-byte public key returned by NIP-42 proof validation. +- **assertion**: a compact JWS minted by the assertion issuer, binding `i` + to `k`. +- **assertion issuer**: the deployment-specific identity authority (e.g. an + OIDC identity provider integration) that authenticates users and mints + assertions. The relay trusts only the issuer's assertion; it does not + contact the IdP directly. -Identity and authorization-state comparisons preserve every tuple component. -Equal `sub` values under different `iss` values are distinct identities; equal -`(i, k)` pairs under different domains are distinct bindings, retired pairs, -and authorization state. [FI-TRACE-CROSS-DOMAIN-COLLISION] +## Assertion contract -Every identifier is either **interoperability-critical** or -**deployment-local**. Header names, public response bytes, token type values, -and trace identifiers are interoperability-critical and fixed here. -`assertion_policy_id`, `transport_contract_id`, domain IDs, snapshot versions, -binding versions, policy versions, and correlation IDs are deployment-local; -their values are opaque outside a deployment, while their stability and -invalidation behavior are normative. +The assertion is a compact JWS carrying the following claims. -## Core security invariants +### Required claims -These labels are the normative home of the NIP-FI invariants. Companion -profiles may add witnesses and bounds but cannot weaken them. +| Claim | Type | Semantics | +|---|---|---| +| `iss` | string | Exact issuer URI. The relay selects an issuer policy by exact match; no normalization is applied. | +| `sub` | string | Opaque, stable, non-reassignable subject identifier for the account lifetime. Never an email address or display name. | +| `nostr_pubkey` | string | Lowercase hexadecimal encoding of exactly one 32-byte Nostr public key. Other encodings deny. | +| `aud` | string or array | Audience. MUST be present. The relay requires an exact match to the configured audience value for this issuer. | +| `iat` | NumericDate | Issuance time. | +| `exp` | NumericDate | Expiry time. MUST be finite. The deployment MUST configure a positive finite maximum TTL; the relay enforces both the token `exp` and the configured `maximum_assertion_age`. | -1. **`FI-INV-01 — partial bijection.`** Active bindings are one-to-one within a - domain: one identity has at most one active key and one key has at most one - active identity. [FI-TRACE-BINDING-CONFLICT] -2. **`FI-INV-02 — durable binding.`** Assertion expiry removes neither a - binding nor its provenance. Fresh eligible evidence may authorize the same - binding later. [FI-TRACE-ASSERTION-REFRESH] -3. **`FI-INV-03 — tombstone monotonicity.`** Ordinary authorization never - removes a retired-pair or revoked-key fact and never recreates a retired - pair. [FI-TRACE-TOMBSTONE-REPLAY] -4. **`FI-INV-04 — server-owned context.`** Every admitted operation uses one - server-resolved domain, target, resource, operation, and proven actor. - Unauthenticated input cannot replace them. [FI-TRACE-DOMAIN-SPOOF] -5. **`FI-INV-05 — independent evidence.`** Direct authorization requires a - current assertion and fresh Nostr proof. If the assertion names a key, it - equals the proven actor. [FI-TRACE-ASSERTION-KEY-MISMATCH] -6. **`FI-INV-06 — stable assertion policy.`** Assertion-policy identity changes - when accepted assertion semantics change, but not when only authenticated - key or status snapshot contents rotate. [FI-TRACE-VERIFIER-PARITY] -7. **`FI-INV-07 — current-snapshot verification.`** Evidence cannot survive - removal of the key or policy snapshot that authenticated it; a changed - snapshot requires revalidation. [FI-TRACE-JWKS-REMOVE] -8. **`FI-INV-08 — read-only preparation.`** Preparation creates no binding, - tombstone, replay claim, receipt, lease, publication, last-seen value, audit - authority, or application mutation. [FI-TRACE-FINAL-DENIAL-NO-MUTATION] -9. **`FI-INV-09 — atomic final admission.`** Enrollment, replay claims, - receipts, and required authorization evidence commit only after complete - final revalidation, all or none. [FI-TRACE-PREPARED-STALE] -10. **`FI-INV-10 — explicit lifecycle authority.`** Retirement, revocation, - rotation, and profile-defined lifecycle changes occur only through their - separately authorized transition. [FI-TRACE-LIFECYCLE-AUTHORITY] -11. **`FI-INV-11 — evidence-bounded leases.`** A lease ends no later than every - evidence, snapshot, proof, binding, local-policy, and implementation bound - on which it depends. [FI-TRACE-LEASE-BOUND] -12. **`FI-INV-12 — current-owner delegation.`** When NIP-FI-DELEG is claimed, - delegation requires the exact current eligible owner binding, fresh - delegate proof, capability intersection, and a positive finite deadline. - [FI-DELEG-OWNER-CURRENT] -13. **`FI-INV-13 — privacy-safe denial.`** Public rejection is many-to-one and - reveals no identity, key, claim, binding, tombstone, enrollment mode, key - identifier, or private policy fact. [FI-TRACE-DENIAL-ORACLE] -14. **`FI-INV-14 — fail closed.`** Unreadable, ambiguous, stale beyond policy, - or inconsistent evidence or authoritative state cannot produce authority. - [FI-TRACE-DEPENDENCY-FAIL-CLOSED] -15. **`FI-INV-15 — uniform authority.`** Every protected ingress in a domain - uses the same current domain policy and final-admission authority. An - uncovered or competing path is unavailable. [FI-TRACE-AUTHORITY-UNIFORM] -16. **`FI-INV-16 — canonical verifier.`** Assertion transports feed one closed, - provider-neutral normalized-result contract and cannot fork final - admission. [FI-TRACE-VERIFIER-PARITY] +### Optional claims + +| Claim | Type | Semantics | +|---|---|---| +| `nbf` | NumericDate | Not-before time. When present, the relay enforces `nbf <= now + skew`. | + +### Token type + +Policy selects exactly one token class before parsing claims: + +- **`nip-fi+jwt`**: a dedicated assertion whose protected `typ` is exactly + `nip-fi+jwt`. +- **`at+jwt` access token**: a resource access token whose protected `typ` is + exactly `at+jwt`. When this class is selected: + - The assertion MUST contain a non-empty `client_id` claim. + - The issuer policy MUST name exactly one authenticated marker claim and two + non-empty, disjoint value sets: one for resource-owner subjects and one for + client-subject tokens. A token whose marker value matches neither set, both + sets, or whose marker claim is absent is ambiguous and denies. + - When client-subject tokens are admitted, the issuer policy MUST record the + non-collision posture: the issuer MUST guarantee that resource-owner and + client-subject `(iss, sub)` coordinates are disjoint. + - Absent, unknown, or ambiguous classification always denies; no fallback to + the other class is attempted. + +OIDC ID tokens always deny, even when `iss`, `aud`, and `sub` match. A +generic or absent `typ` has no accepted class. Failure under one class never +triggers validation under another. [FI-TRACE-TOKEN-CLASS] + +### Time bounds + +**Required claims:** `iat` and `exp` MUST be present; absence denies. + +**Policy knobs:** the relay enforces the following rules. `maximum_assertion_age` +is a required positive finite configuration; a missing or non-positive +configuration denies. `skew` is a non-negative finite maximum with default `0`; +it narrows acceptable bounds and cannot be omitted to mean "unchecked". + +- `now < exp` — equality at expiry is expired +- `iat <= now + skew` — issuance is not in the future beyond allowable skew +- `now < iat + maximum_assertion_age` — caps total assertion age independent of `exp` +- `nbf <= now + skew` — when `nbf` is present (optional claim; absence is not an error) + +[FI-TRACE-ASSERTION-VALIDATION] + +### Assertion–key binding + +`nostr_pubkey` MUST name the exact key the client proves via NIP-42. The relay +denies any token whose `nostr_pubkey` does not match the NIP-42 `pubkey`. +[FI-TRACE-ASSERTION-KEY-MISMATCH] + +This is the entire identity-to-key binding. There is no relay-side binding +ledger; the assertion is the binding claim, and it is the assertion issuer's +responsibility to ensure the assertion names the correct key. + +### Policy identity + +```text +AssertionPolicyId = H(canonical assertion-policy contract) +TransportContractId = H(canonical transport contract) +``` + +`AssertionPolicyId` covers the canonical issuer, audience, token class, +allowed algorithms, key-source contract, identity/key/claim mapping, time and +size rules, and compiled verifier behavior. JWKS key rotation changes the +snapshot, not the policy ID. `TransportContractId` covers the client-attached +field, parsing, attachment, and no-fallback semantics. ## Client-attached transport -Server configuration selects `client-attached` before protected traffic is -accepted. Request fields cannot select, negotiate, or downgrade transport. -Failure never falls back to another transport. [FI-TRACE-TRANSPORT-CLOSED] - -The client sends exactly one field on the request or WebSocket upgrade: +The client sends exactly one field on the WebSocket upgrade request: ```text Nostr-Federated-Identity: Bearer ``` -`Authorization` remains reserved for NIP-98. Assertion and provenance fields -from any other profile are absent. Missing, repeated, comma-combined, empty, -malformed, non-Bearer, or mixed-profile fields deny. Assertions never appear in -URLs, query parameters, Nostr events, tags, filters, application history, or -public identity projections. [FI-TRACE-TRANSPORT-CLOSED] +`Authorization` remains reserved for NIP-98. Missing, repeated, +comma-combined, empty, malformed, non-Bearer, or mixed-profile fields deny. +Assertions MUST NOT appear in URLs, query parameters, Nostr events, tags, or +filters. [FI-TRACE-TRANSPORT-CLOSED] -The core transport contract has deployment-local identity -`transport_contract_id`. It deterministically identifies the exact field, -parsing, request-attachment, no-fallback, and context-preservation semantics. -Changing any of those semantics changes the ID; changing request data does not. -[FI-TRACE-CONTRACT-IDENTITIES] +Server configuration selects `client-attached` before any protected traffic is +accepted. Request fields cannot select, negotiate, or downgrade the transport. +Failure never falls back to another transport. -## Assertion validation +## Verification -A configured assertion policy accepts exactly one bounded compact JWS and -returns this closed result: +The relay verifies assertions **offline** against configured per-issuer JWKS +snapshots. No IdP contact occurs at admission time. + +### Multi-issuer registry + +The relay maintains one [`IssuerRegistry`](../../crates/buzz-auth/src/nip_fi/config.rs): +a map from exact `iss` strings to issuer policies. The `iss` carried in the +signed token selects exactly one policy; unknown issuers deny. A +single-issuer deployment is a registry of length one. [FI-TRACE-CROSS-DOMAIN-COLLISION] + +The existing `FederatedAssertionVerifier` and `ProductionJwksSource` +(merged in PR 3 / `70895b355`) implement the verification procedure described +here. The `require_attested_key` flag in `IssuerPolicy` is the per-issuer +enforcement primitive for the unconditional `nostr_pubkey` requirement in this +section; conformance to NIP-FI v2 requires startup validation that forces this +flag true for every configured issuer. That integration is a follow-on code +change outside this PR. + +### JWKS snapshot + +Each issuer policy configures: + +- `jwks_uri`: HTTPS URI selecting the authenticated key source. SSRF-protected + at both URI validation and DNS-resolution time; no credentials, fragments, + or private-IP endpoints accepted. +- `refresh_interval_seconds`: positive, ≤ 1 year, strictly less than + `key_snapshot_hard_deadline_seconds`. +- `key_snapshot_hard_deadline_seconds`: the outer time bound after which no + assertion verified under this snapshot can authorize. + +The snapshot is re-fetched periodically. A key added to the JWKS is accepted +after the next fetch; a key removed from the JWKS causes any assertion verified +under that key to deny on next revalidation. [FI-TRACE-JWKS-ADD] +[FI-TRACE-JWKS-REMOVE] + +The snapshot is authenticated: no external consumer can relabel one issuer's +JWKS as another's. The maximum number of keys per snapshot is bounded before +any attacker-controlled `kid` lookup. + +### Verification procedure ```text -VerifiedAssertion = ( - identity = (iss, sub), - asserted_key?, - claims_or_capabilities, - authority_deadlines, // non-empty - assertion_policy_id, - transport_contract_id, - revalidation_dependencies +VerifyAssertion(token, D, R_t): + // 1. Select issuer policy + (header, claims) := BoundedJwsDecode(token) or DENY(evidence_rejected) + policy := IssuerRegistry[claims.iss] or DENY(evidence_rejected) + + // 2. Validate token class, typ, and algorithm + ValidateTokenClass(policy, header) or DENY(evidence_rejected) + AssertAsymmetricAlgorithm(header.alg) or DENY(evidence_rejected) + + // 3. Validate signature against current authenticated JWKS + snapshot := policy.key_source.get_snapshot() or DENY(authorization_unavailable) + key := snapshot.find(header.kid) or DENY(evidence_rejected) + VerifySignature(token, key) or DENY(evidence_rejected) + + // 4. Validate claims + AssertExactIss(claims.iss, policy.iss) or DENY(evidence_rejected) + AssertAudienceMatch(claims.aud, policy.aud) or DENY(evidence_rejected) + AssertTimeBounds(claims, policy) or DENY(evidence_rejected) // [FI-TRACE-ASSERTION-VALIDATION] + k_claimed := ParseHexKey(claims.nostr_pubkey) or DENY(evidence_rejected) + + return VerifiedAssertion(identity=(claims.iss, claims.sub), asserted_key=k_claimed, + authority_deadlines=ComputeDeadlines(claims, snapshot)) +``` + +The verifier is **fail-closed**: any unreadable, missing, ambiguous, or +expired input denies. A missing JWKS snapshot denies with +`authorization_unavailable`; all other failures deny with `evidence_rejected`. +[FI-TRACE-DEPENDENCY-FAIL-CLOSED] + +### Admission at connection + +On WebSocket upgrade: + +1. Extract `Nostr-Federated-Identity` header; missing or malformed → deny + `missing_evidence` or `evidence_rejected`. +2. Call `VerifyAssertion`; any error → deny per the rejection table. +3. Complete NIP-42 handshake; validate AUTH event, extract `k`. +4. Assert `verified.asserted_key == k`; mismatch → deny `authorization_denied`. + [FI-TRACE-ASSERTION-KEY-MISMATCH] +5. Admit the connection. The session's authority deadline is the minimum of all + `authority_deadlines`; see Session policy. + +## Session policy + +### Maximum connection lifetime + +Every NIP-FI deployment MUST configure a positive finite +`max_connection_lifetime_seconds`. This is a **required deployment knob**; +there is no default that permits an indefinite session. Operators MUST select +a value; infosec policy governs the specific bound. + +A connected session MUST be terminated no later than `connection_time + max_connection_lifetime_seconds`, +regardless of assertion expiry. + +The effective session deadline is: + +``` +session_deadline = min( + connection_time + max_connection_lifetime_seconds, + min(authority_deadlines), // from VerifiedAssertion + key_snapshot_hard_deadline // from the issuer policy ) ``` -The verifier rejects ambiguous protected-header or claim members, unknown -critical headers, `alg=none`, symmetric algorithms, algorithm/key mismatch, -incompatible JWK usage, ambiguous key selection, and signatures not valid -under exactly one accepted asymmetric key. It bounds the assertion, headers, -claims, subject, key identifiers, and authenticated key set before lookup or -logging. [FI-TRACE-ASSERTION-VALIDATION] +Equality at any deadline is expired. Arithmetic is overflow-safe. +[FI-TRACE-LEASE-BOUND] -The exact `iss` selects an authenticated policy and key source; `iss` and at -least one `aud` value exactly match configured values. `sub` is a non-empty -bounded string. Each policy configures a non-negative finite `skew`, a positive -finite `maximum_assertion_age`, and, for `current-status`, a positive finite -`maximum_status_age`; a missing value denies. `exp` and `iat` are finite -NumericDate values satisfying `now < exp`, `iat <= now + skew`, and -`now < iat + maximum_assertion_age`. Optional `nbf` satisfies -`nbf <= now + skew`. Arithmetic is overflow-safe and equality at an expiry is -expired. [FI-TRACE-ASSERTION-VALIDATION] +### Re-authentication -The Nostr-key claim is named `nostr_pubkey`. When present it MUST be a -lowercase hexadecimal encoding of exactly one 32-byte Nostr public key; other -encodings and aliases deny. In `attested-key` enrollment policy and wherever -current matching issuer attestation is required, this exact claim MUST be -present and equal the proven actor; authorization claims or capabilities use a -closed bounded input set and deterministic canonical encoding. Unchecked claims -never enter the result. [FI-TRACE-VERIFIER-PARITY] +There is **no in-band session renewal**. When a session expires, the relay +closes the WebSocket. The client must open a new connection with a fresh +assertion on the upgrade request and complete a fresh NIP-42 proof. A silent +re-mint riding an existing issuer/IdP session is an issuer implementation +detail; the relay never sees anything other than a new upgrade request. -### Token class +### Reconnect after expiry -Policy selects exactly one token class before parsing claims: +A client whose session expired due to normal TTL expiry may reconnect +immediately provided the issuer can supply a fresh assertion. Session expiry +does not imply key revocation or identity loss; that is the issuer's domain. -- **`at+jwt` access token**: a Buzz-resource access token whose protected - `typ` is exactly `at+jwt` and whose `aud` contains the configured Buzz - resource audience. This class selects tokens carrying the RFC 9068 `at+jwt` - type but validates them under this document's claim contract; it does not - implement the full RFC 9068 validation profile, and the long-form media type - `application/at+jwt` is not accepted; -- **dedicated Buzz assertion**: a separately minted assertion whose protected - `typ` is exactly `nip-fi+jwt`. +## Admin disconnect API -OIDC ID Tokens always deny, even when `iss`, `aud`, and `sub` match. A generic -or absent type has no accepted class: claim presence alone cannot prove a token -disjoint from an OIDC ID token, since an issuer can mint an ID token carrying -`client_id`, and the only authenticated discriminator is `typ`. Failure under -one class never triggers validation under another. An `at+jwt` access token MUST -contain one non-empty bounded `client_id`. Issuer policy MUST distinguish a -resource-owner token from a token -whose subject represents the OAuth client, including a client-credentials token, -using authenticated claim semantics and mutually exclusive validation rules. A -token that admits both interpretations denies. If client-subject tokens are -accepted, the issuer MUST guarantee that their `(iss, sub)` coordinates cannot -collide with resource-owner coordinates; otherwise that token class is -ineligible. Token class and every class-specific validation rule are inputs to -`assertion_policy_id`. [FI-TRACE-TOKEN-CLASS] +The assertion issuer can terminate live relay sessions for a specific public key via an +authenticated `disconnect` call. -### Policy identity and snapshots +### Semantics (session-only) -Core has exactly two semantic contract identities: +A disconnect call causes the relay to close all live WebSocket connections +whose proven `k` equals the target pubkey. This is a **session-only** +operation: it closes existing connections but does not prevent the key from +reconnecting. After disconnection, a client holding a still-valid JWT can +reconnect immediately. + +> **Non-normative note — open product question (session-only vs deny-until-TTL):** +> +> The session-only model means a revoked user retains access until their +> assertion's effective authority expires. After a successful disconnect call +> (all matching sessions closed synchronously), there is no surviving +> old-session window. If the issuer also stops issuing new assertions at +> that point, cumulative residual access is bounded by: +> +> ``` +> max(0, min(exp, iat + maximum_assertion_age) - now) +> ``` +> +> `max_connection_lifetime_seconds` only partitions that interval into +> individual sessions; it does not shorten the total window. A snapshot +> refresh failure, hard-deadline expiry without key replacement, or signing-key +> removal can terminate access earlier, but these are not reliable protocol-level +> bounds: the JWKS snapshot deadline renews on each refresh even when content is +> unchanged, so it does not cap cumulative access. If the issuer +> continues issuing new assertions after the disconnect call, cumulative +> access extends indefinitely — the session-only protocol places no +> protocol-level bound on that case. +> +> If the disconnect call is asynchronous or best-effort, the spec would need +> to define a completion-bound contract; the current normative text assumes +> synchronous close. +> +> The alternative is a **deny-until-TTL** model: the relay holds a +> memory-resident deny-list entry for the pubkey keyed to the issuer's stated +> TTL, and any reconnect attempt for that key is denied `authorization_denied` +> until the entry expires. This eliminates the reconnect window at the cost of +> relay in-memory state and a TTL-propagation contract between issuer and relay. +> +> This document intentionally leaves that decision unresolved. The current +> normative text describes session-only. If deny-until-TTL is chosen, Section 6 +> must be revised to add: the TTL parameter on the disconnect call, the +> deny-list data structure (keyed by pubkey, value = absolute expiry), the +> deny-list check at admission (step 4), and the expiry/eviction rule. + +### Transport + +The disconnect endpoint is an authenticated issuer→relay API, not a public +Nostr protocol. + +### Command JWT + +Authentication uses a short-lived signed command JWT with a dedicated token +type. The relay verifies it with a **dedicated command verifier** that reuses +the same `IssuerRegistry`, bounded JWS parsing, issuer-bound JWKS snapshots, +signature verification, audience, and time-bound primitives as assertion +verification, but operates over a distinct token type and produces a closed +command result. The `VerifyAssertion` primitive is not used here. + +The command JWT protected header MUST carry `"typ": "nip-fi-command+jwt"`. +Any other `typ` value denies before claim parsing. + +The command JWT MUST carry the following claims: + +| Claim | Requirement | +|---|---| +| `iss` | Exact issuer URI matching an authorized issuer in the registry. | +| `sub` | Issuer principal identifier. The relay checks this is an authorized issuer principal. | +| `aud` | Audience matching the relay's configured audience value for this issuer. | +| `iat` | Issuance time. MUST satisfy `iat <= now + skew`. | +| `exp` | Expiry time. MUST be finite; relay enforces `now < exp`. | +| `jti` | Unique, non-guessable identifier for this command. Used for replay prevention; see below. | +| `method` | Exactly `"POST"` (uppercase literal). Binds the command to the HTTP method. | +| `path` | Exactly `"/api/nip-fi/disconnect"` (literal string). Binds the command to the endpoint. | +| `cmd` | Exactly `"disconnect"` (literal string). Operation selector. | +| `target_pubkey` | Lowercase hexadecimal encoding of the target 32-byte Nostr public key — the same encoding required for the assertion `nostr_pubkey` claim. | + +The `maximum_command_age` policy knob is a required positive finite +configuration per authorized issuer, with a normative upper bound of +60 seconds. The relay enforces `0 < maximum_command_age <= 60` and +`now < iat + maximum_command_age` in addition to `now < exp`. A missing, +non-positive, or out-of-range configuration denies. + +The `VerifyCommandJwt` procedure: ```text -assertion_policy_id = H(canonical assertion-policy contract) -transport_contract_id = H(canonical transport contract) +VerifyCommandJwt(token, request_method, request_path, request_body_pubkey): + // 1. Bounded decode and type check + (header, claims) := BoundedJwsDecode(token) or DENY(evidence_rejected) + assert header.typ == "nip-fi-command+jwt" or DENY(evidence_rejected) + + // 2. Select issuer policy; verify signature + policy := IssuerRegistry[claims.iss] or DENY(evidence_rejected) + AssertAsymmetricAlgorithm(header.alg) or DENY(evidence_rejected) + snapshot := policy.key_source.get_snapshot() or DENY(authorization_unavailable) + key := snapshot.find(header.kid) or DENY(evidence_rejected) + VerifySignature(token, key) or DENY(evidence_rejected) + + // 3. Validate claims (pure verification — no side effects) + AssertExactIss(claims.iss, policy.iss) or DENY(evidence_rejected) + AssertAudienceMatch(claims.aud, policy.aud) or DENY(evidence_rejected) + AssertCommandTimeBounds(claims, policy) or DENY(evidence_rejected) + // enforces: now < exp, iat <= now + skew, now < iat + maximum_command_age + assert claims.method == request_method or DENY(evidence_rejected) + assert claims.path == request_path or DENY(evidence_rejected) + assert claims.cmd == "disconnect" or DENY(evidence_rejected) + target_k := ParseHexKey(claims.target_pubkey) or DENY(evidence_rejected) + + // 4. Principal authorization (pure check — no side effects) + AssertAuthorizedIssuerPrincipal(claims.iss, claims.sub) or DENY(authorization_denied) + + // 5. Signed-target / request-body agreement (pure check — no side effects) + assert target_k == request_body_pubkey or DENY(authorization_denied) + + // 6. Atomically reserve jti — final admission step, immediately before side effects. + // The reservation is keyed by (iss, jti) and held until the command's + // effective expiry: min(exp, iat + maximum_command_age). This step MUST + // be the last mutation before disconnect side effects; performing it before + // steps 4 or 5 would burn the signed command identity on failed-authorization + // or mismatched-body requests, violating the fail-closed contract. + effective_expiry := min(claims.exp, claims.iat + policy.maximum_command_age) + AtomicReserveJti(claims.iss, claims.jti, effective_expiry) or DENY(authorization_denied) + + return CommandResult(target_pubkey=target_k, caller=(claims.iss, claims.sub)) ``` -Each uses one implementation-defined but deterministic, versioned encoding and -collision-resistant hash within a deployment. `assertion_policy_id` covers the -canonical issuer, audience, token class, allowed algorithms, authenticated -key/status-source contracts, identity/key/claim mapping, time and size rules, -normalization, freshness class, and compiled verifier behavior. The verifier -fingerprint is an input, not a third identity. `transport_contract_id` covers -the client-attached field, parsing, attachment, context preservation, and -no-fallback semantics; a companion transport may define its own canonical -contract under that same identity slot. A semantic change changes exactly its -owning ID. [FI-TRACE-CONTRACT-IDENTITIES] +Any failure at any step is fail-closed: no side effects occur and the relay +returns the appropriate error. -Mutable contents and deployment state are not contract identities. They remain -in `revalidation_dependencies`: authenticated assertion-snapshot version, -verification-key identity, key-snapshot hard deadline, optional status -source/version/deadline, binding/lifecycle/local-policy/resource versions, -proof and replay witnesses, and a confidential handle to the exact compact JWS. -Adding, removing, or replacing an accepted key changes the snapshot version, -not `assertion_policy_id`. Changed dependencies require revalidation under -current state; a retained key may continue, while an absent key denies. -Unknown-key refresh is bounded and coalesced and has no attacker-triggered -stale-key fallback. [FI-TRACE-JWKS-ADD] [FI-TRACE-JWKS-REMOVE] +This verifier and the disconnect API endpoint are follow-on code changes +outside this PR, in the same way that the `require_attested_key` enforcement +integration is. -The base contract compares the current authenticated snapshot and makes no -anti-rollback promise. A deployment claiming rollback prevention records a -separately authenticated monotonic floor and tests it. [deployment artifact: -assertion-policy review] - -### Freshness class - -Each policy declares exactly one server-owned freshness class, included in -`assertion_policy_id`: - -- **`offline-jwt`** validates the JWT and authenticated key snapshot only. - `upstream_authority_deadline` is the minimum of `exp`, - `iat + maximum_assertion_age`, and the key-snapshot hard deadline. Token age - bounds assertions minted before revocation; it cannot bound an issuer that - continues minting accepted assertions afterward. Enabling this class therefore - requires deployment evidence that revocation stops new accepted issuance, and - discovery reports the unconditional residual bound as unknown (`null`). It - MUST NOT advertise a finite unconditional residual bound. [deployment - artifact: issuer revocation review] -- **`current-status`** additionally requires an authenticated witness - `(iss, sub, token_or_session_id?, active=true, observed_at, valid_until, - status_version, authenticated_source_id)`. Issuer, subject, and optional - session identifier exactly match the assertion. Ambiguous, unauthenticated, - inactive, or expired status denies. `valid_until` is finite and no later than - `observed_at + maximum_status_age`. The upstream deadline is the minimum of - the offline assertion deadlines and `valid_until`. Source outage cannot mint - or extend a witness; an already verified witness remains usable only until - its existing `valid_until`. [FI-TRACE-CURRENT-STATUS-STALE] - -A current-status deployment advertises a tested positive -`maximum_residual_upstream_revocation_seconds`. Prepared evidence and leases -close within that value after upstream revocation, including a revocation racing -final admission. Poll/cache age, event-delivery and processing delay, and -enforcement delay all fit within the advertised value. A push implementation -may close authority sooner but cannot claim a value below its tested worst case. -[FI-TRACE-CURRENT-STATUS-REVOKED] - -An external capability projection whose removal is required to close authority -within a declared revocation bound MUST enter authoritative local-policy state, -not `claims_or_capabilities` from the assertion. That state is reread during -preparation, final admission, and protected lease use. A deployment that carries -such a projection only in assertions cannot claim a revocation bound for its -changes. [FI-TRACE-CAPABILITY-REVOCATION] - -Before enabling an issuer, the operator records authoritative evidence that -`sub` is stable for the account lifetime, never reassigned, and not intentionally -derived from mutable profile data. An issuer that cannot provide this property -is ineligible. [deployment artifact: issuer subject-stability review] - -## Nostr proof and body semantics - -The actor is always returned by fresh Nostr-proof validation, never by an -assertion or unsigned field. NIP-42 binds its AUTH event to the current -challenge, relay URL, connection, and freshness window. NIP-98 binds its event -to the exact server-resolved URL, method, and freshness window. All evidence -agrees with the same `D` and `R_t`. [FI-TRACE-DOMAIN-SPOOF] - -Each protected HTTP operation declares in server policy whether its body is -authorization-relevant; clients cannot select the declaration. - -For a relevant body, the NIP-98 event contains exactly one `payload` tag equal -to lowercase hexadecimal SHA-256 of the **body bytes**: the complete content -after transfer decoding and before any content decoding. Absence, duplication, -mismatch, validation of only a prefix, or substitution of the body bytes after -validation denies. For an irrelevant body, no -authorization decision, target, capability, or effect selector derives from a -body field not bound by NIP-98. A `payload` tag present on an operation whose -body is declared authorization-irrelevant is validated identically against the -body bytes; duplication or mismatch denies. [FI-TRACE-BODY-BINDING] - -Every operation has finite body and spool bounds. A known oversized body is -rejected before hashing; a stream is rejected at octet `limit + 1`; admission -waits for EOF. Before EOF there is no application effect, replay mutation, -receipt, or partial digest authority. Quota failure cleans up staged bytes and -denies. [FI-TRACE-BODY-BOUNDS] - -## Direct preparation - -The following is normative pseudocode; every read is from authoritative state. +### Request ```text -PrepareDirect(request, assertion, proof): - (D, R_t, operation, resource) := ResolveTargetContext(request) or DENY - e := ValidateClientAttached(assertion, D, R_t) or DENY - k := ValidateNostrProof(proof, D, R_t) or DENY - R := SealActor(R_t, k) - i := e.identity +POST /api/nip-fi/disconnect HTTP/1.1 +Nostr-Federated-Identity: Bearer +Content-Type: application/json - if e.asserted_key exists and e.asserted_key != k: DENY(key_mismatch) - atomically read B_D(i), B_D(k), T_D(i,k), Y_D(k), enrollment policy, - local policy, resource, and all dependency versions - if k in Y_D: DENY(key_revoked) - if (i,k) in T_D: DENY(pair_retired) - - if B_D(i) = B_D(k) = binding(i,k): - proposal := existing(binding.version, binding.provenance) - else if B_D(i) exists or B_D(k) exists: - DENY(binding_conflict) - else if enrollment policy = attested-key: - if e.asserted_key != k: DENY(attestation_required) - proposal := enroll(i, k, attested-key) - else if enrollment policy = tofu: - proposal := enroll(i, k, e.asserted_key = k ? attested-key : tofu) - else: - DENY(binding_required) - - EvaluateLocalPolicy(D, R, operation, resource, k, - e.claims_or_capabilities) or DENY - return PreparedAuthorization(evidence, proposal, witnesses, deadlines) +{"pubkey": ""} ``` -TOFU is optional private deployment posture and is not self-advertised. It -accepts that a stolen assertion for a never-enrolled identity can bind an -attacker's proven key; deployments enabling it retain a passing -FI-TRACE-TOFU-THEFT artifact. Binding provenance is immutable. A policy change -affects only future creation. [deployment artifact: TOFU risk review] +The relay calls `VerifyCommandJwt` passing the request method, path, and +body `pubkey` field; any failure denies per the rejection table. On success, +the relay closes all live connections whose proven `k` equals +`CommandResult.target_pubkey`. An unknown or unprovable pubkey is not an +error; the relay responds `200` with `{"disconnected": 0}`. -Preparation, including first-use enrollment, is read-only and produces no -authoritative mutation. [FI-TRACE-FINAL-DENIAL-NO-MUTATION] +### Response -## Final admission - -A prepared value is consumed at most once. Final admission first requires an -exact domain, context, operation, resource, actor, and transport match; both -contract IDs unchanged; and every bound live. A changed dependency is reread -and re-evaluated from authoritative evidence. [FI-TRACE-PREPARED-STALE] - -Two verified assertion results are **equivalent** when: - -1. identity-class fields are byte-equal: `iss`, `sub`, asserted-key presence and - value, canonical claims/capabilities, `assertion_policy_id`, and - `transport_contract_id`; -2. each bounds-class deadline — every `authority_deadlines` member, the - key-snapshot hard deadline, and any status deadline — is live now and is no - later than its prepared value; and -3. provenance-class fields — snapshot version, verification-key identity, - status source and version, binding, lifecycle, local-policy, and resource - versions, proof and replay witnesses, the confidential JWS handle, cache - metadata, ordering, and retrieval time — are ignored after successful - current revalidation. - -Every `revalidation_dependencies` member is bounds-class if it is a deadline -and provenance-class otherwise. Any new or unclassified assertion-content field -belongs to the identity class. A fresher assertion cannot silently extend a -prepared decision. [FI-TRACE-PREPARED-STALE] - -Final admission atomically rereads binding, tombstone, revocation, enrollment, -policy, resource, status, replay, receipt, and invalidation witnesses; -recomputes the complete decision; claims applicable proof replay identities; -creates an eligible proposed binding; and appends its request-bound receipt and -required authorization evidence. All commit or none. A concurrent identical -enrollment may recompute as the same `existing` binding; conflicting enrollment -commits at most one winner. [FI-TRACE-CONCURRENT-ENROLLMENT] - -A failed admission rolls back all authority mutation. The application operation -runs only after committed authorization. If it cannot share the transaction, a -request-bound idempotent receipt prevents the same proof from creating a second -effect. [FI-TRACE-FINAL-DENIAL-NO-MUTATION] - -## Base lifecycle - -Retirement, revocation, and rotation require separate privileged authority -bound to the exact domain, transition, identity, old binding version when -present, target key when present, and request. Each atomically rechecks current -state, appends immutable lifecycle history, and invalidates dependent leases -after commit. Every new target key supplies fresh target-bound Nostr proof and -any policy-required current matching issuer attestation. [FI-TRACE-LIFECYCLE-AUTHORITY] - -- **RetirePair** removes one exact active binding and durably retires its pair. -- **RevokeKey** records the key as revoked even if inactive; if active, it also - removes the binding and retires that pair. Repeating the same authorized - revocation is idempotent. -- **Rotate** replaces one exact active binding with one unused, unrevoked, - non-retired target key, retires the old pair, and creates a fresh binding - version. The replacement provenance is `attested-key` when current matching - issuer attestation was validated and `provisioned` otherwise. Rotation does - not globally revoke the old key. Rotation continues one grant onto a new key - rather than establishing a new one: the replacement preserves every - profile-defined administrative bound carried by the binding it replaces, as an - opaque field core neither interprets nor clears. Only the authority that set - such a bound can change it. [FI-TRACE-LIFECYCLE-AUTHORITY] - -Failure or stale state causes no partial mutation. Ordinary authorization cannot -perform or undo these transitions. Extended disablement, re-enablement, -provisioning, and administrative expiry are defined only by NIP-FI-LIFECYCLE. - -## Request and session bounds - -HTTP authority covers one exact request and is never reusable. - -A WebSocket lease is scoped to one actor, domain, operation set, binding -version, normalized result, policy/resource versions, and invalidation -witnesses. Its deadline is the earliest assertion, upstream-authority, -key-snapshot, proof/connection, local-policy, and implementation deadline. -Arithmetic is overflow-safe and equality is expired. [FI-TRACE-LEASE-BOUND] - -Before each protected use, the service checks actor, domain, operation, -resource, deadline, binding version, contract IDs, snapshot/status versions, -policy versions, and invalidation state. Changed dependencies require current -revalidation to an equivalent result; unreadable or ineligible state denies. -A lease for one key never authorizes another key on the same connection. -[FI-TRACE-MULTI-KEY-SESSION] - -Expiry ends the lease, not the binding. Renewal requires a new connection with -a fresh assertion attached to its WebSocket upgrade, fresh Nostr proof, -preparation, and final admission; there is no in-band renewal path. Confidential -assertion revalidation material is destroyed on expiry, close, or invalidation. +| Condition | Status | Body | +|---|---|---| +| Authorized; action taken or no-op | `200` | `{"disconnected": }` where `n` is the count of sessions closed | +| Missing or invalid command JWT | `401` / `403` | Per the rejection table | +| Malformed request body | `400` | `bad request\n` | ## Rejection and privacy Public class is a function only of evidence the requester supplied, never of private per-principal server state; `authorization_unavailable` is the sole -exception and reveals only that a required authoritative dependency is -unreadable, never any per-principal fact. Replay status is a function of -committed per-principal server state, not of the supplied evidence alone; -replayed evidence is therefore classed `authorization_denied`, indistinguishable -from any other private-state denial, so that resubmitting captured evidence -reveals nothing about whether the original request committed. Under the -private-posture rule, even `key_mismatch` joins the private-state anonymity set. +exception and reveals only that a required dependency is unreadable. -| Private condition | Public class | Nostr prefix and exact text | HTTP response | +| Private condition | Public class | Nostr text | HTTP response | |---|---|---|---| -| assertion/proof absent | `missing_evidence` | `auth-required: authentication required` | `401`; `WWW-Authenticate: Nostr`; `Content-Type: text/plain; charset=utf-8`; `authentication required\n` | -| malformed, invalid, or expired evidence | `evidence_rejected` | `restricted: evidence rejected` | `403`; `Content-Type: text/plain; charset=utf-8`; `evidence rejected\n` | -| replayed evidence; key mismatch; attestation required; binding conflict; retired pair; revoked key; lifecycle gate; binding required/expired; local policy denial | `authorization_denied` | `restricted: authorization denied` | `403`; `Content-Type: text/plain; charset=utf-8`; `authorization denied\n` | -| required current dependency unreadable | `authorization_unavailable` | `restricted: authorization unavailable` | `503`; `Content-Type: text/plain; charset=utf-8`; `authorization unavailable\n` | +| assertion or proof absent | `missing_evidence` | `auth-required: authentication required` | `401`; `WWW-Authenticate: Nostr`; `Content-Type: text/plain; charset=utf-8`; body `authentication required\n` | +| malformed, invalid, or expired evidence | `evidence_rejected` | `restricted: evidence rejected` | `403`; `Content-Type: text/plain; charset=utf-8`; body `evidence rejected\n` | +| assertion–key mismatch; local policy denial; issuer-initiated disconnect (session-only model) | `authorization_denied` | `restricted: authorization denied` | `403`; `Content-Type: text/plain; charset=utf-8`; body `authorization denied\n` | +| required JWKS snapshot unreadable | `authorization_unavailable` | `restricted: authorization unavailable` | `503`; `Content-Type: text/plain; charset=utf-8`; body `authorization unavailable\n` | -Nostr text is the exact UTF-8 text after an applicable NIP-42/NIP-01 prefix. -A denial decided on a WebSocket upgrade request, before any NIP-42 proof -exists, is the HTTP response in the table, sent instead of `101`; a denial -decided after the connection is established is the Nostr text. For HTTP, the -compared denial contract is closed over the -status, complete body, -and exact values of only the header fields named in the table; header order and -other fields are outside that contract and their values cannot depend on the -private condition. The body is the shown UTF-8 bytes with one LF and no other -bytes. The `Nostr` challenge satisfies RFC 9110 Section 15.5.2. -Responses contain no free text, reason code, request ID, issuer, subject, key, -claim, binding state, enrollment posture, token material, or timing hint. All -private conditions in `authorization_denied` produce byte-identical responses. -[FI-TRACE-DENIAL-ORACLE] +A denial decided on a WebSocket upgrade is the HTTP response in place of `101`. +A denial decided after the connection is established is the Nostr text. +Responses contain no free text, reason code, issuer, subject, key, claim, or +timing hint. [FI-TRACE-DENIAL-ORACLE] -NIP-FI defines no public identity projection. Public events, tags, filters, -discovery, responses, logs, metrics, and traces contain no raw assertions or -unredacted `iss`, `sub`, email, display name, or private claim. Access-controlled -authoritative stores retain only what enforcement and investigation require. -A separate presentation protocol cannot confer NIP-FI authority. -[FI-TRACE-PRIVACY-NONPUBLIC] +NIP-FI defines no public identity projection. Raw assertions, `iss`, `sub`, +email, display name, and private claims MUST NOT appear in public events, tags, +filters, discovery, logs, metrics, or traces. [FI-TRACE-PRIVACY-NONPUBLIC] + +## Out of scope + +The following are issuer and deployment concerns. This spec defines no +normative behavior for them: + +- Identity↔key registry, key ownership records, and the one-identity one-key + constraint: issuer-side. +- Key rotation, re-enrollment after device loss: issuer-side. +- Revocation signaling to the issuer/IdP: issuer-side; the issuer stops + issuing assertions, which closes the relay window within assertion TTL. +- Directory integration and account-offboarding automation: issuer-side. +- Audit logging beyond what the relay operator chooses to retain: issuer-side. +- Delegation: out of scope. +- Companion profiles (NIP-FI-EDGE, NIP-FI-LIFECYCLE, NIP-FI-DELEG, NIP-FI-CONF, NIP-FI-MODEL): removed. ## Discovery @@ -513,119 +483,84 @@ A relay SHOULD advertise core support in NIP-11 as: } ``` -NIP-FI-EDGE owns the optional `edge_transports` member and its exact type, -placement, and value semantics. For `current-status`, the final value is a tested -positive integer. Discovery never states enrollment mode or TOFU posture and -never exposes issuer URLs, audiences, claim names, tenant IDs, or -deployment-local identifiers. For a fixed -set of claimed profiles, the complete public discovery output is byte-identical -for every enrollment policy, including `attested-key`, private `tofu`, and any -companion profile mode: no field, flag, value, omission, ordering, or object shape -may distinguish the configured mode. Profile documents own only non-enrollment -public claims. -[FI-TRACE-DISCOVERY-PRIVATE] +Discovery MUST NOT state issuer URLs, audiences, claim names, tenant IDs, or +deployment-local identifiers. [FI-TRACE-DISCOVERY-PRIVATE] -## Worked example (non-normative) - -A protected HTTP POST under `client-attached` with NIP-98 proof and an -authorization-relevant body. Credentials are elided; the NIP-FI-CONF exit -fixture pins the complete request compared objects. - -```text -POST /media HTTP/1.1 -Host: relay.example -Nostr-Federated-Identity: Bearer eyJhbGciOiJFUzI1NiIsInR5cCI6ImF0K2p3dCIs... -Authorization: Nostr eyJpZCI6IjE1ZTI3ZDc0Li4uIiwicHVia2V5IjoiOTljNzQ4Li4u... -Content-Type: application/octet-stream -Content-Length: 4 - -abcd -``` - -The bearer JWS validates under the configured assertion policy: exact `iss` -and `aud`, token class `at+jwt`, live time claims, and `nostr_pubkey` equal to -the NIP-98 event's `pubkey`. The NIP-98 event binds the server-resolved method -and URL, and its single `payload` tag equals the SHA-256 of the four body -bytes. Admission then follows Direct preparation and Final admission; success -returns the application response, and every failure class returns exactly the -bytes fixed in the Rejection table. On a WebSocket upgrade the same header -attaches to the upgrade request and NIP-42 supplies the proof after connect. - -## Core behavioral oracles - -A core claim covers every applicable oracle below at one implementation and -policy revision. NIP-FI-CONF defines evidence and mutation-adequacy rules. +## Behavioral oracles | ID | Required outcome | |---|---| -| `FI-TRACE-TRANSPORT-CLOSED` | Exact one-header input succeeds; missing, repeated, combined, malformed, mixed, URL, and fallback variants deny. | -| `FI-TRACE-ASSERTION-VALIDATION` | Valid boundary input passes; each signature, key-selection, issuer, audience, time, size, ambiguity, and missing-configuration negative denies. | -| `FI-TRACE-TOKEN-CLASS` | An `at+jwt` access token and a dedicated `nip-fi+jwt` assertion pass only their selected class. ID tokens, wrong or generic types, client-only audiences, absent or ambiguous `client_id`, resource-owner/client-subject ambiguity, and every attempted cross-class fallback deny. | -| `FI-TRACE-CONTRACT-IDENTITIES` | Mutate each assertion semantic, transport semantic, and mutable dependency independently: semantic mutations change only their owning contract ID; snapshot/binding/lifecycle/policy/resource/status mutations change neither ID but force current revalidation. | -| `FI-TRACE-VERIFIER-PARITY` | Equal authoritative input and policy produce the same canonical normalized result. | -| `FI-TRACE-JWKS-ADD` | Retained-key rotation revalidates successfully under the changed snapshot version. | -| `FI-TRACE-JWKS-REMOVE` | Evidence and leases under a removed key deny after snapshot change. | -| `FI-TRACE-CURRENT-STATUS-REVOKED` | Revocation, including one racing final admission, closes authority within the advertised tested bound. | -| `FI-TRACE-CURRENT-STATUS-STALE` | Inactive/ambiguous status denies; an issuer, subject, or session-identifier mismatch denies; expiry equality, outage, delayed events, and changed status versions cannot mint or extend a witness. | -| `FI-TRACE-CAPABILITY-REVOCATION` | Removal of a revocation-bounded external capability projection from authoritative local policy closes prepared evidence and lease use within the declared bound; assertion-only projection cannot satisfy this oracle. | -| `FI-TRACE-BODY-BINDING` | Exact complete relevant body passes; absent/duplicate/mutated/partial/substituted payload variants deny without effects; a payload tag on an irrelevant-body operation validates identically and denies on duplication or mismatch. | -| `FI-TRACE-BODY-BOUNDS` | Oversized, over-quota, and pre-EOF variants deny with bounded work, cleanup, and no effects. | -| `FI-TRACE-DOMAIN-SPOOF` | Client routing and forwarded authority cannot replace server-owned context. | -| `FI-TRACE-ASSERTION-KEY-MISMATCH` | Mismatch denies with no mutation and the private-state response. | -| `FI-TRACE-BINDING-CONFLICT` | A binding conflict denies without replacing either existing binding. | -| `FI-TRACE-TOMBSTONE-REPLAY` | Fresh eligible evidence for a retired pair or revoked key denies without recreation. | -| `FI-TRACE-ASSERTION-REFRESH` | Fresh evidence reuses the same eligible durable binding after prior assertion expiry. | -| `FI-TRACE-PREPARED-STALE` | Changed identity-class witnesses or extended bounds deny; provenance-only rotation revalidates. | -| `FI-TRACE-CONCURRENT-ENROLLMENT` | Identical first use converges; conflicting first use commits at most one winner. | -| `FI-TRACE-FINAL-DENIAL-NO-MUTATION` | Every failed phase leaves all authoritative stores and effects unchanged. | -| `FI-TRACE-LIFECYCLE-AUTHORITY` | Unprivileged/stale transitions deny; authorized retirement/revocation/rotation is atomic. | -| `FI-TRACE-LEASE-BOUND` | A lease ends at its earliest bound; equality at any bound is expired. | -| `FI-TRACE-MULTI-KEY-SESSION` | One actor's lease never authorizes another key on the same connection. | -| `FI-TRACE-DENIAL-ORACLE` | Each private row produces its exact fixed bytes on every surface where its condition can be decided — HTTP, a WebSocket upgrade, or after connect; all private-state rows compare byte-identical. | -| `FI-TRACE-DEPENDENCY-FAIL-CLOSED` | Each unreadable authoritative dependency denies. | -| `FI-TRACE-AUTHORITY-UNIFORM` | Every protected ingress reaches one current final-admission authority. | -| `FI-TRACE-CROSS-DOMAIN-COLLISION` | Equal subjects across issuers and equal pairs across domains remain distinct. | +| `FI-TRACE-TRANSPORT-CLOSED` | Exact one-header input succeeds; missing, repeated, combined, malformed, and fallback variants deny. | +| `FI-TRACE-ASSERTION-VALIDATION` | Valid boundary input passes; each signature, key-selection, issuer, audience, time, size, and missing-configuration negative denies. | +| `FI-TRACE-TOKEN-CLASS` | `at+jwt` and `nip-fi+jwt` pass only their selected class; ID tokens, wrong or generic types, and cross-class fallback deny. | +| `FI-TRACE-ASSERTION-KEY-MISMATCH` | Mismatch between `nostr_pubkey` and the NIP-42 proven key denies with the private-state response. | +| `FI-TRACE-JWKS-ADD` | A key added to the JWKS is accepted after the next snapshot refresh. | +| `FI-TRACE-JWKS-REMOVE` | Connections verified under a removed key deny on next revalidation or reconnect. | +| `FI-TRACE-DEPENDENCY-FAIL-CLOSED` | An unreadable JWKS snapshot denies `authorization_unavailable`; no degraded Nostr-only access. | +| `FI-TRACE-LEASE-BOUND` | A session closes at its earliest deadline; equality at any deadline is expired. | +| `FI-TRACE-DENIAL-ORACLE` | Each public-class row produces its exact fixed bytes; all private-state rows compare byte-identical. | +| `FI-TRACE-DISCOVERY-PRIVATE` | Complete discovery bytes do not expose issuer, audience, or deployment-private state. | +| `FI-TRACE-CROSS-DOMAIN-COLLISION` | Equal `sub` values under different `iss` values remain distinct identities. | | `FI-TRACE-PRIVACY-NONPUBLIC` | Private identity does not enter public surfaces. | -| `FI-TRACE-DISCOVERY-PRIVATE` | Complete discovery bytes remain identical across attested-key, TOFU, and companion enrollment modes. | -| `FI-TRACE-TOFU-THEFT` | Stolen-assertion first use denies unless private TOFU is enabled and the attacker also proves its chosen key. | - -## Relationship to other work (non-normative) - -NIP-FI binds an access token to a key the resource server itself verifies, the -goal DPoP (RFC 9449) and mTLS-bound tokens (RFC 8705) reach through a `cnf` -claim. Here the proof is the NIP-42 or NIP-98 event the relay already -validates, so no second proof is defined and the issuer need not attest the -key; `nostr_pubkey` is the optional `cnf` analogue. Unlike those profiles the -binding is durable server state rather than a per-token claim: a stolen -assertion cannot reach an enrolled identity without its key, and revocation is -a local fact rather than a token-lifetime race. One identity, one key per -domain is stricter than WebAuthn's many-credentials-per-account model because -the Nostr key is itself the public identity; additional devices do not create -additional active bindings. Two contract identities plus explicit dependency -versions exist because folding a mutable key snapshot into policy identity would make benign -rotation change policy lineage, while omitting it would let evidence under a -removed key survive. Denial responses deliberately collapse the conditions that -RFC 6750 error codes distinguish. `trusted-proxy-hmac-v2` in NIP-FI-EDGE is a -fixed-component request MAC in the family of HTTP Message Signatures (RFC 9421) -and AWS SigV4, without negotiation and with length-prefixed canonicalization. ## Security considerations -Issuer compromise can impersonate a principal but cannot prove an uncompromised -bound Nostr key. Assertion theft cannot use an existing binding without that -key; private TOFU intentionally retains first-use theft risk. Snapshot -revalidation limits removed-key reuse but the base policy accepts authenticated -key-source rollback as residual issuer risk. Two-phase admission closes the -binding and policy TOCTOU window only when every authoritative witness is reread -atomically. Availability failures deny rather than degrade to Nostr-only access. +**Assertion theft.** A stolen assertion cannot authorize without also proving +the named `nostr_pubkey` via NIP-42. The relay's assertion–key binding check +is the primary control against assertion replay across keys. + +**TTL window after revocation.** Offline JWT verification means the relay +cannot observe IdP-side revocation until the current assertion expires. The +deployment MUST configure a `max_connection_lifetime_seconds` and +assertion TTL consistent with the organization's acceptable revocation latency. + +For upstream revocation without an explicit disconnect call (issuer stops +issuing assertions; no active session termination), access persists until the +live session's effective authority deadlines expire. After the session closes +naturally, a reconnect requires an assertion that remains valid when reverified. +Previously issued assertions that have not yet expired remain valid for +reconnection until `min(exp, iat + maximum_assertion_age)` (subject to possible +earlier termination from a snapshot refresh failure, hard-deadline expiry without +key replacement, or signing-key removal). Stopping issuance prevents minting +assertions that extend this window; it does not invalidate already-issued +assertions. If the issuer continues issuing assertions, access continues. + +For the session-only disconnect model (issuer issues a successful disconnect +call that closes all matching sessions synchronously), there is no surviving +old-session window. If the issuer also stops issuing new assertions at that +point, cumulative residual access is bounded by: + +``` +max(0, min(exp, iat + maximum_assertion_age) - now) +``` + +`max_connection_lifetime_seconds` only partitions that interval into individual +sessions; it does not shorten the total window. A snapshot refresh failure, +hard-deadline expiry without key replacement, or signing-key removal can +terminate access earlier, but these are not reliable protocol-level bounds: the +JWKS snapshot deadline renews on each refresh even when content is unchanged. +If the issuer continues issuing new assertions after the disconnect call, +cumulative access extends indefinitely — the session-only protocol places no +protocol-level bound on that case. See the non-normative note in the Admin +disconnect section for the open product question on the deny-until-TTL +alternative. + +**SSRF.** The JWKS fetcher implements SSRF protection: HTTPS-only URI +validation, DNS resolution with IP deny-list enforcement, address pinning to +prevent DNS rebinding TOCTOU, and redirect denial. The complete IANA +Special-Purpose address deny table is implemented; see `crates/buzz-core/src/network.rs`. + +**Issuer compromise.** A compromised assertion issuer can impersonate any +identity but cannot prove possession of the assertion-named Nostr key. The NIP-42 +proof remains an independent control. + +**Algorithm confusion.** The verifier enforces asymmetric algorithms only; +`alg=none` and symmetric algorithms deny. The exact `kid`-based key selection +is bounded before any attacker-controlled lookup. ## Sources - NIP-42 authentication: -- NIP-98 HTTP authentication: - JWT BCP: - JWT access-token profile: - DPoP: -- OAuth 2.0 mTLS client certificate-bound tokens: -- HTTP Message Signatures: -- Non-normative composed model: [NIP-FI-MODEL.md](NIP-FI-MODEL.md) From 560fea7b4fc5e96a4de45d988e039ffb3fd66662 Mon Sep 17 00:00:00 2001 From: Kalvin C Date: Tue, 1 Sep 2026 16:06:17 -0700 Subject: [PATCH 4/5] feat: add databricks fable 5.1 model capabilities (#7213) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary - add the canonical public Databricks `Claude Fable 5.1` model record - match the existing Fable family contract: adaptive thinking, `low|medium|high|xhigh|max`, default `high`, Anthropic Messages, and no normalization - add generated Rust/TypeScript corpus coverage and canonical Global Defaults label/persistence coverage - remove deployment-specific catalog references from the repository ## Scope This is metadata and regression coverage only. It does not change model-capability resolution or production frontend logic. ## Validation - `cargo fmt --check` - `node --test desktop/src/features/agents/ui/modelCapabilitiesCorpus.test.mjs` - `cargo test -p buzz-agent` - `pnpm build:e2e` - `pnpm exec playwright test --project=smoke --grep 'defaults render the Fable 5.1 label without changing the persisted id'` — 1 passed - repository search for the removed catalog prefix — no matches Signed-off-by: Kalvin Chau Co-authored-by: am <6e30cd56c30e030cd31bb0939b94a7c257c9a09d5ba2d92cf2735da45629f248@buzz.block.builderlab.xyz> --- crates/buzz-agent/src/model_capabilities.rs | 23 ++++------ .../ui/modelCapabilitiesCorpus.test.mjs | 22 ++++----- .../global-agent-config-screenshots.spec.ts | 46 ++++++++++++++++++- scripts/model-capabilities.json | 18 ++++++++ scripts/normative-corpus.json | 26 +++++++++-- 5 files changed, 103 insertions(+), 32 deletions(-) diff --git a/crates/buzz-agent/src/model_capabilities.rs b/crates/buzz-agent/src/model_capabilities.rs index b0e4ebc6e5..53f2d290ff 100644 --- a/crates/buzz-agent/src/model_capabilities.rs +++ b/crates/buzz-agent/src/model_capabilities.rs @@ -634,6 +634,7 @@ mod tests { Q::Vector { id: "resolver-exact-raw-id-probe", provider: "databricks_v2", raw_model_id: "databricks-gpt-5-4-mini", note: Some("Probes a raw id that has an exact record.") }, Q::Vector { id: "dbv2-claude-fable-5-exact-record-probe", provider: "databricks_v2", raw_model_id: "databricks-claude-fable-5", note: Some("Probes the canonical Databricks Fable 5 endpoint record.") }, Q::Vector { id: "dbv2-goose-claude-fable-5-alias-probe", provider: "databricks_v2", raw_model_id: "goose-claude-fable-5", note: Some("Probes a prefixed alias of the Databricks Fable 5 endpoint.") }, + Q::Vector { id: "dbv2-claude-fable-5-1-exact-record-probe", provider: "databricks_v2", raw_model_id: "databricks-claude-fable-5-1", note: Some("Probes the canonical Databricks Fable 5.1 endpoint record.") }, Q::Vector { id: "dbv2-claude-opus-4-8-exact-record-probe", provider: "databricks_v2", raw_model_id: "databricks-claude-opus-4-8", note: Some("Probes the canonical Databricks Opus 4.8 endpoint record.") }, Q::Vector { id: "dbv2-goose-claude-opus-4-8-alias-probe", provider: "databricks_v2", raw_model_id: "goose-claude-opus-4-8", note: Some("Probes a prefixed alias of the Databricks Opus 4.8 endpoint.") }, Q::Vector { id: "dbv2-claude-opus-5-exact-record-probe", provider: "databricks_v2", raw_model_id: "databricks-claude-opus-5", note: Some("Probes the canonical Databricks Opus 5 endpoint record.") }, @@ -743,7 +744,7 @@ mod tests { Q::Vector { id: "dbv2-inkling-exact-record-probe", provider: "databricks_v2", raw_model_id: "databricks-inkling", note: Some("Probes the Inkling endpoint record and label.") }, Q::Vector { id: "dbv2-uc-fqn-gemini-3-5-flash-strip-probe", provider: "databricks_v2", raw_model_id: "system.ai.gemini-3-5-flash", note: Some("Probes strip parity on a system.ai. UC FQN carrying the gemini- token (resolve carries no label; the alias label path is unit-tested).") }, Q::Vector { id: "dbv2-uc-fqn-meta-llama-strip-probe", provider: "databricks_v2", raw_model_id: "system.ai.meta-llama-3-3-70b-instruct", note: Some("Probes strip parity on a UC FQN where the llama- token strips through meta-.") }, - Q::Vector { id: "dbv2-uc-goose-deepseek-strip-probe", provider: "databricks_v2", raw_model_id: "data_workflow_tools.goose.goose-deepseek-v4-pro-0813", note: Some("Probes strip parity on a goose- prefixed UC FQN carrying the deepseek- token.") }, + Q::Vector { id: "dbv2-uc-fqn-deepseek-strip-probe", provider: "databricks_v2", raw_model_id: "system.ai.deepseek-v4-pro-0813", note: Some("Probes strip parity on a UC FQN carrying the deepseek- token.") }, Q::Vector { id: "dbv2-uc-fqn-inkling-strip-probe", provider: "databricks_v2", raw_model_id: "system.ai.inkling", note: Some("Probes strip parity on a UC FQN carrying the bare inkling token.") }, Q::Section { group: "Label/capability token isolation probes (#6955 review pass 1)", note: Some("Pins that label_family_tokens (the UC-humanization superset) never leaks into capability resolve(): capability stripping still uses only claude-/gpt-/kimi-, so a label token appearing before a gpt- marker must NOT displace the gpt-5-pro exact profile.") }, Q::Vector { id: "isolation-openai-gemini-gpt-5-pro-probe", provider: "openai", raw_model_id: "tenant-gemini-gpt-5-pro", note: Some("The gemini- label token must not strip here; capability resolve keeps the gpt-5-pro high-only profile.") }, @@ -843,7 +844,7 @@ mod tests { } #[test] - fn corpus_has_exactly_139_executable_vectors() { + fn corpus_has_exactly_140_executable_vectors() { // Locks the vector count so a silent INPUTS edit can't quietly drop // coverage; must equal the gate in the TS harness // (modelCapabilitiesCorpus.test.mjs). @@ -852,7 +853,7 @@ mod tests { .filter(|q| matches!(q, Q::Vector { .. })) .count(); assert_eq!( - vectors, 139, + vectors, 140, "corpus executable-vector count changed; update this gate deliberately" ); } @@ -873,7 +874,7 @@ mod tests { #[test] fn databricks_v2_fqn_uses_neutral_concrete_unknown_capabilities() { - let fqn = resolve("databricks_v2", "data_workflow_tools.goose.goose-kimi-k3"); + let fqn = resolve("databricks_v2", "system.ai.kimi-k3"); let fallback = resolve("databricks_v2", "some-unknown-xyz"); assert_eq!(fqn.thinking_mode, fallback.thinking_mode); assert_eq!(fqn.supported_efforts, fallback.supported_efforts); @@ -1056,16 +1057,10 @@ mod tests { ("system.ai.qwen35-122b-a10b", "Qwen3.5 122B A10B"), ("system.ai.gemma-3-12b", "Gemma 3 12B"), ("system.ai.inkling", "Inkling"), - ( - "data_workflow_tools.goose.goose-deepseek-v4-flash-0731", - "DeepSeek V4 Flash", - ), - ("data_workflow_tools.goose.goose-glm-5-3", "GLM-5.3"), - ( - "data_workflow_tools.goose.goose-glm-5-3-flash", - "GLM-5.3 Flash", - ), - ("data_workflow_tools.goose.goose-grok-4-6", "Grok 4.6"), + ("system.ai.deepseek-v4-flash-0731", "DeepSeek V4 Flash"), + ("system.ai.glm-5-3", "GLM-5.3"), + ("system.ai.glm-5-3-flash", "GLM-5.3 Flash"), + ("system.ai.grok-4-6", "Grok 4.6"), ] { assert_eq!(databricks_registry_label(fqn), Some(label), "fqn={fqn}"); } diff --git a/desktop/src/features/agents/ui/modelCapabilitiesCorpus.test.mjs b/desktop/src/features/agents/ui/modelCapabilitiesCorpus.test.mjs index 492c1cd6ac..e0d150cc4f 100644 --- a/desktop/src/features/agents/ui/modelCapabilitiesCorpus.test.mjs +++ b/desktop/src/features/agents/ui/modelCapabilitiesCorpus.test.mjs @@ -25,10 +25,10 @@ const corpus = JSON.parse(readFileSync(fileURLToPath(corpusUrl), "utf8")); // (`_group`) are skipped. Mirrors the Rust corpus filter. const executable = corpus.filter((entry) => entry.expect != null); -test("corpus has exactly 139 executable vectors", () => { +test("corpus has exactly 140 executable vectors", () => { // Locks the vector count so a silent corpus edit can't quietly drop coverage; // must equal the gate in the Rust suite (model_capabilities.rs). - assert.equal(executable.length, 139); + assert.equal(executable.length, 140); }); test("registry label aliases refuse an unprefixed query", () => { @@ -45,7 +45,7 @@ test("registry label aliases refuse an unprefixed query", () => { ); }); -test("UC model-family FQNs and goose- aliases humanize onto their base records", () => { +test("UC model-family FQNs humanize onto their base records", () => { // #6918 follow-up: the shared UC-FQN (`system.ai.…`) and goose- alias forms // must resolve onto the same base databricks_v2 records via the new family // tokens. Mirrors the Rust `test_databricks_registry_label_lookup` coverage. @@ -64,13 +64,10 @@ test("UC model-family FQNs and goose- aliases humanize onto their base records", ["system.ai.qwen35-122b-a10b", "Qwen3.5 122B A10B"], ["system.ai.gemma-3-12b", "Gemma 3 12B"], ["system.ai.inkling", "Inkling"], - [ - "data_workflow_tools.goose.goose-deepseek-v4-flash-0731", - "DeepSeek V4 Flash", - ], - ["data_workflow_tools.goose.goose-glm-5-3", "GLM-5.3"], - ["data_workflow_tools.goose.goose-glm-5-3-flash", "GLM-5.3 Flash"], - ["data_workflow_tools.goose.goose-grok-4-6", "Grok 4.6"], + ["system.ai.deepseek-v4-flash-0731", "DeepSeek V4 Flash"], + ["system.ai.glm-5-3", "GLM-5.3"], + ["system.ai.glm-5-3-flash", "GLM-5.3 Flash"], + ["system.ai.grok-4-6", "Grok 4.6"], ]; for (const [fqn, label] of cases) { assert.equal(databricksRegistryLabel(fqn), label, `fqn=${fqn}`); @@ -97,10 +94,7 @@ test("registry label aliases refuse ambiguous stripped record keys", () => { }); test("Unity Catalog FQNs use neutral concrete-unknown capabilities", () => { - const fqn = resolveModelCapabilities( - "databricks_v2", - "data_workflow_tools.goose.goose-kimi-k3", - ); + const fqn = resolveModelCapabilities("databricks_v2", "system.ai.kimi-k3"); const fallback = resolveModelCapabilities( "databricks_v2", "some-unknown-xyz", diff --git a/desktop/tests/e2e/global-agent-config-screenshots.spec.ts b/desktop/tests/e2e/global-agent-config-screenshots.spec.ts index 431a415771..a6ecf0ec56 100644 --- a/desktop/tests/e2e/global-agent-config-screenshots.spec.ts +++ b/desktop/tests/e2e/global-agent-config-screenshots.spec.ts @@ -251,7 +251,7 @@ test.describe("global agent config screenshots", () => { test("defaults render Databricks model labels without changing persisted ids", async ({ page, }) => { - const modelId = "data_workflow_tools.goose.goose-glm-5-3"; + const modelId = "system.ai.glm-5-3"; await installMockBridge(page, { globalAgentConfig: { preferred_runtime: "goose", @@ -291,6 +291,50 @@ test.describe("global agent config screenshots", () => { expect(persisted).toMatchObject({ model: modelId }); }); + test("defaults render the Fable 5.1 label without changing the persisted id", async ({ + page, + }) => { + const modelId = "databricks-claude-fable-5-1"; + await installMockBridge(page, { + globalAgentConfig: { + preferred_runtime: "goose", + provider: "databricks_v2", + model: modelId, + env_vars: {}, + }, + discoverAgentModels: { + models: [{ id: modelId, name: modelId }], + supportsSwitching: true, + selectedModel: modelId, + }, + runtimeFileConfigs: { + goose: { + provider: "databricks_v2", + model: modelId, + satisfiedEnvKeys: ["DATABRICKS_HOST"], + }, + }, + }); + + await openAiDefaultsSettings(page); + + const model = page.getByTestId("global-agent-model"); + await expect(model).toHaveText("Claude Fable 5.1"); + await expect(model).toHaveAttribute("data-value", modelId); + + const persisted = await page.evaluate(async () => + ( + window as typeof window & { + __BUZZ_E2E_INVOKE_MOCK_COMMAND__?: ( + command: string, + payload: unknown, + ) => Promise; + } + ).__BUZZ_E2E_INVOKE_MOCK_COMMAND__?.("get_global_agent_config", null), + ); + expect(persisted).toMatchObject({ model: modelId }); + }); + test("defaults honor credentials set in the harness config file", async ({ page, }) => { diff --git a/scripts/model-capabilities.json b/scripts/model-capabilities.json index eb3a230a4d..edcf25a783 100644 --- a/scripts/model-capabilities.json +++ b/scripts/model-capabilities.json @@ -487,6 +487,24 @@ "_provenance": "all axes materialized from family:anthropic-adaptive-xhigh-fable-5", "_source": "models.dev anthropic catalog label; Databricks workspace endpoint is absent from its provider catalog" }, + { + "provider": "databricks_v2", + "raw_model_id": "databricks-claude-fable-5-1", + "registry_label": "Claude Fable 5.1", + "thinking_mode": "adaptive", + "supported_efforts": [ + "low", + "medium", + "high", + "xhigh", + "max" + ], + "default_effort": "high", + "databricks_v2_wire_route": "anthropic-messages", + "normalization_policy": "none", + "_provenance": "all axes materialized from family:anthropic-adaptive-xhigh-fable-5", + "_source": "models.dev anthropic catalog label; Databricks workspace endpoint is absent from its provider catalog" + }, { "provider": "databricks_v2", "raw_model_id": "databricks-claude-opus-4-8", diff --git a/scripts/normative-corpus.json b/scripts/normative-corpus.json index 23aedb1e64..b75e4eb0d5 100644 --- a/scripts/normative-corpus.json +++ b/scripts/normative-corpus.json @@ -759,6 +759,26 @@ "registry_label": null } }, + { + "id": "dbv2-claude-fable-5-1-exact-record-probe", + "provider": "databricks_v2", + "raw_model_id": "databricks-claude-fable-5-1", + "_note": "Probes the canonical Databricks Fable 5.1 endpoint record.", + "expect": { + "thinking_mode": "adaptive", + "supported_efforts": [ + "low", + "medium", + "high", + "xhigh", + "max" + ], + "default_effort": "high", + "databricks_v2_wire_route": "anthropic-messages", + "normalization_policy": "none", + "registry_label": "Claude Fable 5.1" + } + }, { "id": "dbv2-claude-opus-4-8-exact-record-probe", "provider": "databricks_v2", @@ -2708,10 +2728,10 @@ } }, { - "id": "dbv2-uc-goose-deepseek-strip-probe", + "id": "dbv2-uc-fqn-deepseek-strip-probe", "provider": "databricks_v2", - "raw_model_id": "data_workflow_tools.goose.goose-deepseek-v4-pro-0813", - "_note": "Probes strip parity on a goose- prefixed UC FQN carrying the deepseek- token.", + "raw_model_id": "system.ai.deepseek-v4-pro-0813", + "_note": "Probes strip parity on a UC FQN carrying the deepseek- token.", "expect": { "thinking_mode": "none", "supported_efforts": [ From 1c8321cd08feb597f8bcff5195c21148fb3e98ed Mon Sep 17 00:00:00 2001 From: Taylor Ho Date: Tue, 1 Sep 2026 16:07:35 -0700 Subject: [PATCH 5/5] fix(desktop): retain automatic mentions only in threads (#7144) **Category:** fix **User Impact:** Top-level channel messages now notify only agents explicitly selected for that message, while thread replies visibly retain their addressed agents. **Problem:** Channel-root and thread composers presented the same automatic-mention model even though retained recipients are only predictable within an ongoing thread. That could make a new top-level message notify an agent the sender did not deliberately choose for that message. **Solution:** Make retained audiences a thread-only capability. Root messages remain explicit and one-shot; threads retain visible, removable agent recipients, with the automatic-mention setting exposed directly in the mention picker.
File changes **desktop/src/features/channels/ui/ChannelPane.tsx** Removes persistent audience state from the channel-root composer. **desktop/src/features/messages/ui/ComposerAddressControls.tsx** Uses the broader **Manage mentions** label because the picker includes people as well as automatic agent controls. **desktop/src/features/messages/ui/ComposerAddressControls.test.mjs** Locks the updated accessible label and active treatment. **desktop/src/features/messages/ui/MentionAutocomplete.tsx** Shows the right-aligned automatic-mention setting directly, uses thread-specific copy, preserves keyboard/focus behavior, and keeps the current mention when retention is unchecked. **desktop/src/features/messages/ui/MentionAutocomplete.test.mjs** Covers the always-visible setting, compact layout, copy, and thread-scoped agent actions. **desktop/src/features/messages/ui/MessageComposer.tsx** Separates unpinning an agent for future replies from removing its current draft mention. **desktop/src/features/messages/ui/MessageComposer.types.ts** Narrows retained audience contexts to threads. **desktop/src/features/messages/ui/persistentAgentAudienceHosts.test.mjs** Prevents channel-root and new-message hosts from opting back into retained audiences. **desktop/src/features/messages/ui/useAgentAddressLockPicker.ts** Splits unpin and current-mention removal semantics. **desktop/src/features/messages/ui/useAgentAddressLockPicker.test.mjs** Verifies unpinning retains the current draft mention. **desktop/src/features/settings/ui/AgentsSettingsPanel.tsx** Describes the preference as addressing selected agents in thread replies. **desktop/tests/e2e/persistent-agent-audience.spec.ts** Moves retained-audience lifecycle coverage to thread composers and adds root, settings, layout, focus, keyboard, unpin, and draft regressions. **desktop/src/features/messages/ui/MessageComposerAutocompletes.tsx** Preserves composer focus ownership while routing the thread-only controls. **desktop/src/features/messages/ui/useComposerFocusOwnership.ts** Keeps focus within the composer while interacting with its mention overlay controls.
### Reproduction steps 1. Enable **Automatically mention agents** under agent settings. 2. In a channel root, select an agent and send a message. Confirm the agent is addressed once, no retained-recipient control appears, and the next root message has no agent recipient. 3. Open a thread and select an agent. Confirm the visible recipient persists into later replies. 4. Open **Manage mentions** in the thread composer. Confirm the automatic-mention setting is immediately visible, right-aligned, and labeled **Address selected agents in thread replies**. 5. Uncheck a selected agent. Confirm its current draft mention remains, while later replies no longer retain it automatically. --------- Signed-off-by: Taylor Ho Co-authored-by: Rizz <302abe414ca6e3134763d2539bfcf145aea2a63fe5f8455204ed602fd40cf381@buzz.block.builderlab.xyz> Co-authored-by: Carl --- .../src/features/channels/ui/ChannelPane.tsx | 1 - .../src/features/home/ui/InboxDetailPane.tsx | 13 +- .../lib/persistentAgentAudience.test.mjs | 15 + .../messages/lib/persistentAgentAudience.ts | 17 + .../ui/ComposerAddressControls.test.mjs | 22 +- .../messages/ui/ComposerAddressControls.tsx | 14 +- .../messages/ui/MentionAutocomplete.test.mjs | 303 ++-------- .../messages/ui/MentionAutocomplete.tsx | 157 ++--- .../features/messages/ui/MessageComposer.tsx | 34 +- .../messages/ui/MessageComposer.types.ts | 3 +- .../messages/ui/MessageThreadPanel.tsx | 5 +- .../ui/persistentAgentAudienceHosts.test.mjs | 9 +- .../ui/useAgentAddressLockPicker.test.mjs | 23 +- .../messages/ui/useAgentAddressLockPicker.ts | 88 +-- .../messages/ui/useThreadAgentAudience.ts | 36 ++ .../settings/ui/AgentsSettingsPanel.tsx | 2 +- desktop/tests/e2e/mentions.spec.ts | 28 +- .../e2e/persistent-agent-audience.spec.ts | 545 ++++++++---------- .../tests/e2e/send-channel-binding.spec.ts | 4 +- 19 files changed, 511 insertions(+), 808 deletions(-) create mode 100644 desktop/src/features/messages/ui/useThreadAgentAudience.ts diff --git a/desktop/src/features/channels/ui/ChannelPane.tsx b/desktop/src/features/channels/ui/ChannelPane.tsx index 8b56ec8af9..ea3c264786 100644 --- a/desktop/src/features/channels/ui/ChannelPane.tsx +++ b/desktop/src/features/channels/ui/ChannelPane.tsx @@ -754,7 +754,6 @@ export const ChannelPane = React.memo(function ChannelPane({ ) : null} message.id === item.conversationId) + ?.tags ?? []) + : []; const contextLabel = isThreadContext ? isDirectMessage ? `Thread with ${item.senderLabel}` @@ -804,7 +808,14 @@ function InboxMessageDetailPane({ />
{ assert.deepEqual(currentAudiences(store), { [scope]: [agentA] }); }); +test("initialization preserves exclusions across thread remounts", async () => { + const store = await loadStore(14); + const scope = `${ownerA}:channel-a:thread-a`; + + store.initializePersistentAgentAudience(scope, [agentA]); + assert.deepEqual(currentAudiences(store), { [scope]: [agentA] }); + + store.excludePersistentAgentAudienceMember(scope, agentA); + store.initializePersistentAgentAudience(scope, [agentA]); + assert.deepEqual(currentAudiences(store), { [scope]: [] }); + + store.addPersistentAgentAudienceMember(scope, agentA); + assert.deepEqual(currentAudiences(store), { [scope]: [agentA] }); +}); + test("explicit re-selection reinstates an excluded agent", async () => { const store = await loadStore(13); const scope = `${ownerA}:channel-a:channel`; diff --git a/desktop/src/features/messages/lib/persistentAgentAudience.ts b/desktop/src/features/messages/lib/persistentAgentAudience.ts index 78819e0c73..bda3ecabd4 100644 --- a/desktop/src/features/messages/lib/persistentAgentAudience.ts +++ b/desktop/src/features/messages/lib/persistentAgentAudience.ts @@ -160,6 +160,23 @@ export function removePersistentAgentAudienceMembersIfUnchanged({ return true; } +export function initializePersistentAgentAudience( + scope: string, + pubkeys: Iterable, +): void { + if (!scope) return; + const excluded = excludedPubkeysByScope.get(scope); + const initialPubkeys = normalizePubkeys(pubkeys).filter( + (pubkey) => + !(audiences[scope] ?? []).includes(pubkey) && !excluded?.has(pubkey), + ); + if (initialPubkeys.length === 0) return; + setPersistentAgentAudience(scope, [ + ...(audiences[scope] ?? []), + ...initialPubkeys, + ]); +} + export function addPersistentAgentAudienceMember( scope: string, pubkey: string, diff --git a/desktop/src/features/messages/ui/ComposerAddressControls.test.mjs b/desktop/src/features/messages/ui/ComposerAddressControls.test.mjs index 5dfc851bfd..91c69a962e 100644 --- a/desktop/src/features/messages/ui/ComposerAddressControls.test.mjs +++ b/desktop/src/features/messages/ui/ComposerAddressControls.test.mjs @@ -72,7 +72,7 @@ test("mention control expands with automatically mentioned agents", async () => const avatar = view.getByTestId("composer-address-lock-agent-pubkey"); assert.ok(avatar); const manage = view.getByRole("button", { - name: "Manage automatic agent mentions", + name: "Manage mentions", }); assert.match(manage.className, /(?:^|\s)-ml-2(?:\s|$)/); assert.match(manage.className, /(?:^|\s)pl-2(?:\s|$)/); @@ -82,18 +82,18 @@ test("mention control expands with automatically mentioned agents", async () => /(?:^|\s)pr-1\.5(?:\s|$)/, ); assert.match( - view.getByRole("button", { name: "Manage automatic agent mentions" }) - .parentElement?.className ?? "", + view.getByRole("button", { name: "Manage mentions" }).parentElement + ?.className ?? "", /(?:^|\s)bg-primary\/15(?:\s|$)/, ); assert.match( - view.getByRole("button", { name: "Manage automatic agent mentions" }) - .parentElement?.className ?? "", + view.getByRole("button", { name: "Manage mentions" }).parentElement + ?.className ?? "", /(?:^|\s)text-primary(?:\s|$)/, ); assert.doesNotMatch( - view.getByRole("button", { name: "Manage automatic agent mentions" }) - .parentElement?.className ?? "", + view.getByRole("button", { name: "Manage mentions" }).parentElement + ?.className ?? "", /(?:^|\s)bg-accent\/70(?:\s|$)/, ); assert.doesNotMatch( @@ -110,7 +110,13 @@ test("mention control expands with automatically mentioned agents", async () => /scale\(0.8\)/, ); } - const remove = view.getByTestId("composer-address-lock-remove-agent-pubkey"); + const remove = view.getByRole("button", { + name: "Don't automatically mention Agent Ada in this thread", + }); + assert.equal( + remove.getAttribute("aria-label")?.includes("conversation"), + false, + ); const removeChrome = remove.querySelector("span.absolute"); assert.match( removeChrome?.className ?? "", diff --git a/desktop/src/features/messages/ui/ComposerAddressControls.tsx b/desktop/src/features/messages/ui/ComposerAddressControls.tsx index 795d07c380..5ce8e117fb 100644 --- a/desktop/src/features/messages/ui/ComposerAddressControls.tsx +++ b/desktop/src/features/messages/ui/ComposerAddressControls.tsx @@ -180,11 +180,7 @@ export function ComposerMentionButton({ + + Automatically mention agents + + + Address selected agents in thread replies + + + event.preventDefault()} + /> +
) : null} - {/* biome-ignore lint/a11y/noStaticElementInteractions: pointer-only guard, same as the options surface — here it covers presses on the scrollbar and the list's padding ring. */} + {/* biome-ignore lint/a11y/noStaticElementInteractions: pointer-only guard keeps padding and scrollbar presses from blurring the owning editor. */}
{isAlwaysAddressed - ? "Don't automatically mention in this conversation" + ? "Don't automatically mention in this thread" : "Automatically mention"} {alwaysAddressShortcut ? ( diff --git a/desktop/src/features/messages/ui/MessageComposer.tsx b/desktop/src/features/messages/ui/MessageComposer.tsx index ecdfb01431..888370e7a3 100644 --- a/desktop/src/features/messages/ui/MessageComposer.tsx +++ b/desktop/src/features/messages/ui/MessageComposer.tsx @@ -28,14 +28,8 @@ import { import { useComposerFocusOwnership } from "@/features/messages/lib/useComposerFocusOwnership"; import { isMentionCodeContext } from "@/features/messages/lib/mentionCodeContext"; import { useMentions } from "@/features/messages/lib/useMentions"; -import { - getPersistentAgentAudienceScope, - usePersistentAgentAudience, -} from "@/features/messages/lib/persistentAgentAudience"; -import { - setKeepMentionedAgentsPinned, - useKeepMentionedAgentsPinned, -} from "@/features/messages/lib/autoPinMentionedAgentsPreference"; +import { getPersistentAgentAudienceScope } from "@/features/messages/lib/persistentAgentAudience"; +import { setKeepMentionedAgentsPinned } from "@/features/messages/lib/autoPinMentionedAgentsPreference"; import { useIdentityQuery } from "@/shared/api/hooks"; import { CUSTOM_EMOJI_NODE_NAME } from "@/features/messages/lib/customEmojiNode"; import { @@ -66,6 +60,7 @@ import { useComposerContentState } from "./useComposerContentState"; import { useComposerPasteHandler } from "./useComposerPasteHandler"; import { useDraftPersistLifecycle } from "./useDraftPersistSnapshot"; import { useImplicitAgentMentionProvenance } from "./useImplicitAgentMentionProvenance"; +import { useThreadAgentAudience } from "./useThreadAgentAudience"; import { submitMessageEdit } from "./submitMessageEdit"; import { prepareBackgroundLinkPreviews } from "@/features/messages/lib/linkPreviewPreparationStore"; import { useComposerLinkPreviews } from "./useComposerLinkPreviews"; @@ -326,8 +321,12 @@ function MessageComposerImpl({ onLinkSelectionChangeRef.current = linkEditor.showFromCursor; onLinkShortcutRef.current = linkEditor.openFromShortcut; useComposerSpoilerParticles(richText.editor, composerScrollRef); - const persistentAudience = usePersistentAgentAudience(audienceScope); - const keepMentionedAgentsPinned = useKeepMentionedAgentsPinned(); + const { audience: persistentAudience, keepMentionedAgentsPinned } = + useThreadAgentAudience({ + isAgentPubkey: mentions.isAgentPubkey, + rootTags: audienceContext?.rootTags ?? [], + scope: audienceScope, + }); const addressPulse = useAddressMentionPulse(); const { completeOptionsReveal: completeMentionOptionsReveal, @@ -425,16 +424,11 @@ function MessageComposerImpl({ setSpoileredAttachmentUrls(restoredSpoileredAttachmentUrls); } }, [editTarget?.id]); - // ── Focus on reply ────────────────────────────────────────────────── - // Use focusPreserve so that re-renders (e.g. new messages arriving in - // a thread) don't yank the cursor to the end while the user is editing. React.useEffect(() => { if (!replyTarget || composerDisabled) return; richText.focusPreserve(); }, [composerDisabled, replyTarget, richText.focusPreserve]); useComposerAutofocus(richText.focus, effectiveDraftKey, composerDisabled); - // Hooks return a plain-text edit descriptor; `replacePlainTextRange` - // applies it as a single ProseMirror transaction (no markdown round-trip). const applyAutocompleteEdit = React.useCallback( (edit: AutocompleteEdit) => { richText.replacePlainTextRange( @@ -511,10 +505,6 @@ function MessageComposerImpl({ const insertEmoji = React.useCallback( (emoji: string) => { if (!richText.editor) return; - // A `:shortcode:` for a known custom emoji becomes a selectable atom - // node (same as the input rule / autocomplete), so it can be selected, - // copied, and deleted as one unit. Everything else (native unicode) - // inserts as plain content. const match = /^:([^:\s]+):$/.exec(emoji); const shortcode = match?.[1]?.toLowerCase(); const known = @@ -891,7 +881,11 @@ function MessageComposerImpl({ onEmojiSelect={applyEmojiInsert} onMentionSelect={selectMentionSuggestion} onOptionsRevealComplete={completeMentionOptionsReveal} - onToggleAlwaysAddressAgent={toggleAlwaysAddressAgent} + onToggleAlwaysAddressAgent={(suggestion) => + toggleAlwaysAddressAgent(suggestion, { + preserveMention: true, + }) + } /> {media.uploadState.status === "error" ? (
diff --git a/desktop/src/features/messages/ui/MessageComposer.types.ts b/desktop/src/features/messages/ui/MessageComposer.types.ts index ac3e40d57d..5704988e30 100644 --- a/desktop/src/features/messages/ui/MessageComposer.types.ts +++ b/desktop/src/features/messages/ui/MessageComposer.types.ts @@ -25,7 +25,8 @@ export type MessageComposerEditTarget = { export type MessageComposerProps = { audienceContext?: { - type: "channel" | "thread"; + rootTags?: readonly string[][]; + type: "thread"; } | null; channelId?: string | null; channelName: string; diff --git a/desktop/src/features/messages/ui/MessageThreadPanel.tsx b/desktop/src/features/messages/ui/MessageThreadPanel.tsx index 16ee26bcc7..1de7b140fd 100644 --- a/desktop/src/features/messages/ui/MessageThreadPanel.tsx +++ b/desktop/src/features/messages/ui/MessageThreadPanel.tsx @@ -830,7 +830,10 @@ export function MessageThreadPanel({ > { +test("only thread conversation hosts opt into persistent audiences", async () => { const [channelPane, threadPanel, newMessage, inboxDetail] = await Promise.all( [ source("../../channels/ui/ChannelPane.tsx"), @@ -16,9 +16,12 @@ test("supported conversation hosts opt into explicit audience contexts", async ( ], ); - assert.match(channelPane, /audienceContext=\{\{ type: "channel" \}\}/); + assert.doesNotMatch(channelPane, /audienceContext=/); assert.doesNotMatch(newMessage, /audienceContext=/); - assert.match(threadPanel, /audienceContext=\{\{ type: "thread" \}\}/); + assert.match( + threadPanel, + /audienceContext=\{\{[\s\S]*type: "thread",[\s\S]*rootTags: threadHead\.tags,[\s\S]*\}\}/, + ); assert.match(inboxDetail, /type: "thread"/); assert.doesNotMatch(threadPanel, /audienceContext=\{[\s\S]*threadRootId/); assert.doesNotMatch(inboxDetail, /audienceContext=\{[\s\S]*threadRootId/); diff --git a/desktop/src/features/messages/ui/useAgentAddressLockPicker.test.mjs b/desktop/src/features/messages/ui/useAgentAddressLockPicker.test.mjs index 8ed155234f..60ead28616 100644 --- a/desktop/src/features/messages/ui/useAgentAddressLockPicker.test.mjs +++ b/desktop/src/features/messages/ui/useAgentAddressLockPicker.test.mjs @@ -140,7 +140,7 @@ test("always addressing a new agent delegates the first add for immediate confir assert.deepEqual(pulsedPubkeys, []); }); -test("toggling an addressed agent keeps autocomplete open and removes the lock", async () => { +test("unpinning an addressed agent keeps its current mention and autocomplete open", async () => { const { act, renderHook } = await import("@testing-library/react"); const { useAgentAddressLockPicker } = await import( "./useAgentAddressLockPicker.ts" @@ -187,20 +187,17 @@ test("toggling an addressed agent keeps autocomplete open and removes the lock", ); act(() => { - result.current.toggleAlwaysAddressAgent({ - pubkey: "agent-pubkey", - displayName: "Agent Ada", - isAgent: true, - }); + result.current.toggleAlwaysAddressAgent( + { + pubkey: "agent-pubkey", + displayName: "Agent Ada", + isAgent: true, + }, + { preserveMention: true }, + ); }); - assert.deepEqual(appliedEdits, [ - { - replaceFromOffset: 4, - replaceToOffset: 15, - insertText: "", - }, - ]); + assert.deepEqual(appliedEdits, []); assert.equal(cancelCount, 0); assert.deepEqual(removedPubkeys, ["agent-pubkey"]); assert.deepEqual(pulsedPubkeys, []); diff --git a/desktop/src/features/messages/ui/useAgentAddressLockPicker.ts b/desktop/src/features/messages/ui/useAgentAddressLockPicker.ts index 4a9ca5f15c..4dfe40b14b 100644 --- a/desktop/src/features/messages/ui/useAgentAddressLockPicker.ts +++ b/desktop/src/features/messages/ui/useAgentAddressLockPicker.ts @@ -14,45 +14,6 @@ import { normalizePubkey, truncatePubkey } from "@/shared/lib/pubkey"; import type { ComposerAddressAgent } from "./ComposerAddressControls"; import type { MentionSuggestion } from "./MentionAutocomplete"; -function buildMentionRemovalEdits( - text: string, - displayNames: readonly string[], - queryRange?: { start: number; end: number }, -): AutocompleteEdit[] { - const ranges = displayNames.flatMap((displayName) => - getMentionOffsets(text, displayName).map((start) => { - let end = start + `@${displayName}`.length; - if (text[end] === " ") end += 1; - return { start, end }; - }), - ); - if (queryRange) { - ranges.push({ - start: Math.max(0, Math.min(queryRange.start, text.length)), - end: Math.max(0, Math.min(queryRange.end, text.length)), - }); - } - - const merged = ranges - .filter(({ start, end }) => start < end) - .sort((left, right) => left.start - right.start) - .reduce>((result, range) => { - const previous = result.at(-1); - if (previous && range.start <= previous.end) { - previous.end = Math.max(previous.end, range.end); - } else { - result.push({ ...range }); - } - return result; - }, []); - - return merged.reverse().map(({ start, end }) => ({ - replaceFromOffset: start, - replaceToOffset: end, - insertText: "", - })); -} - export function useAgentAddressLockPicker({ applyAutocompleteEdit, audience, @@ -177,13 +138,21 @@ export function useAgentAddressLockPicker({ ], ); - const removeAddressedAgent = React.useCallback( + const unpinAddressedAgent = React.useCallback( (pubkey: string) => { const normalized = normalizePubkey(pubkey); if (!audienceScope || !normalized) return; unpinnedAgentPubkeysRef.current.add(normalized); const excludePubkey = audience.excludePubkey ?? audience.removePubkey; excludePubkey(normalized); + }, + [audience.excludePubkey, audience.removePubkey, audienceScope], + ); + const removeAddressedAgent = React.useCallback( + (pubkey: string) => { + const normalized = normalizePubkey(pubkey); + if (!audienceScope || !normalized) return; + unpinAddressedAgent(normalized); const displayName = lockedAgents.find( (agent) => agent.pubkey === normalized, )?.displayName; @@ -206,43 +175,27 @@ export function useAgentAddressLockPicker({ }, [ applyAutocompleteEdit, - audience.excludePubkey, - audience.removePubkey, audienceScope, lockedAgents, onImplicitPrefixRemoved, richText.getPlainTextAndCursor, - ], - ); - const removeAddressedAgentMentions = React.useCallback( - (pubkey: string) => { - const normalized = normalizePubkey(pubkey); - if (!audienceScope || !normalized) return; - const { text } = richText.getPlainTextAndCursor(); - const matchingDisplayNames = mentions - .getDraftMentionRefs(text) - .filter((ref) => normalizePubkey(ref.pubkey) === normalized) - .map((ref) => ref.displayName); - for (const edit of buildMentionRemovalEdits(text, matchingDisplayNames)) { - applyAutocompleteEdit(edit); - } - removeAddressedAgent(normalized); - }, - [ - applyAutocompleteEdit, - audienceScope, - mentions.getDraftMentionRefs, - removeAddressedAgent, - richText.getPlainTextAndCursor, + unpinAddressedAgent, ], ); const toggleAlwaysAddressAgent = React.useCallback( - (suggestion: MentionSuggestion) => { + ( + suggestion: MentionSuggestion, + options: { preserveMention?: boolean } = {}, + ) => { const pubkey = normalizePubkey(suggestion.pubkey ?? ""); if (!audienceScope || !pubkey || !suggestion.isAgent) return; if (lockedAgentPubkeys.has(pubkey)) { - removeAddressedAgentMentions(pubkey); + if (options.preserveMention) { + unpinAddressedAgent(pubkey); + } else { + removeAddressedAgent(pubkey); + } setAnnouncement( `Stopped automatically mentioning ${suggestion.displayName}`, ); @@ -313,9 +266,10 @@ export function useAgentAddressLockPicker({ onAddressAgentMention, onImplicitPrefixInserted, onPulseAddressLock, - removeAddressedAgentMentions, + removeAddressedAgent, richText.getPlainTextAndCursor, trackMentionAddressedAgent, + unpinAddressedAgent, ], ); diff --git a/desktop/src/features/messages/ui/useThreadAgentAudience.ts b/desktop/src/features/messages/ui/useThreadAgentAudience.ts new file mode 100644 index 0000000000..e10dad9cf0 --- /dev/null +++ b/desktop/src/features/messages/ui/useThreadAgentAudience.ts @@ -0,0 +1,36 @@ +import * as React from "react"; + +import { useKeepMentionedAgentsPinned } from "@/features/messages/lib/autoPinMentionedAgentsPreference"; +import { + initializePersistentAgentAudience, + usePersistentAgentAudience, +} from "@/features/messages/lib/persistentAgentAudience"; + +export function useThreadAgentAudience({ + isAgentPubkey, + rootTags, + scope, +}: { + isAgentPubkey: (pubkey: string) => boolean; + rootTags: readonly string[][]; + scope: string | null; +}) { + const audience = usePersistentAgentAudience(scope); + const keepMentionedAgentsPinned = useKeepMentionedAgentsPinned(); + + const rootAgentPubkeys = React.useMemo( + () => + rootTags.flatMap((tag) => { + const pubkey = tag[0] === "p" ? tag[1] : null; + return pubkey && isAgentPubkey(pubkey) ? [pubkey] : []; + }), + [isAgentPubkey, rootTags], + ); + + React.useEffect(() => { + if (!scope || !keepMentionedAgentsPinned) return; + initializePersistentAgentAudience(scope, rootAgentPubkeys); + }, [keepMentionedAgentsPinned, rootAgentPubkeys, scope]); + + return { audience, keepMentionedAgentsPinned }; +} diff --git a/desktop/src/features/settings/ui/AgentsSettingsPanel.tsx b/desktop/src/features/settings/ui/AgentsSettingsPanel.tsx index 50538868d0..b2fde755ff 100644 --- a/desktop/src/features/settings/ui/AgentsSettingsPanel.tsx +++ b/desktop/src/features/settings/ui/AgentsSettingsPanel.tsx @@ -37,7 +37,7 @@ export function AgentsSettingsPanel() { className="mt-0.5 text-sm text-muted-foreground/70" data-settings-subcopy > - After you mention them once + Address selected agents in thread replies

readOutgoingMentionPubkeys(page, "@carl local")) .toEqual([managedPubkey]); - await expect(input).toHaveText("@carl "); + await expect(input).toHaveText(""); - await page.getByTestId(`composer-address-lock-${managedPubkey}`).click(); await input.fill("@carl"); const reopenedDropdown = autocomplete(page); await expect(reopenedDropdown).toBeVisible(); const reopenedRelayRow = reopenedDropdown.getByTestId( `mention-suggestion-${relayPubkey}`, ); - await reopenedRelayRow - .getByRole("button", { name: "Automatically mention carl", exact: true }) - .click(); + await reopenedRelayRow.getByRole("button", { name: "Mention carl" }).click(); await expect( page.getByTestId(`composer-address-lock-${relayPubkey}`), - ).toBeVisible(); + ).toHaveCount(0); await expect( page.getByTestId(`composer-address-lock-${managedPubkey}`), ).toHaveCount(0); @@ -1441,7 +1436,7 @@ test("managed relay-profile agents with member roles can be addressed explicitly await expect(charlieRow.getByText("agent")).toBeVisible(); await charlieRow .getByRole("button", { - name: "Automatically mention charlie", + name: "Mention charlie", exact: true, }) .click(); @@ -1450,12 +1445,7 @@ test("managed relay-profile agents with member roles can be addressed explicitly await expect(input.locator(".agent-mention-highlight")).toHaveText("charlie"); await expect( page.getByTestId(`composer-address-lock-${TEST_IDENTITIES.charlie.pubkey}`), - ).toBeVisible(); - await expect( - page.getByRole("status").filter({ - hasText: "Automatically mentioning charlie", - }), - ).toBeVisible(); + ).toHaveCount(0); }); test("other-owned agents without a shared channel are hidden from mentions", async ({ @@ -2957,7 +2947,7 @@ test("a managed non-member agent from a DM can be addressed explicitly", async ( await expect(input.locator(".mention-chip")).toHaveCount(0); await charlieRow .getByRole("button", { - name: "Automatically mention charlie", + name: "Mention charlie", exact: true, }) .click(); @@ -2966,7 +2956,7 @@ test("a managed non-member agent from a DM can be addressed explicitly", async ( await expect(input.locator(".agent-mention-highlight")).toHaveText("charlie"); await expect( page.getByTestId(`composer-address-lock-${TEST_IDENTITIES.charlie.pubkey}`), - ).toBeVisible(); + ).toHaveCount(0); }); test("global non-member people can be selected from channel mentions", async ({ diff --git a/desktop/tests/e2e/persistent-agent-audience.spec.ts b/desktop/tests/e2e/persistent-agent-audience.spec.ts index 6cc2f22a88..d1174132c7 100644 --- a/desktop/tests/e2e/persistent-agent-audience.spec.ts +++ b/desktop/tests/e2e/persistent-agent-audience.spec.ts @@ -49,6 +49,24 @@ async function automaticallyMention( await composer.locator("[data-mention-picker-trigger]").click(); } +async function waitForMockLiveSubscription(page: Page, channelName: string) { + await expect + .poll(() => + page.evaluate( + (currentChannelName) => + window.__BUZZ_E2E_HAS_MOCK_LIVE_SUBSCRIPTION__?.({ + channelName: currentChannelName, + }) ?? false, + channelName, + ), + ) + .toBe(true); +} + +async function waitForTimelineSettled(page: Page) { + await expect(page.locator("[data-render-pending]")).toHaveCount(0); +} + async function openGeneral(page: Page) { await page.goto(`/#/channels/${CHANNEL_ID}`, { waitUntil: "domcontentloaded", @@ -92,6 +110,20 @@ async function pressPrimaryShiftM(page: Page) { await page.keyboard.press(`${isMac ? "Meta" : "Control"}+Shift+M`); } +async function readPersistedDraftContent(page: Page, draftKey: string) { + return page.evaluate((key) => { + for (const storageKey of Object.keys(window.localStorage)) { + if (!storageKey.startsWith("buzz-drafts.v2:")) continue; + const drafts = JSON.parse( + window.localStorage.getItem(storageKey) ?? "{}", + ) as Record; + const draft = drafts[key]; + if (draft) return draft.content ?? ""; + } + return ""; + }, draftKey); +} + async function readOutgoingMentionPubkeys(page: Page, content: string) { return page.evaluate((expectedContent) => { const signedEvent = window.__BUZZ_E2E_SIGNED_EVENTS__?.find( @@ -271,9 +303,9 @@ test("automatically mentions multiple agents from the mention picker", async ({ page, }) => { await installAudienceFixtures(page); - await openGeneral(page); + await openThread(page); - const composer = channelComposer(page); + const composer = threadComposer(page); await automaticallyMention(composer, "Morgarita"); await automaticallyMention(composer, "Vogue"); @@ -284,7 +316,7 @@ test("automatically mentions multiple agents from the mention picker", async ({ composer.getByTestId(`composer-address-lock-${AGENT_B}`), ).toBeVisible(); await expect( - composer.getByRole("button", { name: "Manage automatic agent mentions" }), + composer.getByRole("button", { name: "Manage mentions" }), ).toBeVisible(); }); @@ -292,21 +324,25 @@ test("keeps the composer and global automatic mention settings synchronized", as page, }) => { await installAudienceFixtures(page); - await openGeneral(page); + await openThread(page); - const composer = channelComposer(page); + const composer = threadComposer(page); await composer.getByTestId("message-insert-mention").click(); - const optionsTrigger = composer.getByTestId("mention-options-trigger"); - await expect(optionsTrigger).toHaveAttribute("aria-expanded", "false"); - await composer - .getByTestId("mention-autocomplete") - .getByRole("button", { name: "Automatically mention Morgarita" }) - .click(); - await expect(optionsTrigger).toHaveAttribute("aria-expanded", "true"); const composerToggle = composer.getByTestId( "mention-keep-agents-pinned-toggle", ); await expect(composerToggle).toHaveAttribute("data-state", "unchecked"); + const settings = composer.getByTestId("mention-options-settings"); + await expect(settings).toBeVisible(); + const settingsBox = await settings.boundingBox(); + const heading = settings.getByText("Automatically mention agents"); + const headingBox = await heading.boundingBox(); + expect(settingsBox?.width).toBeGreaterThanOrEqual(300); + expect(headingBox?.height).toBeLessThanOrEqual(20); + await composer + .getByTestId("mention-autocomplete") + .getByRole("button", { name: "Automatically mention Morgarita" }) + .click(); await expect(composerToggle).toHaveAttribute("data-state", "checked", { timeout: 1_500, }); @@ -316,8 +352,7 @@ test("keeps the composer and global automatic mention settings synchronized", as .getByRole("button", { name: "Turn off" }) .click(); await expect(composer.getByTestId("mention-autocomplete")).toBeVisible(); - await expect(optionsTrigger).toHaveAttribute("aria-expanded", "true"); - await expect(composerToggle).toHaveAttribute("data-state", "checked"); + await expect(composer.getByTestId("mention-options-settings")).toBeVisible(); await expect(composerToggle).toHaveAttribute("data-state", "unchecked", { timeout: 1_500, }); @@ -336,7 +371,6 @@ test("keeps the composer and global automatic mention settings synchronized", as composer.getByTestId(`composer-address-lock-${AGENT_A}`), ).toHaveCount(0); await composer.getByTestId("message-insert-mention").click(); - await composer.getByTestId("mention-options-trigger").click(); await expect( composer.getByTestId("mention-keep-agents-pinned-toggle"), ).toHaveAttribute("data-state", "unchecked"); @@ -347,7 +381,7 @@ test("keeps the composer and global automatic mention settings synchronized", as ).toHaveAttribute("aria-pressed", "false"); }); -test("hides automatic mention state while disabled without clearing the draft", async ({ +test("the disabled root composer preserves its explicit draft without audience controls", async ({ page, }) => { await installAudienceFixtures(page); @@ -355,11 +389,8 @@ test("hides automatic mention state while disabled without clearing the draft", const composer = channelComposer(page); const input = composer.getByTestId("message-input"); - await automaticallyMention(composer, "Morgarita"); - await input.type("draft text"); - await expect( - composer.getByTestId(`composer-address-lock-${AGENT_A}`), - ).toBeVisible(); + await input.fill("explicit draft text"); + await expect(composer.getByTestId("composer-address-locks")).toHaveCount(0); await page.getByTestId("channel-management-trigger").click(); await expect(page.getByTestId("channel-management-sheet")).toBeVisible(); @@ -368,22 +399,8 @@ test("hides automatic mention state while disabled without clearing the draft", await page.getByTestId("auxiliary-panel-close").click(); await expect(input).toHaveAttribute("contenteditable", "false"); - await expect(input).toHaveText("@Morgarita draft text"); - await expect( - composer.getByTestId(`composer-address-lock-${AGENT_A}`), - ).toHaveCount(0); - - await page.getByTestId("channel-management-trigger").click(); - await expect(page.getByTestId("channel-management-sheet")).toBeVisible(); - await page.getByTestId("channel-management-unarchive").click(); - await expect(page.getByTestId("channel-management-archive")).toBeVisible(); - await page.getByTestId("auxiliary-panel-close").click(); - - await expect(input).toHaveAttribute("contenteditable", "true"); - await expect(input).toHaveText("@Morgarita draft text"); - await expect( - composer.getByTestId(`composer-address-lock-${AGENT_A}`), - ).toBeVisible(); + await expect(input).toHaveText("explicit draft text"); + await expect(composer.getByTestId("composer-address-locks")).toHaveCount(0); }); test("Tab inserts a one-time agent mention by default", async ({ page }) => { @@ -418,12 +435,11 @@ test("disabling automatic mentions leaves the composer empty after send", async }) => { await keepMentionedAgentsPinned(page); await installAudienceFixtures(page); - await openGeneral(page); + await openThread(page); - const composer = channelComposer(page); + const composer = threadComposer(page); const input = composer.getByTestId("message-input"); await composer.getByTestId("message-insert-mention").click(); - await composer.getByTestId("mention-options-trigger").click(); const preference = composer.getByTestId("mention-keep-agents-pinned-toggle"); await expect(preference).toHaveAttribute("data-state", "checked"); await preference.click(); @@ -452,9 +468,9 @@ test("primary+Shift+M addresses the default agent, then toggles the highlighted }) => { await keepMentionedAgentsPinned(page); await installAudienceFixtures(page); - await openGeneral(page); + await openThread(page); - const composer = channelComposer(page); + const composer = threadComposer(page); const input = composer.getByTestId("message-input"); await input.fill("draft text"); await pressPrimaryShiftM(page); @@ -500,10 +516,10 @@ test("primary+Shift+M favors the most recently mentioned eligible agent", async page, }) => { await installAudienceFixtures(page); - await openGeneral(page); + await openThread(page); await emitMockMessage(page, "Please ask Vogue", [AGENT_B]); - const input = channelComposer(page).getByTestId("message-input"); + const input = threadComposer(page).getByTestId("message-input"); await input.fill("draft text"); await input.press("ArrowLeft"); await input.press("ArrowLeft"); @@ -522,13 +538,13 @@ test("the mention button opens settings and can undo an address", async ({ page, }) => { await installAudienceFixtures(page); - await openGeneral(page); + await openThread(page); - const composer = channelComposer(page); + const composer = threadComposer(page); await automaticallyMention(composer, "Morgarita"); const input = composer.getByTestId("message-input"); const ingress = composer.getByRole("button", { - name: "Manage automatic agent mentions", + name: "Manage mentions", }); await input.type("draft text"); @@ -536,31 +552,34 @@ test("the mention button opens settings and can undo an address", async ({ const menu = composer.getByTestId("mention-autocomplete"); await expect(menu).toBeVisible(); await expect(input).toHaveText("@Morgarita draft text"); - await page.getByTestId("mention-options-trigger").click(); await expect( page.getByTestId("mention-keep-agents-pinned-toggle"), ).toBeVisible(); const layerBox = await composer .getByTestId("mention-autocomplete-layer") .boundingBox(); - const optionsBox = await page - .getByTestId("mention-options-trigger") + const settingsBox = await page + .getByTestId("mention-options-settings") .boundingBox(); + const menuBox = await menu.boundingBox(); expect(layerBox).not.toBeNull(); - expect(optionsBox).not.toBeNull(); - if (!layerBox || !optionsBox) throw new Error("Mention tray is not laid out"); - await page.mouse.click(layerBox.x + 4, optionsBox.y + optionsBox.height / 2); + expect(settingsBox).not.toBeNull(); + expect(menuBox).not.toBeNull(); + if (!layerBox || !settingsBox || !menuBox) { + throw new Error("Mention tray is not laid out"); + } + await page.mouse.click( + layerBox.x + layerBox.width / 2, + (settingsBox.y + settingsBox.height + menuBox.y) / 2, + ); await expect(menu).toHaveCount(0); await expect(input).toHaveText("@Morgarita draft text"); await ingress.click(); await expect(menu).toBeVisible(); - await expect(page.getByTestId("mention-options-trigger")).toHaveAttribute( - "aria-expanded", - "false", - ); + await expect(page.getByTestId("mention-options-settings")).toBeVisible(); await expect( page.getByTestId("mention-keep-agents-pinned-toggle"), - ).toHaveCount(0); + ).toBeVisible(); await ingress.click(); await expect(menu).toHaveCount(0); await expect(input).toHaveText("@Morgarita draft text"); @@ -569,16 +588,16 @@ test("the mention button opens settings and can undo an address", async ({ await expect(page.getByTestId("user-profile-panel")).toHaveCount(0); await expect( menu.getByRole("button", { - name: "Don't automatically mention Morgarita in this conversation", + name: "Don't automatically mention Morgarita in this thread", }), ).toHaveAttribute("aria-pressed", "true"); await menu .getByRole("button", { - name: "Don't automatically mention Morgarita in this conversation", + name: "Don't automatically mention Morgarita in this thread", }) .click(); - await expect(input).toHaveText("draft text"); + await expect(input).toHaveText("@Morgarita draft text"); await expect( composer.getByRole("button", { name: "Mention someone" }), ).toBeVisible(); @@ -656,7 +675,7 @@ test("always-mentioned agents remain selected without replaying their animation { timeout: 500 }, ); await expect( - composer.getByRole("button", { name: "Manage automatic agent mentions" }), + composer.getByRole("button", { name: "Manage mentions" }), ).toBeVisible(); await expect(input).toBeFocused(); await expect(composer.getByTestId("mention-autocomplete")).toHaveCount(0); @@ -690,7 +709,7 @@ test("always-mentioned agents remain selected without replaying their animation ).toHaveCount(1); }); -test("the unfocused main composer keeps its dismissed mention menu closed through a thread send", async ({ +test("the unfocused root composer keeps its dismissed mention menu closed through a thread send", async ({ page, }) => { await installAudienceFixtures(page, { sendMessageDelayMs: 1_500 }); @@ -698,12 +717,7 @@ test("the unfocused main composer keeps its dismissed mention menu closed throug const mainComposer = channelComposer(page); const mainInput = mainComposer.getByTestId("message-input"); - await automaticallyMention(mainComposer, "Morgarita"); - await mainInput.fill("@Morgarita earlier message"); - await mainInput.press("Enter"); - await expect(mainInput).toHaveText("@Morgarita "); - await mainInput.fill("@Morgarita"); - await expect(mainInput).toHaveText("@Morgarita"); + await mainInput.fill("@Mor"); await expect(mainComposer.getByTestId("mention-autocomplete")).toBeVisible(); await mainInput.press("Escape"); await expect(mainComposer.getByTestId("mention-autocomplete")).toHaveCount(0); @@ -760,9 +774,9 @@ test("pressing a mention overlay's own container keeps it open", async ({ }) => { await keepMentionedAgentsPinned(page); await installAudienceFixtures(page); - await openGeneral(page); + await openThread(page); - const composer = channelComposer(page); + const composer = threadComposer(page); const input = composer.getByTestId("message-input"); await composer.getByTestId("message-insert-mention").click(); const list = composer.getByTestId("mention-autocomplete"); @@ -782,7 +796,6 @@ test("pressing a mention overlay's own container keeps it open", async ({ // Same hazard on the options surface, where it needs no exotic scrollbar // setting to reproduce: the switch's label text is a container press, so the // overlay used to vanish before the forwarded click reached the switch. - await composer.getByTestId("mention-options-trigger").click(); const preference = composer.getByTestId("mention-keep-agents-pinned-toggle"); await expect(preference).toHaveAttribute("data-state", "checked"); await composer @@ -793,14 +806,14 @@ test("pressing a mention overlay's own container keeps it open", async ({ await expect(list).toBeVisible(); }); -test("the mention Options controls are reachable and operable by keyboard", async ({ +test("the mention setting is reachable and operable by keyboard", async ({ page, }) => { await keepMentionedAgentsPinned(page); await installAudienceFixtures(page); await openThread(page); - const mainComposer = channelComposer(page); + const mainComposer = threadComposer(page); const mainInput = mainComposer.getByTestId("message-input"); await mainInput.click(); await mainInput.fill("@Mor"); @@ -821,23 +834,15 @@ test("the mention Options controls are reachable and operable by keyboard", asyn }); // Forward Tab still selects the highlighted suggestion, so Shift+Tab is the - // route into the overlay. It only reaches the Options controls if the focus - // gate treats them as composer-owned focus rather than unmounting on the - // editor's blur. + // route into the always-visible setting. The focus gate must treat it as + // composer-owned focus rather than unmounting on the editor's blur. await mainInput.press("Shift+Tab"); - const optionsTrigger = mainComposer.getByTestId("mention-options-trigger"); - await expect(optionsTrigger).toBeFocused(); - - await page.keyboard.press("Enter"); const preference = mainComposer.getByTestId( "mention-keep-agents-pinned-toggle", ); - await expect(preference).toBeVisible(); + await expect(preference).toBeFocused(); await expect(preference).toHaveAttribute("data-state", "checked"); - // The switch sits before its trigger in the expanded surface's tab order. - await page.keyboard.press("Shift+Tab"); - await expect(preference).toBeFocused(); await page.keyboard.press("Space"); await expect(preference).toHaveAttribute("data-state", "unchecked"); @@ -866,9 +871,9 @@ test("the mention Options controls are reachable and operable by keyboard", asyn // under test. await mainInput.fill("@Mor"); await expect(list).toBeVisible(); - const threadInput = threadComposer(page).getByTestId("message-input"); - await threadInput.focus(); - await expect(threadInput).toBeFocused(); + const rootInput = channelComposer(page).getByTestId("message-input"); + await rootInput.focus(); + await expect(rootInput).toBeFocused(); await expect(list).toHaveCount(0); }); @@ -907,9 +912,9 @@ test("a manual mention persists when automatic mentions are enabled", async ({ }) => { await keepMentionedAgentsPinned(page); await installAudienceFixtures(page, { sendMessageDelayMs: 1_500 }); - await openGeneral(page); + await openThread(page); - const composer = channelComposer(page); + const composer = threadComposer(page); const input = composer.getByTestId("message-input"); await input.fill("@Mor"); await expect(composer.getByTestId("mention-autocomplete")).toBeVisible(); @@ -928,7 +933,7 @@ test("a manual mention persists when automatic mentions are enabled", async ({ await expect(autoPinConfirmation).not.toContainText( "Future messages in this channel will include this agent.", ); - await expect(autoPinConfirmation).toHaveAttribute("data-side", "right"); + await expect(autoPinConfirmation).toHaveAttribute("data-side", "left"); await expect(autoPinConfirmation.locator("span")).toHaveCSS( "white-space", "nowrap", @@ -948,8 +953,8 @@ test("a manual mention persists when automatic mentions are enabled", async ({ if (!addressControlBox || !confirmationBox) { throw new Error("Automatic mention confirmation is not laid out"); } - expect(confirmationBox.x).toBeGreaterThan( - addressControlBox.x + addressControlBox.width, + expect(confirmationBox.x + confirmationBox.width).toBeLessThanOrEqual( + addressControlBox.x, ); const turnOffAction = autoPinConfirmation.getByRole("button", { name: "Turn off", @@ -974,6 +979,9 @@ test("a manual mention persists when automatic mentions are enabled", async ({ .poll(() => readOutgoingMentionPubkeys(page, "@Morgarita hello")) .toContain(AGENT_A); + await expect(input).toHaveAttribute("contenteditable", "true", { + timeout: 2_500, + }); await input.fill("follow up"); await expect( composer.getByTestId(`composer-address-lock-${AGENT_A}`), @@ -989,9 +997,9 @@ test("the auto-pin popover can turn off automatic agent mentions", async ({ }) => { await keepMentionedAgentsPinned(page); await installAudienceFixtures(page); - await openGeneral(page); + await openThread(page); - const composer = channelComposer(page); + const composer = threadComposer(page); const input = composer.getByTestId("message-input"); await input.fill("@Mor"); await expect(composer.getByTestId("mention-autocomplete")).toBeVisible(); @@ -1010,10 +1018,7 @@ test("the auto-pin popover can turn off automatic agent mentions", async ({ await autoPinConfirmation.getByRole("button", { name: "Turn off" }).click(); await expect(composer.getByTestId("mention-autocomplete")).toBeVisible(); - await expect(composer.getByTestId("mention-options-trigger")).toHaveAttribute( - "aria-expanded", - "true", - ); + await expect(composer.getByTestId("mention-options-settings")).toBeVisible(); await expect( composer.getByTestId("mention-keep-agents-pinned-toggle"), ).toHaveAttribute("data-state", "unchecked"); @@ -1029,9 +1034,9 @@ test("the auto-pin popover can turn off automatic agent mentions", async ({ test("the auto-pin popover remains open while hovered", async ({ page }) => { await keepMentionedAgentsPinned(page); await installAudienceFixtures(page); - await openGeneral(page); + await openThread(page); - const composer = channelComposer(page); + const composer = threadComposer(page); const input = composer.getByTestId("message-input"); await input.fill("@Mor"); await expect(composer.getByTestId("mention-autocomplete")).toBeVisible(); @@ -1053,9 +1058,9 @@ test("removing the mention chip dismisses the auto-pin popover", async ({ }) => { await keepMentionedAgentsPinned(page); await installAudienceFixtures(page); - await openGeneral(page); + await openThread(page); - const composer = channelComposer(page); + const composer = threadComposer(page); const input = composer.getByTestId("message-input"); await input.fill("@Mor"); await expect(composer.getByTestId("mention-autocomplete")).toBeVisible(); @@ -1076,31 +1081,23 @@ test("removing the mention chip dismisses the auto-pin popover", async ({ await expect(autoPinConfirmation).toHaveCount(0); }); -test("automatic mentions are scoped to their channel or thread composer", async ({ +test("automatic mentions exist only in thread composers and stay thread-scoped", async ({ page, }) => { await installAudienceFixtures(page); await openGeneral(page); - await automaticallyMention(channelComposer(page), "Morgarita"); - await expect( - channelComposer(page).getByTestId(`composer-address-lock-${AGENT_A}`), - ).toBeVisible(); - await openThread(page); + const rootComposer = channelComposer(page); + await rootComposer.getByTestId("message-insert-mention").click(); await expect( - threadComposer(page).getByTestId(`composer-address-lock-${AGENT_A}`), + rootComposer.getByTestId("mention-options-settings"), + ).toHaveCount(0); + await expect( + rootComposer.getByRole("button", { name: /^Automatically mention / }), ).toHaveCount(0); + await openThread(page); await automaticallyMention(threadComposer(page), "Vogue"); - await openGeneral(page); - await expect( - channelComposer(page).getByTestId(`composer-address-lock-${AGENT_A}`), - ).toBeVisible(); - await expect( - channelComposer(page).getByTestId(`composer-address-lock-${AGENT_B}`), - ).toHaveCount(0); - - await openThread(page); await expect( threadComposer(page).getByTestId(`composer-address-lock-${AGENT_B}`), ).toBeVisible(); @@ -1111,72 +1108,119 @@ test("automatic mentions are scoped to their channel or thread composer", async ).toHaveCount(0); }); -test("a thread automatic mention preserves an explicitly unpinned root agent", async ({ +test("a root agent mention is explicit for one message and never becomes retained", async ({ page, }) => { await keepMentionedAgentsPinned(page); await installAudienceFixtures(page, { agentAName: "claude code" }); await openGeneral(page); - const rootComposer = channelComposer(page); - const rootInput = rootComposer.getByTestId("message-input"); - await automaticallyMention(rootComposer, "claude code"); + const composer = channelComposer(page); + const input = composer.getByTestId("message-input"); + const firstRootMessage = "@claude code one time"; + await input.fill("@cla"); + await expect(composer.getByTestId("mention-autocomplete")).toBeVisible(); + await input.press("Tab"); + await input.pressSequentially(" one time"); + await expect(input).toHaveText(firstRootMessage); await expect( - rootComposer.getByTestId(`composer-address-lock-${AGENT_A}`), - ).toBeVisible(); - await rootComposer - .getByTestId(`composer-address-lock-remove-${AGENT_A}`) - .click(); - await expect(rootInput).toHaveText(""); - await expect( - rootComposer.getByTestId(`composer-address-lock-${AGENT_A}`), + composer.getByTestId(`composer-address-lock-${AGENT_A}`), ).toHaveCount(0); + await input.press("Enter"); - await rootInput.fill("@cla"); - await expect(rootComposer.getByTestId("mention-autocomplete")).toBeVisible(); - await rootInput.press("Tab"); - await rootInput.type("one time"); - await rootInput.press("Enter"); - await expect(rootInput).toHaveText(""); - await expect( - rootComposer.getByTestId(`composer-address-lock-${AGENT_A}`), - ).toHaveCount(0); - - await openThread(page); - const activeThreadComposer = threadComposer(page); - const threadInput = activeThreadComposer.getByTestId("message-input"); - await threadInput.fill("@cla"); - await expect( - activeThreadComposer.getByTestId("mention-autocomplete"), - ).toBeVisible(); - await threadInput.press("Tab"); - await expect( - activeThreadComposer.getByTestId(`composer-address-lock-${AGENT_A}`), - ).toBeVisible(); - await threadInput.type("thread message"); - await threadInput.press("Enter"); + await expect(input).toHaveText(""); + await expect + .poll(() => + page.evaluate( + (pubkey) => + Boolean( + window.__BUZZ_E2E_SIGNED_EVENTS__?.some((event) => + (event.tags ?? []).some( + (tag) => tag[0] === "p" && tag[1] === pubkey, + ), + ), + ), + AGENT_A, + ), + ) + .toBe(true); + await input.fill("next root message"); + await input.press("Enter"); + await expect + .poll(() => readOutgoingMentionPubkeys(page, "next root message")) + .toEqual([]); +}); +test("a removed root-inherited agent stays excluded after the thread reopens", async ({ + page, +}) => { + await keepMentionedAgentsPinned(page); + await installAudienceFixtures(page); await openGeneral(page); - await expect( - channelComposer(page).getByTestId(`composer-address-lock-${AGENT_A}`), - ).toHaveCount(0); + await waitForMockLiveSubscription(page, "general"); - const restoredRootInput = channelComposer(page).getByTestId("message-input"); - await restoredRootInput.fill("@cla"); + const rootId = "c".repeat(64); + const rootContent = "@Morgarita root request"; + await page.evaluate( + ({ agentPubkey, content, eventId }) => { + window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__?.({ + channelName: "general", + content, + id: eventId, + mentionPubkeys: [agentPubkey], + }); + }, + { agentPubkey: AGENT_A, content: rootContent, eventId: rootId }, + ); + await waitForTimelineSettled(page); + + const rootRow = page + .getByTestId("message-timeline") + .locator(`[data-testid="message-row"][data-message-id="${rootId}"]`); + await expect(rootRow).toBeVisible(); + await rootRow.hover(); + await rootRow.getByRole("button", { name: "Reply" }).click(); + await expect(page.getByTestId("message-thread-panel")).toBeVisible(); + + let composer = threadComposer(page); await expect( - channelComposer(page).getByTestId("mention-autocomplete"), + composer.getByTestId(`composer-address-lock-${AGENT_A}`), ).toBeVisible(); - await restoredRootInput.press("Tab"); - await expect( - channelComposer(page).getByTestId(`composer-address-lock-${AGENT_A}`), - ).toHaveCount(0); - await restoredRootInput.type("one time"); - await restoredRootInput.press("Enter"); + await expect(composer.getByTestId("message-input")).toHaveText("@Morgarita "); + await composer.getByTestId(`composer-address-lock-remove-${AGENT_A}`).click(); + await expect(composer.getByTestId("message-input")).toHaveText(""); - await expect(restoredRootInput).toHaveText(""); + const threadPanel = page.getByTestId("message-thread-panel"); + await threadPanel.getByTestId("auxiliary-panel-close").click(); + await expect(threadPanel).toBeHidden(); + + const loadedRootTags = await page.evaluate(async (eventId) => { + const raw = await window.__BUZZ_E2E_INVOKE_MOCK_COMMAND__?.("get_event", { + eventId, + }); + return typeof raw === "string" + ? (JSON.parse(raw) as { tags?: string[][] }).tags + : null; + }, rootId); + expect(loadedRootTags).toContainEqual(["p", AGENT_A]); + + await rootRow.hover(); + await rootRow.getByRole("button", { name: "Reply" }).click(); + await expect(threadPanel).toBeVisible(); + + composer = threadComposer(page); + const input = composer.getByTestId("message-input"); await expect( - channelComposer(page).getByTestId(`composer-address-lock-${AGENT_A}`), + composer.getByTestId(`composer-address-lock-${AGENT_A}`), ).toHaveCount(0); + await expect(input).toHaveText(""); + + const reply = "plain reply after reopening"; + await input.fill(reply); + await input.press("Enter"); + await expect + .poll(() => readOutgoingMentionPubkeys(page, reply)) + .not.toContain(AGENT_A); }); test("an unchecked agent remains excluded while automatic mentions stay enabled", async ({ @@ -1184,10 +1228,10 @@ test("an unchecked agent remains excluded while automatic mentions stay enabled" }) => { await keepMentionedAgentsPinned(page); await installAudienceFixtures(page); - await openGeneral(page); - await automaticallyMention(channelComposer(page), "Morgarita"); + await openThread(page); + await automaticallyMention(threadComposer(page), "Morgarita"); - const composer = channelComposer(page); + const composer = threadComposer(page); const input = composer.getByTestId("message-input"); await composer.getByTestId(`composer-address-lock-remove-${AGENT_A}`).click(); await expect(input).toHaveText(""); @@ -1208,9 +1252,9 @@ test("re-adding a deleted automatic mention restores its automatic mention state }) => { await keepMentionedAgentsPinned(page); await installAudienceFixtures(page); - await openGeneral(page); + await openThread(page); - const composer = channelComposer(page); + const composer = threadComposer(page); const input = composer.getByTestId("message-input"); await automaticallyMention(composer, "Morgarita"); @@ -1246,13 +1290,13 @@ test("implicit automatic mentions stay out of persisted drafts", async ({ page, }) => { await installAudienceFixtures(page); - await openGeneral(page); - await automaticallyMention(channelComposer(page), "Morgarita"); - const input = channelComposer(page).getByTestId("message-input"); + await openThread(page); + await automaticallyMention(threadComposer(page), "Morgarita"); + const input = threadComposer(page).getByTestId("message-input"); await input.type("draft text"); - await openThread(page); await openGeneral(page); + await openThread(page); await expect(input).toHaveText("@Morgarita draft text"); await expect(input.locator(".agent-mention-highlight")).toHaveCount(1); @@ -1266,19 +1310,7 @@ test("implicit automatic mentions stay out of persisted drafts", async ({ await expect(page.getByTestId("chat-title")).toHaveText("random"); await expect - .poll(() => - page.evaluate((channelId) => { - for (const storageKey of Object.keys(window.localStorage)) { - if (!storageKey.startsWith("buzz-drafts.v2:")) continue; - const drafts = JSON.parse( - window.localStorage.getItem(storageKey) ?? "{}", - ) as Record; - const draft = drafts[channelId]; - if (draft?.channelId === channelId) return draft.content ?? ""; - } - return ""; - }, CHANNEL_ID), - ) + .poll(() => readPersistedDraftContent(page, `thread:${THREAD_ROOT_ID}`)) .toBe("draft text continues"); }); @@ -1286,37 +1318,23 @@ test("an authored duplicate leading mention survives draft restoration", async ( page, }) => { await installAudienceFixtures(page); - await openGeneral(page); - await automaticallyMention(channelComposer(page), "Morgarita"); - const input = channelComposer(page).getByTestId("message-input"); + await openThread(page); + await automaticallyMention(threadComposer(page), "Morgarita"); + const input = threadComposer(page).getByTestId("message-input"); await input.pressSequentially("@Morgarita authored duplicate"); - await openThread(page); await openGeneral(page); + await openThread(page); - await expect(input).toHaveText("@Morgarita @Morgarita authored duplicate"); - // Exact typed mentions now resolve on Space, so both the automatic prefix and - // the authored duplicate retain mention identity after restoration. - await expect(input.locator(".agent-mention-highlight")).toHaveCount(2); + await expect(input).toHaveText("@Morgarita authored duplicate"); + await expect(input.locator(".agent-mention-highlight")).toHaveCount(1); await page.goto(`/#/channels/${RANDOM_CHANNEL_ID}`, { waitUntil: "domcontentloaded", }); await expect(page.getByTestId("chat-title")).toHaveText("random"); await expect - .poll(() => - page.evaluate((channelId) => { - for (const storageKey of Object.keys(window.localStorage)) { - if (!storageKey.startsWith("buzz-drafts.v2:")) continue; - const drafts = JSON.parse( - window.localStorage.getItem(storageKey) ?? "{}", - ) as Record; - const draft = drafts[channelId]; - if (draft?.channelId === channelId) return draft.content ?? ""; - } - return ""; - }, CHANNEL_ID), - ) + .poll(() => readPersistedDraftContent(page, `thread:${THREAD_ROOT_ID}`)) .toBe("@Morgarita authored duplicate"); }); @@ -1324,8 +1342,8 @@ test("typed deletion preserves an identical authored mention in drafts", async ( page, }) => { await installAudienceFixtures(page); - await openGeneral(page); - const composer = channelComposer(page); + await openThread(page); + const composer = threadComposer(page); const input = composer.getByTestId("message-input"); await automaticallyMention(composer, "Morgarita"); @@ -1344,20 +1362,7 @@ test("typed deletion preserves an identical authored mention in drafts", async ( waitUntil: "domcontentloaded", }); await expect - .poll(() => - page.evaluate((channelId) => { - for (const storageKey of Object.keys(window.localStorage)) { - if (!storageKey.startsWith("buzz-drafts.v2:")) continue; - const draft = ( - JSON.parse( - window.localStorage.getItem(storageKey) ?? "{}", - ) as Record - )[channelId]; - if (draft) return draft.content ?? ""; - } - return ""; - }, CHANNEL_ID), - ) + .poll(() => readPersistedDraftContent(page, `thread:${THREAD_ROOT_ID}`)) .toBe("@Morgarita manual after typed deletion"); }); @@ -1365,8 +1370,8 @@ test("removing an automatic mention preserves an identical authored mention in d page, }) => { await installAudienceFixtures(page); - await openGeneral(page); - const composer = channelComposer(page); + await openThread(page); + const composer = threadComposer(page); const input = composer.getByTestId("message-input"); await automaticallyMention(composer, "Morgarita"); @@ -1377,20 +1382,7 @@ test("removing an automatic mention preserves an identical authored mention in d }); await expect - .poll(() => - page.evaluate((channelId) => { - for (const storageKey of Object.keys(window.localStorage)) { - if (!storageKey.startsWith("buzz-drafts.v2:")) continue; - const draft = ( - JSON.parse( - window.localStorage.getItem(storageKey) ?? "{}", - ) as Record - )[channelId]; - if (draft) return draft.content ?? ""; - } - return ""; - }, CHANNEL_ID), - ) + .poll(() => readPersistedDraftContent(page, `thread:${THREAD_ROOT_ID}`)) .toBe("@Morgarita manual after removal"); }); @@ -1398,35 +1390,22 @@ test("multiple automatic mentions stay out of persisted drafts", async ({ page, }) => { await installAudienceFixtures(page); - await openGeneral(page); - const composer = channelComposer(page); + await openThread(page); + const composer = threadComposer(page); const input = composer.getByTestId("message-input"); await automaticallyMention(composer, "Morgarita"); await automaticallyMention(composer, "Vogue"); await input.pressSequentially("draft text"); - await openThread(page); await openGeneral(page); - await expect(input).toHaveText("@Vogue @Morgarita draft text"); + await openThread(page); + await expect(input).toHaveText("@Morgarita @Vogue draft text"); await expect(input.locator(".agent-mention-highlight")).toHaveCount(2); await page.goto(`/#/channels/${RANDOM_CHANNEL_ID}`, { waitUntil: "domcontentloaded", }); await expect - .poll(() => - page.evaluate((channelId) => { - for (const storageKey of Object.keys(window.localStorage)) { - if (!storageKey.startsWith("buzz-drafts.v2:")) continue; - const draft = ( - JSON.parse( - window.localStorage.getItem(storageKey) ?? "{}", - ) as Record - )[channelId]; - if (draft) return draft.content ?? ""; - } - return ""; - }, CHANNEL_ID), - ) + .poll(() => readPersistedDraftContent(page, `thread:${THREAD_ROOT_ID}`)) .toBe("draft text"); }); @@ -1434,8 +1413,8 @@ test("re-enabling an automatic mention preserves an authored duplicate after dra page, }) => { await installAudienceFixtures(page); - await openGeneral(page); - const composer = channelComposer(page); + await openThread(page); + const composer = threadComposer(page); const input = composer.getByTestId("message-input"); await automaticallyMention(composer, "Morgarita"); @@ -1444,30 +1423,18 @@ test("re-enabling an automatic mention preserves an authored duplicate after dra await automaticallyMention(composer, "Morgarita"); await input.pressSequentially("@Morgarita authored duplicate"); - await openThread(page); await openGeneral(page); + await openThread(page); - await expect(input).toHaveText("@Morgarita @Morgarita authored duplicate"); - await expect(input.locator(".agent-mention-highlight")).toHaveCount(2); + await expect(input).toHaveText("@Morgarita authored duplicate"); + await expect(input.locator(".agent-mention-highlight")).toHaveCount(1); await page.goto(`/#/channels/${RANDOM_CHANNEL_ID}`, { waitUntil: "domcontentloaded", }); await expect(page.getByTestId("chat-title")).toHaveText("random"); await expect - .poll(() => - page.evaluate((channelId) => { - for (const storageKey of Object.keys(window.localStorage)) { - if (!storageKey.startsWith("buzz-drafts.v2:")) continue; - const drafts = JSON.parse( - window.localStorage.getItem(storageKey) ?? "{}", - ) as Record; - const draft = drafts[channelId]; - if (draft?.channelId === channelId) return draft.content ?? ""; - } - return ""; - }, CHANNEL_ID), - ) + .poll(() => readPersistedDraftContent(page, `thread:${THREAD_ROOT_ID}`)) .toBe("@Morgarita authored duplicate"); }); @@ -1475,8 +1442,8 @@ test("a restored multi-word automatic mention remains a chip with the caret afte page, }) => { await installAudienceFixtures(page, { agentAName: "claude code" }); - await openGeneral(page); - const originalComposer = channelComposer(page); + await openThread(page); + const originalComposer = threadComposer(page); await automaticallyMention(originalComposer, "claude code"); const originalInput = originalComposer.getByTestId("message-input"); await originalInput.pressSequentially("hello"); @@ -1490,9 +1457,9 @@ test("a restored multi-word automatic mention remains a chip with the caret afte waitUntil: "domcontentloaded", }); await expect(page.getByTestId("chat-title")).toHaveText("random"); - await openGeneral(page); + await openThread(page); - const composer = channelComposer(page); + const composer = threadComposer(page); const input = composer.getByTestId("message-input"); const expectedContent = "@claude code "; await expect(input).toHaveText(expectedContent); @@ -1501,7 +1468,7 @@ test("a restored multi-word automatic mention remains a chip with the caret afte composer.getByTestId(`composer-address-lock-${AGENT_A}`), ).toBeVisible(); await expect( - composer.getByRole("button", { name: "Manage automatic agent mentions" }), + composer.getByRole("button", { name: "Manage mentions" }), ).toBeVisible(); await page.waitForTimeout(500); await expect(input.locator(".agent-mention-highlight")).toHaveCount(1); @@ -1524,9 +1491,9 @@ test("reduced motion removes addressed agents without spatial animation", async await page.emulateMedia({ reducedMotion: "reduce" }); await keepMentionedAgentsPinned(page); await installAudienceFixtures(page); - await openGeneral(page); + await openThread(page); - const composer = channelComposer(page); + const composer = threadComposer(page); const input = composer.getByTestId("message-input"); await input.fill("@Mor"); await expect(composer.getByTestId("mention-autocomplete")).toBeVisible(); @@ -1572,7 +1539,7 @@ test("the mention-button placement fits the narrow composer", async ({ await automaticallyMention(overlay, "Vogue"); await expect(overlay.getByTestId("composer-address-locks")).toBeVisible(); await expect( - overlay.getByRole("button", { name: "Manage automatic agent mentions" }), + overlay.getByRole("button", { name: "Manage mentions" }), ).toBeVisible(); await waitForAnimations(page); await composer.screenshot({ path: `${SHOTS}/narrow-mention-button.png` }); @@ -1581,9 +1548,9 @@ test("the mention-button placement fits the narrow composer", async ({ test("captures the lightweight auto-pin popover", async ({ page }) => { await seedTheme(page, "buzz-dark"); await installAudienceFixtures(page); - await openGeneral(page); + await openThread(page); - const composer = channelComposer(page); + const composer = threadComposer(page); const input = composer.getByTestId("message-input"); await input.fill("draft text"); await pressPrimaryShiftM(page); diff --git a/desktop/tests/e2e/send-channel-binding.spec.ts b/desktop/tests/e2e/send-channel-binding.spec.ts index 3a7fdf7613..7854658dd0 100644 --- a/desktop/tests/e2e/send-channel-binding.spec.ts +++ b/desktop/tests/e2e/send-channel-binding.spec.ts @@ -84,12 +84,12 @@ test("message with agent mention lands in compose-time channel despite mid-send await input.press("Enter"); await page.keyboard.type(` ${MESSAGE_TEXT}`); - // Verify the inline mention and persistent address are present before submitting. + // Verify the inline mention is present without creating thread-retained state. await expect(input).toHaveText(`@BotA ${MESSAGE_TEXT}`); await expect(input.locator(".agent-mention-highlight")).toHaveText("BotA"); await expect( page.getByTestId(`composer-address-lock-${OUT_OF_CHANNEL_BOT_PUBKEY}`), - ).toBeVisible(); + ).toHaveCount(0); // Snapshot the baseline command count before sending const baselineCommands = await readCommandLog(page);