From c31b0d989f78d8cbfa8b317b4eb5d2439a9d8e94 Mon Sep 17 00:00:00 2001 From: Pavlo Shylo Date: Mon, 14 Sep 2026 19:29:03 +0100 Subject: [PATCH] fix(notifications): click payload in wire shape, no legacy context MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The shell projected an envelope's routing fields into a `{ context: {…} }` click payload, read field by field from the `type` + `attributes` pair with a fallback onto the typed `context` the backend no longer writes. The frontend's `resolveNatsNotificationRoute` now reads `type` + `attributes` only, so a `{ context }` payload resolved to nothing and every OS-toast click would land on the notifications page. The click payload is now the envelope narrowed to its routing fields, in the envelope's own shape: top-level `type` plus `attributes` with `ticketId`, `dialogId` and `approvalRequestId`. The action buttons read the same shape, and `note_resolution` takes the envelope's `attributes` only. The Windows activation URI and the button arguments carry the payload as `payload=` instead of `context=`; a toast left in the Action Center by a previous build opens the window without navigating. --- docs/auth-and-notifications.md | 13 +- src-tauri/src/lib.rs | 4 +- src-tauri/src/notification_actions.rs | 153 +++++++++-------- src-tauri/src/notifications.rs | 226 ++++++++++---------------- src-tauri/src/windows_toast.rs | 23 ++- 5 files changed, 194 insertions(+), 225 deletions(-) diff --git a/docs/auth-and-notifications.md b/docs/auth-and-notifications.md index d571ae2..8f500dd 100644 --- a/docs/auth-and-notifications.md +++ b/docs/auth-and-notifications.md @@ -286,10 +286,11 @@ dials with a fresh bearer. - Notifications fire **only when the main window is hidden or unfocused**. The webview's own subscription keeps driving the in-app drawer — duplicate delivery, different sinks. A badge accumulates while hidden and clears on focus. -- The click payload is the envelope's routing `context`, narrowed to the fields - the frontend's route mapping reads plus `approvalRequestId`, which the macOS - action buttons need. The rest of a context can be arbitrarily large, and it has - to fit inside a Windows activation URI. +- The click payload is the envelope narrowed to its routing fields, in the + envelope's own shape: top-level `type` plus `attributes` with `ticketId`, + `dialogId` and `approvalRequestId` — what the frontend's route mapping reads, + plus the id the action buttons need. The rest of `attributes` can be + arbitrarily large, and the payload has to fit inside a Windows activation URI. - Every logout path tears the subscription down — including a session death the shell detects itself, which the webview may not notice for hours while idle in the tray. Otherwise the previous user's notification content would keep @@ -378,8 +379,8 @@ been decided — under the same notification id, carrying: | Field | Meaning | |---|---| | `eventType` | `CREATED` for the first push, `UPDATED` for one that supersedes it. Absent → `CREATED`. | -| `context.resolution` | Backend `ApprovalResolution`: `PENDING`, `APPROVED`, `REJECTED`, `CANCELLED`. Absent until decided. | -| `context.resolvedByName` | Who decided it. | +| `attributes.resolution` | Backend `ApprovalResolution`: `PENDING`, `APPROVED`, `REJECTED`, `CANCELLED`. | +| `attributes.resolvedByName` | Who decided it. | That is a *correction*, not a second notification, and the shell has to read it as one — otherwise the republished copy arrives as a fresh banner still offering a diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index 2122711..a983b4b 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -73,7 +73,7 @@ pub(crate) const MAIN_LABEL: &str = "main"; /// in the user's **default browser**, where their SSO session already is, so /// the ticket cannot be read out of a webview we control; it has to be handed /// back by the OS. -/// - `openframe-console://notify?context=…` — what a Windows toast activates +/// - `openframe-console://notify?payload=…` — what a Windows toast activates /// (`notifications::CLICK_URI_PREFIX`). /// /// Registered on macOS by `Info.plist` (`CFBundleURLTypes`) and on Windows by @@ -1648,7 +1648,7 @@ mod tests { #[test] fn other_hosts_on_our_scheme_are_not_callbacks() { assert!(!is_auth_callback(&parse( - "openframe-console://notify?context=%7B%7D" + "openframe-console://notify?payload=%7B%7D" ))); } diff --git a/src-tauri/src/notification_actions.rs b/src-tauri/src/notification_actions.rs index c3c45a1..2729ba7 100644 --- a/src-tauri/src/notification_actions.rs +++ b/src-tauri/src/notification_actions.rs @@ -30,17 +30,11 @@ const DIALOG_CHANGED_EVENT: &str = "chat:dialog-changed"; /// Notification types that earn action buttons. The rest of the set (tickets, /// client chats) has no action the shell can complete on its own. /// -/// Three spellings for one decision: the spec catalog split the approval type by -/// ticket linkage, the legacy context calls both `ADMIN_APPROVAL_REQUEST`, and -/// which arrives depends on the contract that wrote the envelope. The buttons -/// are the same either way — a press resolves the request by id. -const APPROVAL_TYPES: [&str; 3] = [ - "ADMIN_APPROVAL_REQUEST", - "TICKET_APPROVAL_REQUEST", - "MINGO_APPROVAL_REQUEST", -]; -/// Unsplit, and spelled the same in both contracts. -const MESSAGE_CONTEXT_TYPE: &str = "ADMIN_AI_MESSAGE"; +/// Two spellings for one decision: the catalog splits the approval type by +/// ticket linkage. The buttons are the same either way — a press resolves the +/// request by id. +const APPROVAL_TYPES: [&str; 2] = ["TICKET_APPROVAL_REQUEST", "MINGO_APPROVAL_REQUEST"]; +const MESSAGE_TYPE: &str = "ADMIN_AI_MESSAGE"; /// Which button set a notification gets. Derived from the click payload rather /// than passed in, so the envelope contract lives in one place and the same @@ -72,7 +66,7 @@ fn kind_for(click: Option<&serde_json::Value>) -> ActionKind { /// The approval request a payload is about — the primitive fact the Approval /// button set is derived from, rather than the other way round. `None` unless -/// the context says it is an approval **and** the id survived the payload +/// the type says it is an approval **and** the id survived the payload /// projection, because an Approve that can resolve nothing is worse than no /// Approve at all. /// @@ -80,17 +74,17 @@ fn kind_for(click: Option<&serde_json::Value>) -> ActionKind { /// toast this replaces, and which request a press resolves are all the same /// question. pub(crate) fn approval_request_id(click: Option<&serde_json::Value>) -> Option { - context_str(click, "type") + payload_type(click) .is_some_and(|kind| APPROVAL_TYPES.contains(&kind.as_str())) - .then(|| context_str(click, "approvalRequestId")) + .then(|| payload_attr(click, "approvalRequestId")) .flatten() } /// The conversation a payload is about, on the same terms as /// [`approval_request_id`]. fn dialog_id(click: Option<&serde_json::Value>) -> Option { - (context_str(click, "type").as_deref() == Some(MESSAGE_CONTEXT_TYPE)) - .then(|| context_str(click, "dialogId")) + (payload_type(click).as_deref() == Some(MESSAGE_TYPE)) + .then(|| payload_attr(click, "dialogId")) .flatten() } @@ -115,19 +109,17 @@ const SETTLED_RESOLUTIONS: [&str; 3] = ["APPROVED", "REJECTED", "CANCELLED"]; /// Record the verdict an envelope carries, if it carries one. The gateway /// republishes an approval request once it is decided — same notification id, -/// `eventType: "UPDATED"`, the verdict in `context.resolution` — so that every -/// consumer can bring its copy up to date. Taking it at face value is what lets -/// a decision made anywhere (the web UI, another device, another admin) reach -/// the banner sitting in this machine's Action Center. +/// `eventType: "UPDATED"`, the verdict in `attributes.resolution` — so that +/// every consumer can bring its copy up to date. Taking it at face value is what +/// lets a decision made anywhere (the web UI, another device, another admin) +/// reach the banner sitting in this machine's Action Center. /// -/// Takes a source object carrying the resolution fields — the envelope's -/// `attributes` map (the spec contract) or its legacy `context` object — not -/// the projected click payload the rest of this module reads: `resolution` is -/// not one of the fields that survives the projection. The caller reads both -/// sources; recording is idempotent. -pub(crate) fn note_resolution(context: Option<&serde_json::Value>) { - let Some(context) = context else { return }; - let settled = string_field(context, "resolution").is_some_and(|resolution| { +/// Takes the envelope's `attributes` map — not the projected click payload the +/// rest of this module reads: `resolution` is not one of the fields that +/// survives the projection. Recording is idempotent. +pub(crate) fn note_resolution(attributes: Option<&serde_json::Value>) { + let Some(attributes) = attributes else { return }; + let settled = string_field(attributes, "resolution").is_some_and(|resolution| { SETTLED_RESOLUTIONS .iter() .any(|settled| resolution.eq_ignore_ascii_case(settled)) @@ -135,7 +127,7 @@ pub(crate) fn note_resolution(context: Option<&serde_json::Value>) { if !settled { return; } - if let Some(request_id) = string_field(context, "approvalRequestId") { + if let Some(request_id) = string_field(attributes, "approvalRequestId") { remember_resolved(&request_id); } } @@ -246,7 +238,7 @@ fn announce_dialog_change(app: &AppHandle, context: &ActionContext) { // exists to decide whether a REPLY has somewhere to go. An approval carries // a conversation too, and resolving one changes it just as much — the gate // would drop exactly the notification whose dialog most needs refetching. - let Some(dialog_id) = context_str(context.payload.as_ref(), "dialogId") else { + let Some(dialog_id) = payload_attr(context.payload.as_ref(), "dialogId") else { return; }; if let Err(e) = app.emit_to( @@ -399,33 +391,38 @@ pub(crate) fn post( crate::windows_toast::post(app, title, body, click, user_id, kind, delivery); } -/// A string field of the click payload's `context`, if present and non-empty. -fn context_str(click: Option<&serde_json::Value>, key: &str) -> Option { - string_field(click?.pointer("/context")?, key) +/// The click payload's `type`, if present and non-empty. +fn payload_type(click: Option<&serde_json::Value>) -> Option { + string_field(click?, "type") +} + +/// A string field of the click payload's `attributes`, if present and non-empty. +fn payload_attr(click: Option<&serde_json::Value>, key: &str) -> Option { + string_field(click?.pointer("/attributes")?, key) } #[cfg(test)] mod tests { use super::*; - fn click(context: serde_json::Value) -> serde_json::Value { - serde_json::json!({ "context": context }) + fn click(kind: &str, attributes: serde_json::Value) -> serde_json::Value { + serde_json::json!({ "type": kind, "attributes": attributes }) } #[test] - fn actionable_contexts_get_their_buttons() { + fn actionable_payloads_get_their_buttons() { assert_eq!( - kind_for(Some(&click(serde_json::json!({ - "type": "ADMIN_APPROVAL_REQUEST", - "approvalRequestId": "req-1", - })))), + kind_for(Some(&click( + "MINGO_APPROVAL_REQUEST", + serde_json::json!({ "approvalRequestId": "req-1" }) + ))), ActionKind::Approval ); assert_eq!( - kind_for(Some(&click(serde_json::json!({ - "type": "ADMIN_AI_MESSAGE", - "dialogId": "dlg-1", - })))), + kind_for(Some(&click( + "ADMIN_AI_MESSAGE", + serde_json::json!({ "dialogId": "dlg-1" }) + ))), ActionKind::Message ); } @@ -433,33 +430,55 @@ mod tests { /// No id, no button: an Approve that cannot resolve anything is worse than /// no Approve at all. #[test] - fn contexts_without_the_id_the_action_needs_stay_default() { + fn payloads_without_the_id_the_action_needs_stay_default() { assert_eq!( kind_for(Some(&click( - serde_json::json!({ "type": "ADMIN_APPROVAL_REQUEST", "ticketId": "t-1" }) + "TICKET_APPROVAL_REQUEST", + serde_json::json!({ "ticketId": "t-1" }) ))), ActionKind::Default ); assert_eq!( kind_for(Some(&click( - serde_json::json!({ "type": "ADMIN_AI_MESSAGE", "dialogId": "" }) + "ADMIN_AI_MESSAGE", + serde_json::json!({ "dialogId": "" }) ))), ActionKind::Default ); assert_eq!( kind_for(Some(&click( - serde_json::json!({ "type": "TICKET_ASSIGNED", "ticketId": "t-1" }) + "TICKET_ASSIGNED", + serde_json::json!({ "ticketId": "t-1" }) ))), ActionKind::Default ); assert_eq!(kind_for(None), ActionKind::Default); } + /// The ids live under `attributes`; the retired `context` wrapper, or ids at + /// the top level, earn nothing. + #[test] + fn ids_outside_attributes_earn_no_buttons() { + assert_eq!( + kind_for(Some(&serde_json::json!({ + "context": { "type": "MINGO_APPROVAL_REQUEST", "approvalRequestId": "req-1" } + }))), + ActionKind::Default + ); + assert_eq!( + kind_for(Some(&serde_json::json!({ + "type": "MINGO_APPROVAL_REQUEST", + "approvalRequestId": "req-1" + }))), + ActionKind::Default + ); + } + fn approval(request_id: &str) -> serde_json::Value { - click(serde_json::json!({ - "type": "ADMIN_APPROVAL_REQUEST", - "approvalRequestId": request_id, - })) + click( + "MINGO_APPROVAL_REQUEST", + serde_json::json!({ "approvalRequestId": request_id }), + ) } /// The verdict on the wire is what settles a request, wherever it was made. @@ -468,7 +487,6 @@ mod tests { #[test] fn only_a_terminal_resolution_settles_a_request() { let pending = serde_json::json!({ - "type": "ADMIN_APPROVAL_REQUEST", "approvalRequestId": "wire-pending", "resolution": "PENDING", }); @@ -478,14 +496,13 @@ mod tests { for (i, resolution) in ["APPROVED", "rejected", "CANCELLED"].iter().enumerate() { let id = format!("wire-settled-{i}"); note_resolution(Some(&serde_json::json!({ - "type": "ADMIN_APPROVAL_REQUEST", "approvalRequestId": id, "resolution": resolution, }))); assert!(is_resolved(&id), "{resolution} should settle the request"); } - // Nothing to record: no verdict, no id, no context at all. + // Nothing to record: no verdict, no id, no attributes at all. note_resolution(Some( &serde_json::json!({ "approvalRequestId": "wire-bare" }), )); @@ -493,23 +510,22 @@ mod tests { note_resolution(None); } - /// Both catalog spellings earn the same buttons as the legacy one — without - /// this, approvals lose their buttons the day the spec type is what arrives. + /// Both halves of the catalog's approval split earn the same buttons. #[test] fn every_approval_spelling_earns_the_same_buttons() { for kind in APPROVAL_TYPES { assert_eq!( - kind_for(Some(&click(serde_json::json!({ - "type": kind, - "approvalRequestId": "req-1", - })))), + kind_for(Some(&click( + kind, + serde_json::json!({ "approvalRequestId": "req-1" }) + ))), ActionKind::Approval, "{kind}" ); } } - /// The verdict as the spec contract carries it. + /// The verdict as the wire carries it. #[test] fn a_verdict_in_attributes_settles_a_request() { note_resolution(Some(&serde_json::json!({ @@ -568,19 +584,22 @@ mod tests { approval_request_id(Some(&approval("req-1"))).as_deref(), Some("req-1") ); - assert!(approval_request_id(Some(&click(serde_json::json!({ - "type": "ADMIN_AI_MESSAGE", "dialogId": "dlg-1", - })))) + assert!(approval_request_id(Some(&click( + "ADMIN_AI_MESSAGE", + serde_json::json!({ "dialogId": "dlg-1" }) + ))) .is_none()); assert!(approval_request_id(Some(&click( - serde_json::json!({ "type": "TICKET_ASSIGNED", "ticketId": "t-1" }) + "TICKET_ASSIGNED", + serde_json::json!({ "ticketId": "t-1" }) ))) .is_none()); assert!(approval_request_id(None).is_none()); // The id an approval carries is the id a press resolves: a payload whose // id did not survive the projection has neither. assert!(approval_request_id(Some(&click( - serde_json::json!({ "type": "ADMIN_APPROVAL_REQUEST", "ticketId": "t-1" }) + "TICKET_APPROVAL_REQUEST", + serde_json::json!({ "ticketId": "t-1" }) ))) .is_none()); } diff --git a/src-tauri/src/notifications.rs b/src-tauri/src/notifications.rs index 0a8e41b..7fee294 100644 --- a/src-tauri/src/notifications.rs +++ b/src-tauri/src/notifications.rs @@ -205,9 +205,7 @@ fn maybe_notify(app: &AppHandle, envelope: &serde_json::Value, user_id: &str) { // that is true whether or not this envelope is one the user gets to see. #[cfg(any(target_os = "macos", target_os = "windows"))] { - // Both contracts, since either may carry the verdict; recording is idempotent. crate::notification_actions::note_resolution(envelope.get("attributes")); - crate::notification_actions::note_resolution(envelope.get("context")); } let Some(title) = string_field(envelope, "title") else { @@ -303,50 +301,49 @@ fn delivery_of(envelope: &serde_json::Value) -> Delivery { // Click payload + delivery // --------------------------------------------------------------------------- -/// The webview's `notification:click` payload: the envelope's routing fields -/// in wire shape, which the frontend's `resolveNatsNotificationAction` maps to a -/// route. Only the fields that mapping reads — plus `approvalRequestId`, which -/// the Approve/Reject buttons resolve against the chat API — survive. The rest -/// of the envelope can be arbitrarily large (an approval request carries the -/// whole `toolCalls` array), and the payload has to fit both in a Windows +/// The webview's `notification:click` payload: the envelope narrowed to its +/// routing fields, in the envelope's own shape — top-level `type` plus a flat +/// `attributes` map — which the frontend's `resolveNatsNotificationRoute` maps +/// to a route. Only the fields that mapping reads — plus `approvalRequestId`, +/// which the Approve/Reject buttons resolve against the chat API — survive. The +/// rest of the envelope can be arbitrarily large (an approval request carries +/// the whole `toolCalls` JSON), and the payload has to fit both in a Windows /// activation URI and in a toast payload that repeats it once per button; every /// id kept here is a UUID. `None` when the envelope points at nothing openable; /// the click then only raises the window. /// -/// Read field by field from both contracts: `type` + `attributes` is the spec -/// one, `context` the legacy fallback that leaves the wire once the backend -/// stops dual-writing. The payload keeps its own `context` wrapper — that name -/// is this app's protocol with the webview, not the backend's. +/// The typed `context` the backend used to nest beside `attributes` is retired +/// and ignored if it still appears. fn click_payload(envelope: &serde_json::Value) -> Option { - let attributes = envelope.get("attributes"); - let context = envelope.get("context"); - let mut routing = serde_json::Map::new(); - let kind = envelope - .get("type") - .filter(|value| !value.is_null()) - .or_else(|| context.and_then(|context| context.get("type"))); - if let Some(kind) = kind { - routing.insert("type".to_string(), kind.clone()); - } - for key in ["ticketId", "dialogId", "approvalRequestId"] { - let value = attributes - .and_then(|attributes| attributes.get(key)) - .or_else(|| context.and_then(|context| context.get(key))); - if let Some(value) = value { - routing.insert(key.to_string(), value.clone()); + let mut payload = serde_json::Map::new(); + if let Some(kind) = envelope.get("type").filter(|value| !value.is_null()) { + payload.insert("type".to_string(), kind.clone()); + } + let mut attributes = serde_json::Map::new(); + if let Some(source) = envelope.get("attributes") { + for key in ["ticketId", "dialogId", "approvalRequestId"] { + if let Some(value) = source.get(key) { + attributes.insert(key.to_string(), value.clone()); + } } } - (!routing.is_empty()).then(|| serde_json::json!({ "context": routing })) + if !attributes.is_empty() { + payload.insert( + "attributes".to_string(), + serde_json::Value::Object(attributes), + ); + } + (!payload.is_empty()).then_some(serde_json::Value::Object(payload)) } -/// `openframe-console://notify?context=`. +/// `openframe-console://notify?payload=`. #[cfg(any(target_os = "windows", test))] pub(crate) fn click_uri(click: Option<&serde_json::Value>) -> String { use percent_encoding::{utf8_percent_encode, NON_ALPHANUMERIC}; - match click.and_then(|c| c.get("context")) { - Some(context) => format!( - "{CLICK_URI_PREFIX}?context={}", - utf8_percent_encode(&context.to_string(), NON_ALPHANUMERIC) + match click { + Some(click) => format!( + "{CLICK_URI_PREFIX}?payload={}", + utf8_percent_encode(&click.to_string(), NON_ALPHANUMERIC) ), None => CLICK_URI_PREFIX.to_string(), } @@ -360,23 +357,21 @@ fn parse_click_uri(uri: &str) -> Option { let query = rest.strip_prefix('/').unwrap_or(rest).strip_prefix('?')?; let encoded = query .split('&') - .find_map(|pair| pair.strip_prefix("context="))?; + .find_map(|pair| pair.strip_prefix("payload="))?; let json = percent_encoding::percent_decode_str(encoded) .decode_utf8() .ok()?; - payload_from_context_json(&json) + payload_from_json(&json) } -/// Wrap a decoded `context` object back into the shape every click path carries -/// it in. Both Windows activation transports land here — the URI a toast body -/// activates and the arguments a toast button carries — so the envelope shape -/// the webview and `kind_for` depend on is asserted once. +/// Decode a click payload carried as JSON. Both Windows activation transports +/// land here — the URI a toast body activates and the arguments a toast button +/// carries — so the shape the webview and `kind_for` depend on is asserted once: +/// an object, or nothing. #[cfg(any(target_os = "windows", test))] -pub(crate) fn payload_from_context_json(json: &str) -> Option { - let context: serde_json::Value = serde_json::from_str(json).ok()?; - context - .is_object() - .then(|| serde_json::json!({ "context": context })) +pub(crate) fn payload_from_json(json: &str) -> Option { + let payload: serde_json::Value = serde_json::from_str(json).ok()?; + payload.is_object().then_some(payload) } /// Handles an `openframe-console://notify` URI from a Windows toast click, @@ -401,13 +396,11 @@ pub(crate) fn handle_notification_uri(app: &AppHandle, uri: &str) -> bool { pub(crate) fn deliver_click(app: &AppHandle, payload: Option) { match payload { Some(payload) => { - // Context can carry user-facing content — identify it at info, dump - // it only at debug, same policy as notification_router. + // The payload can carry user-facing content — identify it at info, + // dump it only at debug, same policy as notification_router. log::info!( - "[notifications] activated — forwarding context.type={}", - payload - .pointer("/context/type") - .unwrap_or(&serde_json::Value::Null) + "[notifications] activated — forwarding type={}", + payload.get("type").unwrap_or(&serde_json::Value::Null) ); log::debug!("[notifications] click payload: {payload}"); emit_or_stash(app, payload); @@ -578,7 +571,8 @@ mod tests { fn click_uri_roundtrip() { let envelope = serde_json::json!({ "title": "Ticket updated", - "context": { "type": "TICKET_STATUS_CHANGED", "ticketId": "6a4fda9ba8b65c28c4dbf6ba" } + "type": "TICKET_STATUS_CHANGED", + "attributes": { "ticketId": "6a4fda9ba8b65c28c4dbf6ba" } }); let payload = click_payload(&envelope).unwrap(); assert_eq!( @@ -590,7 +584,8 @@ mod tests { #[test] fn click_uri_roundtrip_survives_reserved_chars() { let envelope = serde_json::json!({ - "context": { "type": "CLIENT_AI_MESSAGE", "dialogId": "abc/д ф&x=1" } + "type": "CLIENT_AI_MESSAGE", + "attributes": { "dialogId": "abc/д ф&x=1" } }); let payload = click_payload(&envelope).unwrap(); assert_eq!( @@ -599,10 +594,10 @@ mod tests { ); } - /// The spec contract: top-level `type` + flat `attributes`, no `context` at - /// all — the shape the wire settles on once the backend stops dual-writing. + /// The payload is the envelope in its own shape, narrowed to the routing + /// fields: top-level `type`, the ids under `attributes`. #[test] - fn spec_shaped_envelope_routes_without_context() { + fn click_payload_is_the_envelope_narrowed() { let envelope = serde_json::json!({ "title": "Approval required", "type": "TICKET_APPROVAL_REQUEST", @@ -612,74 +607,50 @@ mod tests { "toolCalls": "[]", } }); - let payload = click_payload(&envelope).unwrap(); assert_eq!( - payload, - serde_json::json!({ "context": { + click_payload(&envelope).unwrap(), + serde_json::json!({ "type": "TICKET_APPROVAL_REQUEST", - "ticketId": "abc", - "approvalRequestId": "0a2a0b3c-9d1e-4f5a-8b7c-6d5e4f3a2b1c", - } }) + "attributes": { + "ticketId": "abc", + "approvalRequestId": "0a2a0b3c-9d1e-4f5a-8b7c-6d5e4f3a2b1c", + } + }) ); } - /// Dual-write window: both shapes on the wire, the spec half wins. A null - /// top-level `type` (a legacy-path document) falls back to the context's. + /// The retired typed `context` is ignored: on its own it yields no payload, + /// and beside `attributes` it contributes nothing. #[test] - fn attributes_win_over_context_and_null_type_falls_back() { - let envelope = serde_json::json!({ + fn a_legacy_context_on_the_envelope_is_ignored() { + let legacy_only = serde_json::json!({ + "title": "Ticket updated", + "context": { "type": "CLIENT_AI_MESSAGE", "dialogId": "d-2" } + }); + assert!(click_payload(&legacy_only).is_none()); + + let dual = serde_json::json!({ "type": "TICKET_APPROVAL_REQUEST", "attributes": { "ticketId": "new" }, "context": { "type": "ADMIN_APPROVAL_REQUEST", "ticketId": "old", "dialogId": "d-1" } }); - let payload = click_payload(&envelope).unwrap(); assert_eq!( - payload, - serde_json::json!({ "context": { + click_payload(&dual).unwrap(), + serde_json::json!({ "type": "TICKET_APPROVAL_REQUEST", - "ticketId": "new", - "dialogId": "d-1", - } }) - ); - - let legacy_only = serde_json::json!({ - "type": serde_json::Value::Null, - "context": { "type": "CLIENT_AI_MESSAGE", "dialogId": "d-2" } - }); - let payload = click_payload(&legacy_only).unwrap(); - assert_eq!( - payload, - serde_json::json!({ "context": { "type": "CLIENT_AI_MESSAGE", "dialogId": "d-2" } }) + "attributes": { "ticketId": "new" } + }) ); } #[test] - fn envelopes_without_context_have_no_payload() { + fn envelopes_without_routing_fields_have_no_payload() { assert!(click_payload(&serde_json::json!({ "title": "Hi" })).is_none()); - assert!(click_payload(&serde_json::json!({ "context": "not-an-object" })).is_none()); - assert!(click_payload(&serde_json::json!({ "context": { "ticketId": "" } })).is_some()); + assert!(click_payload(&serde_json::json!({ "attributes": "not-an-object" })).is_none()); + assert!(click_payload(&serde_json::json!({ "attributes": { "ticketId": "" } })).is_some()); assert_eq!(click_uri(None), CLICK_URI_PREFIX); } - /// Each contract fills what the other leaves out, rather than the payload - /// being taken wholesale from whichever was found first. - #[test] - fn the_two_contracts_are_read_field_by_field() { - let envelope = serde_json::json!({ - "type": "TICKET_ESCALATED_BY_USER", - "attributes": { "ticketId": "t-3" }, - "context": { "type": "TICKET_ESCALATED_BY_USER", "ticketId": "t-3", "dialogId": "dlg-3" }, - }); - assert_eq!( - click_payload(&envelope).unwrap(), - serde_json::json!({ "context": { - "type": "TICKET_ESCALATED_BY_USER", - "ticketId": "t-3", - "dialogId": "dlg-3", - } }) - ); - } - /// New catalog types are expected to arrive without a shell release, and the /// webview routes by id. #[test] @@ -690,12 +661,15 @@ mod tests { }); assert_eq!( click_payload(&envelope).unwrap(), - serde_json::json!({ "context": { "type": "SOMETHING_SHIPPED_LATER", "ticketId": "t-4" } }) + serde_json::json!({ "type": "SOMETHING_SHIPPED_LATER", "attributes": { "ticketId": "t-4" } }) ); } /// The projection is a whitelist: the catalog adds attributes without asking - /// the shell, and they must not reach the activation URI. + /// the shell, and they must not reach the activation URI. The bulk an + /// approval carries (its `toolCalls` JSON) is the case that matters — + /// Windows truncates the URI at ~2 KB, and the ids the action buttons act on + /// must survive it. #[test] fn unknown_attributes_stay_out_of_the_payload() { let envelope = serde_json::json!({ @@ -704,41 +678,16 @@ mod tests { "approvalRequestId": "req-1", "dialogId": "dlg-1", "somethingShippedLater": "y".repeat(4096), + "toolCalls": "x".repeat(4096), }, }); let payload = click_payload(&envelope).unwrap(); assert_eq!( payload, - serde_json::json!({ "context": { + serde_json::json!({ "type": "MINGO_APPROVAL_REQUEST", - "dialogId": "dlg-1", - "approvalRequestId": "req-1", - } }) - ); - assert!(click_uri(Some(&payload)).len() < 2048); - } - - /// The bulk a context can carry (an approval request's toolCalls) must not - /// reach the activation URI — Windows truncates it at ~2 KB. The ids the - /// macOS action buttons act on must survive it. - #[test] - fn click_payload_keeps_only_routing_fields() { - let envelope = serde_json::json!({ - "context": { - "type": "ADMIN_APPROVAL_REQUEST", - "ticketId": "abc", - "approvalRequestId": "0a2a0b3c-9d1e-4f5a-8b7c-6d5e4f3a2b1c", - "toolCalls": [{ "toolExplanation": "x".repeat(4096) }], - } - }); - let payload = click_payload(&envelope).unwrap(); - assert_eq!( - payload, - serde_json::json!({ "context": { - "type": "ADMIN_APPROVAL_REQUEST", - "ticketId": "abc", - "approvalRequestId": "0a2a0b3c-9d1e-4f5a-8b7c-6d5e4f3a2b1c", - } }) + "attributes": { "dialogId": "dlg-1", "approvalRequestId": "req-1" } + }) ); assert!(click_uri(Some(&payload)).len() < 2048); } @@ -773,9 +722,10 @@ mod tests { #[test] fn malformed_uris_have_no_payload() { assert!(parse_click_uri(CLICK_URI_PREFIX).is_none()); - assert!(parse_click_uri("openframe-console://notify?context=%7B%7D").is_some()); - assert!(parse_click_uri("openframe-console://notify?context=not-json").is_none()); + assert!(parse_click_uri("openframe-console://notify?payload=%7B%7D").is_some()); + assert!(parse_click_uri("openframe-console://notify?payload=not-json").is_none()); + assert!(parse_click_uri("openframe-console://notify?payload=%5B%5D").is_none()); assert!(parse_click_uri("openframe-console://notify?id=x").is_none()); - assert!(parse_click_uri("openframe-chat://notify?context=%7B%7D").is_none()); + assert!(parse_click_uri("openframe-chat://notify?payload=%7B%7D").is_none()); } } diff --git a/src-tauri/src/windows_toast.rs b/src-tauri/src/windows_toast.rs index 5a80ef2..b037136 100644 --- a/src-tauri/src/windows_toast.rs +++ b/src-tauri/src/windows_toast.rs @@ -339,8 +339,8 @@ fn encode_action_args( if let Some(user_id) = user_id { args.push_str(&format!("&user={}", encode(user_id))); } - if let Some(context) = click.and_then(|click| click.get("context")) { - args.push_str(&format!("&context={}", encode(&context.to_string()))); + if let Some(click) = click { + args.push_str(&format!("&payload={}", encode(&click.to_string()))); } args } @@ -371,9 +371,7 @@ pub(crate) fn parse_action_args(args: &str) -> Option { "action" => parsed.action = value, "title" => parsed.context.title = value, "user" => parsed.context.user_id = Some(value).filter(|user| !user.is_empty()), - "context" => { - parsed.context.payload = crate::notifications::payload_from_context_json(&value) - } + "payload" => parsed.context.payload = crate::notifications::payload_from_json(&value), _ => {} } } @@ -438,10 +436,10 @@ mod tests { use super::*; fn approval_click() -> serde_json::Value { - serde_json::json!({ "context": { - "type": "ADMIN_APPROVAL_REQUEST", - "approvalRequestId": "0a2a0b3c-9d1e-4f5a-8b7c-6d5e4f3a2b1c", - } }) + serde_json::json!({ + "type": "MINGO_APPROVAL_REQUEST", + "attributes": { "approvalRequestId": "0a2a0b3c-9d1e-4f5a-8b7c-6d5e4f3a2b1c" }, + }) } #[test] @@ -460,9 +458,10 @@ mod tests { /// the right request. #[test] fn action_args_survive_reserved_chars() { - let click = serde_json::json!({ "context": { - "type": "ADMIN_AI_MESSAGE", "dialogId": "abc/д ф&x=1", - } }); + let click = serde_json::json!({ + "type": "ADMIN_AI_MESSAGE", + "attributes": { "dialogId": "abc/д ф&x=1" }, + }); let args = encode_action_args(REPLY_ACTION, "a&b=c д", Some("u&1"), Some(&click)); let parsed = parse_action_args(&args).unwrap(); assert_eq!(parsed.context.title, "a&b=c д");