From 2c66a3d5aaafa303faaacc29e070a0c6505ef027 Mon Sep 17 00:00:00 2001 From: zhom <2717306+zhom@users.noreply.github.com> Date: Tue, 3 Nov 2026 21:45:19 +0400 Subject: [PATCH] feat: feedback profile picker --- e2e/tests/ui.test.mjs | 36 ++++++++++++ src-tauri/src/feedback.rs | 82 ++++++++++++++++++++++++++++ src-tauri/src/mcp_server.rs | 19 ++++++- src/app/page.tsx | 1 + src/components/agent-tasks/parts.tsx | 30 +++++++++- src/components/feedback-dialog.tsx | 45 +++++++++++++++ src/i18n/locales/de.json | 4 ++ src/i18n/locales/en.json | 4 ++ src/i18n/locales/es.json | 4 ++ src/i18n/locales/fr.json | 4 ++ src/i18n/locales/ja.json | 4 ++ src/i18n/locales/ko.json | 4 ++ src/i18n/locales/pt.json | 4 ++ src/i18n/locales/ru.json | 4 ++ src/i18n/locales/tr.json | 4 ++ src/i18n/locales/vi.json | 4 ++ src/i18n/locales/zh.json | 4 ++ 17 files changed, 254 insertions(+), 3 deletions(-) diff --git a/e2e/tests/ui.test.mjs b/e2e/tests/ui.test.mjs index 8a415386..474c945c 100644 --- a/e2e/tests/ui.test.mjs +++ b/e2e/tests/ui.test.mjs @@ -7365,6 +7365,17 @@ test("feedback goes from the More menu to the team, with the logs the person pre }), /FEEDBACK_EMAIL_INVALID/, ); + assert.match( + await app.invokeError("send_feedback", { + kind: "bug", + message: "Hangs", + includeLogs: false, + email: null, + locale: "en", + profileId: "00000000-0000-4000-8000-000000000000", + }), + /PROFILE_NOT_FOUND/, + ); assert.equal(capture.received.length, 0); await app.waitFor( async () => @@ -7459,6 +7470,24 @@ test("feedback goes from the More menu to the team, with the logs the person pre '[data-slot="feedback-email"]', "me@example.com", ); + // The report may name one profile; only its id leaves the app. + assert.equal( + await app.execute( + `return document.querySelector('#feedback-profile')?.innerText.trim();`, + ), + en.agent.composer.noProfile, + ); + await app.clickSelector("#feedback-profile"); + await app.clickSelector( + `[data-slot="agent-profile-option"][data-profile-id="${profile.id}"]`, + ); + await app.waitFor( + async () => + (await app.execute( + `return document.querySelector('#feedback-profile')?.innerText.trim();`, + )) === "Secret Shop Account", + { description: "the chosen profile on the picker" }, + ); await app.capture("feedback-compose"); await app.clickSelector('[data-slot="feedback-send"]'); await app.waitFor( @@ -7492,6 +7521,8 @@ test("feedback goes from the More menu to the team, with the logs the person pre assert.equal(sent.body.locale, "en"); assert.equal(typeof sent.body.appVersion, "string"); assert.ok(sent.body.logs.includes("Donut started")); + assert.equal(sent.body.profileId, profile.id); + assert.ok(!JSON.stringify(sent.body).includes("Secret Shop Account")); await app.clickSelector('[data-slot="feedback-sent"] button'); await app.waitFor( @@ -7528,6 +7559,11 @@ test("feedback goes from the More menu to the team, with the logs the person pre assert.equal(await feedbackMessage(app), "Dark tray icon"); assert.equal(capture.received.length, 2); assert.equal(capture.received[1].body.kind, "idea"); + assert.equal( + capture.received[1].body.profileId, + undefined, + "a sent draft does not keep its profile", + ); assert.equal( capture.received[1].body.logs, undefined, diff --git a/src-tauri/src/feedback.rs b/src-tauri/src/feedback.rs index 98298e91..d60fd296 100644 --- a/src-tauri/src/feedback.rs +++ b/src-tauri/src/feedback.rs @@ -77,6 +77,8 @@ pub struct Feedback { pub logs: Option, pub agent_client: Option, pub context: Option, + /// The profile the report is about. Only its id leaves the app. + pub profile_id: Option, } #[derive(Debug, Serialize)] @@ -98,6 +100,8 @@ struct Payload<'a> { agent_client: Option<&'a str>, #[serde(skip_serializing_if = "Option::is_none")] context: Option<&'a serde_json::Value>, + #[serde(skip_serializing_if = "Option::is_none")] + profile_id: Option, } impl<'a> Payload<'a> { @@ -118,6 +122,7 @@ impl<'a> Payload<'a> { locale: feedback.locale.as_deref(), agent_client: feedback.agent_client.as_deref(), context: feedback.context.as_ref(), + profile_id: feedback.profile_id, } } } @@ -172,6 +177,32 @@ pub fn validate_email(email: Option<&str>) -> Result, String> { } } +/// The profile a report is about: blank is none, anything else must be the id +/// of an existing profile. +pub fn validate_profile_id( + profile_id: Option<&str>, + exists: impl FnOnce(&uuid::Uuid) -> Result, +) -> Result, String> { + let Some(profile_id) = profile_id.map(str::trim).filter(|id| !id.is_empty()) else { + return Ok(None); + }; + let not_found = || crate::backend_error("PROFILE_NOT_FOUND"); + let profile_id = uuid::Uuid::parse_str(profile_id).map_err(|_| not_found())?; + if exists(&profile_id)? { + Ok(Some(profile_id)) + } else { + Err(not_found()) + } +} + +/// Whether a profile with this id is on this device. +pub fn profile_exists(profile_id: &uuid::Uuid) -> Result { + crate::profile::ProfileManager::instance() + .lookup_profile(profile_id) + .map(|profile| profile.is_some()) + .map_err(|e| crate::backend_error_with_detail("INTERNAL_ERROR", e)) +} + /// The log excerpt feedback carries: anonymous facts, then the newest app and /// proxy worker log lines, redacted, at most `MAX_LOG_BYTES` /// (`diagnostics::excerpt`). @@ -342,10 +373,12 @@ pub async fn send_feedback( include_logs: bool, email: Option, locale: Option, + profile_id: Option, ) -> Result { let kind = FeedbackKind::parse(&kind).ok_or_else(|| crate::backend_error("FEEDBACK_INVALID"))?; let message = validate_message(&message)?; let email = validate_email(email.as_deref())?; + let profile_id = validate_profile_id(profile_id.as_deref(), profile_exists)?; let logs = if include_logs { Some( log_excerpt( @@ -366,6 +399,7 @@ pub async fn send_feedback( logs, agent_client: None, context: None, + profile_id, }) .await?; tauri::async_runtime::spawn_blocking(crate::settings_manager::note_feedback_sent); @@ -443,6 +477,51 @@ mod tests { } } + #[test] + fn a_profile_is_optional_but_must_exist() { + let id = uuid::Uuid::new_v4(); + let never = |_: &uuid::Uuid| -> Result { panic!("not looked up") }; + assert_eq!(validate_profile_id(None, never).unwrap(), None); + assert_eq!(validate_profile_id(Some(" "), never).unwrap(), None); + assert_eq!( + validate_profile_id(Some(&format!(" {id} ")), |seen| Ok(*seen == id)).unwrap(), + Some(id) + ); + assert_eq!( + code_of(&validate_profile_id(Some("not-a-uuid"), never).unwrap_err()), + "PROFILE_NOT_FOUND" + ); + assert_eq!( + code_of(&validate_profile_id(Some(&id.to_string()), |_| Ok(false)).unwrap_err()), + "PROFILE_NOT_FOUND" + ); + let failed = validate_profile_id(Some(&id.to_string()), |_| { + Err(crate::backend_error("INTERNAL_ERROR")) + }); + assert_eq!(code_of(&failed.unwrap_err()), "INTERNAL_ERROR"); + } + + #[test] + fn the_payload_names_a_profile_by_id_only() { + let id = uuid::Uuid::new_v4(); + let mut feedback = Feedback { + kind: FeedbackKind::Bug, + source: FeedbackSource::Person, + message: "the proxy check hangs".to_string(), + email: None, + locale: None, + logs: None, + agent_client: None, + context: None, + profile_id: Some(id), + }; + let sent = serde_json::to_value(Payload::of(&feedback)).unwrap(); + assert_eq!(sent["profileId"], id.to_string()); + feedback.profile_id = None; + let sent = serde_json::to_value(Payload::of(&feedback)).unwrap(); + assert!(sent.get("profileId").is_none(), "{sent}"); + } + #[test] fn an_agent_may_send_a_bounded_amount_per_hour() { let mut sends = VecDeque::new(); @@ -510,6 +589,7 @@ mod tests { logs: Some("line\n".to_string()), agent_client: None, context: None, + profile_id: None, }; let body = serde_json::to_vec(&Payload::of(&feedback)).unwrap(); let receipt = post( @@ -544,6 +624,7 @@ mod tests { logs: None, agent_client: Some("Claude Code 2.1.0".to_string()), context: Some(serde_json::json!({ "recentCalls": [] })), + profile_id: None, }; let sent = serde_json::to_value(Payload::of(&feedback)).unwrap(); assert_eq!(sent["source"], "agent"); @@ -569,6 +650,7 @@ mod tests { logs: Some(String::new()), agent_client: None, context: None, + profile_id: None, }; let body = serde_json::to_vec(&Payload::of(&feedback)).unwrap(); let error = post(format!("{}/api/feedback", server.uri()), body, None) diff --git a/src-tauri/src/mcp_server.rs b/src-tauri/src/mcp_server.rs index a983e013..393d02d6 100644 --- a/src-tauri/src/mcp_server.rs +++ b/src-tauri/src/mcp_server.rs @@ -6998,7 +6998,8 @@ impl McpServer { "enum": ["bug", "idea", "praise", "other"], "description": "bug for something broken, idea for something missing, praise for something that worked well (default: other)" }, - "include_logs": { "type": "boolean", "description": "Attach the recent app logs (default: false)" } + "include_logs": { "type": "boolean", "description": "Attach the recent app logs (default: false)" }, + "profile_id": { "type": "string", "description": "The id of the profile the feedback is about, if any. Only the id is sent, never the profile's name or data" } }, "required": ["message"] }), @@ -12814,6 +12815,16 @@ impl McpServer { .get("include_logs") .and_then(serde_json::Value::as_bool) .unwrap_or(false); + let profile_id = match arguments.get("profile_id") { + None | Some(serde_json::Value::Null) => None, + Some(value) => { + let id = value + .as_str() + .ok_or_else(|| Self::invalid_argument("profile_id must be a string"))?; + crate::feedback::validate_profile_id(Some(id), crate::feedback::profile_exists) + .map_err(|_| Self::invalid_argument("profile_id is not the id of a profile"))? + } + }; if !crate::feedback::agent_send_allowed() { return Err(McpError { @@ -12857,6 +12868,7 @@ impl McpServer { agent_client, context: (!recent_calls.is_empty()) .then(|| serde_json::json!({ "recentCalls": recent_calls })), + profile_id, }) .await .map_err(|error| { @@ -17166,6 +17178,11 @@ mod tests { 5, serde_json::json!({ "message": "x".repeat(crate::feedback::MAX_MESSAGE_CHARS + 1) }), ), + ( + 6, + serde_json::json!({ "message": "ok", "profile_id": "not-a-uuid" }), + ), + (7, serde_json::json!({ "message": "ok", "profile_id": 7 })), ] { let refused = send(&session, id, arguments).await; assert_eq!( diff --git a/src/app/page.tsx b/src/app/page.tsx index fd12b4cb..5858f3f0 100644 --- a/src/app/page.tsx +++ b/src/app/page.tsx @@ -3357,6 +3357,7 @@ export default function Home() { setFeedbackDialogOpen(false); }} accountEmail={cloudUser?.email ?? null} + profiles={profiles} /> void; disabled?: boolean; single?: boolean; + noneLabel?: string; className?: string; }) { const { t } = useTranslation(); @@ -214,7 +219,8 @@ export function ProfilePicker({ const known = profiles.filter((profile) => chosen.has(profile.id)); const label = known.length === 0 - ? t(single ? "agent.chat.chooseProfile" : "agent.tasks.chooseProfiles") + ? (noneLabel ?? + t(single ? "agent.chat.chooseProfile" : "agent.tasks.chooseProfiles")) : known.length === 1 ? known[0].name : t("agent.tasks.profileCount", { count: known.length }); @@ -258,6 +264,26 @@ export function ProfilePicker({ {t("agent.tasks.noProfileMatch")} + {single && noneLabel && ( + { + onChange([]); + setOpen(false); + }} + className="text-muted-foreground" + > + + )} {!single && ( void; /** The signed-in account's email; replies go there. */ accountEmail: string | null; + /** The profiles the report may be about; only the chosen id is sent. */ + profiles: BrowserProfile[]; } /** @@ -88,6 +92,7 @@ export function FeedbackDialog({ isOpen, onClose, accountEmail, + profiles, }: FeedbackDialogProps) { const { t, i18n } = useTranslation(); const animate = useMotionAllowed(); @@ -97,6 +102,7 @@ export function FeedbackDialog({ const [includeLogs, setIncludeLogs] = useState(true); const [logsChosen, setLogsChosen] = useState(false); const [email, setEmail] = useState(""); + const [profileId, setProfileId] = useState(null); const [previewOpen, setPreviewOpen] = useState(false); const [logs, setLogs] = useState(null); const [logsLoading, setLogsLoading] = useState(false); @@ -173,11 +179,18 @@ export function FeedbackDialog({ setLogsChosen(false); setPreviewOpen(false); setLogs(null); + setProfileId(null); } setError(null); onClose(); }, [onClose, sent]); + useEffect(() => { + if (profileId && !profiles.some((profile) => profile.id === profileId)) { + setProfileId(null); + } + }, [profileId, profiles]); + const canSend = message.trim().length > 0 && !sending; const send = useCallback(async () => { @@ -191,6 +204,7 @@ export function FeedbackDialog({ includeLogs, email: accountEmail ? null : email.trim() || null, locale: i18n.language, + profileId, }); setSent({ logs: includeLogs, @@ -209,6 +223,7 @@ export function FeedbackDialog({ includeLogs, kind, message, + profileId, sending, t, ]); @@ -437,6 +452,36 @@ export function FeedbackDialog({ )} + {profiles.length > 0 && ( +
+ +
+ { + setProfileId(ids[0] ?? null); + }} + noneLabel={t("agent.composer.noProfile")} + className="shrink-0" + /> +

+ {t("feedback.profile.idOnly")} +

+
+
+ )} + {accountEmail ? (