diff --git a/desktop/src-tauri/src/commands/channel_templates.rs b/desktop/src-tauri/src/commands/channel_templates.rs index 2f00fbad42..fdcf69c843 100644 --- a/desktop/src-tauri/src/commands/channel_templates.rs +++ b/desktop/src-tauri/src/commands/channel_templates.rs @@ -72,6 +72,11 @@ fn validate_visibility(value: &str) -> Result<(), String> { } #[tauri::command] +/// Open a native multi-folder picker for channel-template project folders. +/// +/// Returns an empty list when the user cancels. Returns an error if the dialog +/// closes unexpectedly or any selected folder cannot be represented as a local +/// filesystem path. pub async fn pick_channel_template_project_folder(app: AppHandle) -> Result, String> { use tauri_plugin_dialog::DialogExt; diff --git a/desktop/src-tauri/src/commands/team_snapshot.rs b/desktop/src-tauri/src/commands/team_snapshot.rs index b5bc3a7e05..41c7f17632 100644 --- a/desktop/src-tauri/src/commands/team_snapshot.rs +++ b/desktop/src-tauri/src/commands/team_snapshot.rs @@ -160,10 +160,8 @@ pub(crate) fn build_import_team( persona_ids: Vec, now: &str, ) -> Result { - let name = snapshot.team.name.trim(); - if name.is_empty() { - return Err("Team snapshot name is empty.".to_string()); - } + let name = crate::managed_agents::validate_team_name(&snapshot.team.name) + .map_err(|error| format!("Invalid team snapshot: {error}"))?; Ok(TeamRecord { id: Uuid::new_v4().to_string(), diff --git a/desktop/src-tauri/src/commands/team_snapshot/tests.rs b/desktop/src-tauri/src/commands/team_snapshot/tests.rs index c9a6d8812a..c6ae14427c 100644 --- a/desktop/src-tauri/src/commands/team_snapshot/tests.rs +++ b/desktop/src-tauri/src/commands/team_snapshot/tests.rs @@ -329,6 +329,17 @@ fn team_import_definitions_are_built_for_all_members() { assert_eq!(definitions[0].system_prompt, "Alice prompt"); } +#[test] +fn team_import_rejects_names_that_cannot_be_mentioned() { + for invalid_name in ["x".repeat(201), "Review\nTeam".to_string()] { + let mut source = snapshot(vec![member("Alice")]); + source.team.name = invalid_name; + + let error = build_import_team(&source, vec!["alice".to_string()], "now").unwrap_err(); + assert!(error.starts_with("Invalid team snapshot: Team name")); + } +} + #[test] fn team_import_keeps_or_clears_every_member_allowlist_with_one_toggle() { let source = snapshot(vec![member("Alice"), member("Bob")]); diff --git a/desktop/src-tauri/src/commands/teams.rs b/desktop/src-tauri/src/commands/teams.rs index 4377ddaa43..7fd68f62e8 100644 --- a/desktop/src-tauri/src/commands/teams.rs +++ b/desktop/src-tauri/src/commands/teams.rs @@ -5,19 +5,12 @@ use crate::{ app_state::AppState, managed_agents::{ delete_team_with_cascade, ensure_persona_ids_are_active, load_personas, load_teams, - save_teams, try_regenerate_nest, CreateTeamRequest, TeamRecord, UpdateTeamRequest, + save_teams, try_regenerate_nest, validate_team_name, CreateTeamRequest, TeamRecord, + UpdateTeamRequest, }, util::now_iso, }; -fn trim_required(value: &str, label: &str) -> Result { - let trimmed = value.trim(); - if trimmed.is_empty() { - return Err(format!("{label} is required")); - } - Ok(trimmed.to_string()) -} - fn trim_optional(value: Option) -> Option { value.and_then(|candidate| { let trimmed = candidate.trim(); @@ -145,7 +138,7 @@ pub async fn create_team(input: CreateTeamRequest, app: AppHandle) -> Result(); - let name = trim_required(&input.name, "Team name")?; + let name = validate_team_name(&input.name)?; let description = trim_optional(input.description); let instructions = trim_optional(input.instructions); let now = now_iso(); @@ -186,7 +179,7 @@ pub async fn update_team(input: UpdateTeamRequest, app: AppHandle) -> Result(); - let name = trim_required(&input.name, "Team name")?; + let name = validate_team_name(&input.name)?; let description = trim_optional(input.description); let instructions = trim_optional(input.instructions); diff --git a/desktop/src-tauri/src/events/mention_reference_tags.rs b/desktop/src-tauri/src/events/mention_reference_tags.rs index 9d77ffa6a2..a638b108e7 100644 --- a/desktop/src-tauri/src/events/mention_reference_tags.rs +++ b/desktop/src-tauri/src/events/mention_reference_tags.rs @@ -1,6 +1,7 @@ +use crate::managed_agents::MAX_TEAM_NAME_CHARS; use nostr::Tag; -const MAX_TEAM_MENTION_FIELD_CHARS: usize = 200; +const MAX_TEAM_MENTION_ID_CHARS: usize = 200; pub(super) fn parse_mention_reference_tag(mention: &[String]) -> Result { match mention.first().map(String::as_str) { @@ -40,8 +41,8 @@ fn parse_team_mention(mention: &[String]) -> Result { if team_id.is_empty() || display_name.is_empty() { return Err("team mention id and display name must not be blank".into()); } - if team_id.chars().count() > MAX_TEAM_MENTION_FIELD_CHARS - || display_name.chars().count() > MAX_TEAM_MENTION_FIELD_CHARS + if team_id.chars().count() > MAX_TEAM_MENTION_ID_CHARS + || display_name.chars().count() > MAX_TEAM_NAME_CHARS { return Err("team mention id and display name must be at most 200 characters".into()); } diff --git a/desktop/src-tauri/src/managed_agents/team_snapshot.rs b/desktop/src-tauri/src/managed_agents/team_snapshot.rs index 96082acc76..febc1795d6 100644 --- a/desktop/src-tauri/src/managed_agents/team_snapshot.rs +++ b/desktop/src-tauri/src/managed_agents/team_snapshot.rs @@ -206,9 +206,8 @@ pub(crate) fn validate_team_snapshot(snapshot: &TeamSnapshot) -> Result<(), Stri snapshot.version )); } - if snapshot.team.name.trim().is_empty() { - return Err("Team snapshot team.name is empty".to_string()); - } + crate::managed_agents::validate_team_name(&snapshot.team.name) + .map_err(|error| format!("Invalid team snapshot: {error}"))?; if snapshot.members.is_empty() { return Err("Team snapshot must have at least one member".to_string()); } @@ -423,7 +422,18 @@ mod tests { let bytes = serde_json::to_vec(&snapshot).unwrap(); let result = decode_team_snapshot_json(&bytes); assert!(result.is_err()); - assert!(result.unwrap_err().contains("team.name is empty")); + assert!(result.unwrap_err().contains("Team name is required")); + } + + #[test] + fn validate_rejects_team_name_that_cannot_be_mentioned() { + for invalid_name in ["x".repeat(201), "Review\nTeam".to_string()] { + let mut snapshot = two_member_team(); + snapshot.team.name = invalid_name; + let bytes = serde_json::to_vec(&snapshot).unwrap(); + let error = decode_team_snapshot_json(&bytes).unwrap_err(); + assert!(error.starts_with("Invalid team snapshot: Team name")); + } } #[test] diff --git a/desktop/src-tauri/src/managed_agents/teams.rs b/desktop/src-tauri/src/managed_agents/teams.rs index 937893d531..326ef900db 100644 --- a/desktop/src-tauri/src/managed_agents/teams.rs +++ b/desktop/src-tauri/src/managed_agents/teams.rs @@ -9,6 +9,27 @@ use crate::{ use super::team_repair::team_persona_key; +/// Maximum team-name length accepted by the `team_mention` event tag. +pub const MAX_TEAM_NAME_CHARS: usize = 200; + +/// Normalize and validate a team name before it can reach storage or event +/// construction. +pub fn validate_team_name(value: &str) -> Result { + let name = value.trim(); + if name.is_empty() { + return Err("Team name is required".to_string()); + } + if name.chars().count() > MAX_TEAM_NAME_CHARS { + return Err(format!( + "Team name must be at most {MAX_TEAM_NAME_CHARS} characters" + )); + } + if name.chars().any(char::is_control) { + return Err("Team name must not contain control characters".to_string()); + } + Ok(name.to_string()) +} + pub(crate) fn teams_store_path(app: &AppHandle) -> Result { Ok(managed_agents_base_dir(app)?.join("teams.json")) } diff --git a/desktop/src-tauri/src/managed_agents/teams_tests.rs b/desktop/src-tauri/src/managed_agents/teams_tests.rs index 1ffa60eda9..0133f4cbc0 100644 --- a/desktop/src-tauri/src/managed_agents/teams_tests.rs +++ b/desktop/src-tauri/src/managed_agents/teams_tests.rs @@ -5,7 +5,7 @@ use super::{ agents_referencing_team, load_teams_readonly, merge_teams, merge_teams_impl, sort_teams, - validate_team_deletion, BuiltInTeam, + validate_team_deletion, validate_team_name, BuiltInTeam, MAX_TEAM_NAME_CHARS, }; use crate::managed_agents::{ManagedAgentRecord, TeamRecord}; @@ -26,6 +26,31 @@ fn team(id: &str, name: &str) -> TeamRecord { } } +#[test] +fn validate_team_name_normalizes_valid_names() { + assert_eq!( + validate_team_name(" Review Team ").unwrap(), + "Review Team" + ); + assert!(validate_team_name(&"é".repeat(MAX_TEAM_NAME_CHARS)).is_ok()); +} + +#[test] +fn validate_team_name_rejects_unmentionable_names() { + assert_eq!( + validate_team_name(" \t ").unwrap_err(), + "Team name is required" + ); + assert_eq!( + validate_team_name(&"x".repeat(MAX_TEAM_NAME_CHARS + 1)).unwrap_err(), + "Team name must be at most 200 characters" + ); + assert_eq!( + validate_team_name("Review\nTeam").unwrap_err(), + "Team name must not contain control characters" + ); +} + #[test] fn sort_teams_alphabetical_case_insensitive() { let mut teams = vec![team("3", "Zulu"), team("1", "alpha"), team("2", "Bravo")]; diff --git a/desktop/src-tauri/src/templates/types.rs b/desktop/src-tauri/src/templates/types.rs index af4c233a1f..7194c6bfc4 100644 --- a/desktop/src-tauri/src/templates/types.rs +++ b/desktop/src-tauri/src/templates/types.rs @@ -38,8 +38,13 @@ pub struct TemplateAgentRoster { #[derive(Debug, Clone, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] +/// Workspace settings applied when creating a worktree from a channel template. +/// `location` is the parent directory for new worktrees; `base_branch` is the +/// branch fetched from origin and used as each worktree's starting point. pub struct TemplateWorktreeConfig { + /// Parent directory where new worktrees are created. pub location: String, + /// Branch used as the base for each new worktree. pub base_branch: String, } diff --git a/desktop/src/features/agents/ui/TeamDialog.tsx b/desktop/src/features/agents/ui/TeamDialog.tsx index 3b9d07a346..870ea5ff5f 100644 --- a/desktop/src/features/agents/ui/TeamDialog.tsx +++ b/desktop/src/features/agents/ui/TeamDialog.tsx @@ -144,7 +144,11 @@ export function TeamDialog({ ...baseInput, }; } - return baseInput; + return { + ...baseInput, + description: teamDescription.trim() || undefined, + instructions: instructions.trim() || undefined, + }; } async function handleSubmit() { diff --git a/desktop/src/features/messages/ui/useMentionSendFlow.helpers.test.mjs b/desktop/src/features/messages/ui/useMentionSendFlow.helpers.test.mjs new file mode 100644 index 0000000000..f92f6b14ff --- /dev/null +++ b/desktop/src/features/messages/ui/useMentionSendFlow.helpers.test.mjs @@ -0,0 +1,28 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { + exceedsMentionPubkeyLimit, + MAX_MENTION_PUBKEYS, +} from "./useMentionSendFlow.helpers.ts"; + +test("mention preflight accepts the maximum number of unique targets", () => { + const pubkeys = Array.from({ length: MAX_MENTION_PUBKEYS }, (_, index) => + index.toString(16).padStart(64, "0"), + ); + + assert.equal(exceedsMentionPubkeyLimit(pubkeys, []), false); + assert.equal(exceedsMentionPubkeyLimit([...pubkeys, pubkeys[0]], []), false); +}); + +test("mention preflight counts unresolved persona targets before provisioning", () => { + const pubkeys = Array.from({ length: MAX_MENTION_PUBKEYS - 1 }, (_, index) => + index.toString(16).padStart(64, "0"), + ); + + assert.equal(exceedsMentionPubkeyLimit(pubkeys, ["persona-1"]), false); + assert.equal( + exceedsMentionPubkeyLimit(pubkeys, ["persona-1", "persona-2"]), + true, + ); +}); diff --git a/desktop/src/features/messages/ui/useMentionSendFlow.helpers.ts b/desktop/src/features/messages/ui/useMentionSendFlow.helpers.ts index e148131a3e..96a97bec05 100644 --- a/desktop/src/features/messages/ui/useMentionSendFlow.helpers.ts +++ b/desktop/src/features/messages/ui/useMentionSendFlow.helpers.ts @@ -81,6 +81,21 @@ export function uniqueNormalizedPubkeys(pubkeys: Iterable) { return [...new Set([...pubkeys].map(normalizePubkey))].filter(Boolean); } +export const MAX_MENTION_PUBKEYS = 50; + +export function exceedsMentionPubkeyLimit( + resolvedPubkeys: Iterable, + unresolvedPersonaIds: Iterable, +) { + const targetPubkeys = new Set( + [...resolvedPubkeys].map(normalizePubkey).filter(Boolean), + ); + for (const personaId of unresolvedPersonaIds) { + if (personaId.trim()) targetPubkeys.add(`persona:${personaId}`); + } + return targetPubkeys.size > MAX_MENTION_PUBKEYS; +} + export function isManagedAgentRunning(agent: ManagedAgent) { return agent.status === "running" || agent.status === "deployed"; } diff --git a/desktop/src/features/messages/ui/useMentionSendFlow.ts b/desktop/src/features/messages/ui/useMentionSendFlow.ts index 78c9ded31a..922a8796a7 100644 --- a/desktop/src/features/messages/ui/useMentionSendFlow.ts +++ b/desktop/src/features/messages/ui/useMentionSendFlow.ts @@ -31,6 +31,7 @@ import type { AcpRuntime, ManagedAgent } from "@/shared/api/types"; import { normalizePubkey, truncatePubkey } from "@/shared/lib/pubkey"; import { buildMentionTags, + exceedsMentionPubkeyLimit, getErrorMessage, isManagedAgentRunning, isProviderBackedAgent, @@ -683,6 +684,23 @@ export function useMentionSendFlow({ return; } + const preflightMentionPubkeys = mentions.extractMentionPubkeys(trimmed); + const preflightPersonaIds = mentions + .extractMentionPersonas(trimmed) + .map(({ persona }) => persona.id); + if ( + exceedsMentionPubkeyLimit( + preflightMentionPubkeys, + preflightPersonaIds, + ) + ) { + const message = + "Too many mentions. Messages can mention at most 50 people or agents."; + setNonMemberPromptError(message); + toast.error(message); + return; + } + let effectiveChannelId = capturedChannelId; if (!effectiveChannelId && onPrepareSendChannel) { effectiveChannelId = await onPrepareSendChannel(); @@ -798,6 +816,7 @@ export function useMentionSendFlow({ mentions.isManagedAgentPubkey, mentions.getDraftMentionRefs, onPrepareSendChannel, + mentions.extractMentionPersonas, ], ); diff --git a/desktop/src/features/settings/ui/ChannelTemplatesSettingsCard.tsx b/desktop/src/features/settings/ui/ChannelTemplatesSettingsCard.tsx index 1058aaac64..ac801d1cb4 100644 --- a/desktop/src/features/settings/ui/ChannelTemplatesSettingsCard.tsx +++ b/desktop/src/features/settings/ui/ChannelTemplatesSettingsCard.tsx @@ -496,6 +496,12 @@ export function TemplateFormDialog({ const input: UpdateChannelTemplateInput = { id: template.id, name: trimmedName, + // The current editor intentionally does not expose channel type or + // visibility, so preserve the stored values when updating unrelated + // template fields rather than letting the backend's defaults overwrite + // them. + channelType: template.channelType, + visibility: template.visibility, description: template.description ?? undefined, canvasTemplate: canvasTemplate.trim() || undefined, projectFolders, diff --git a/desktop/src/features/sidebar/lib/useCreateChannelForm.test.mjs b/desktop/src/features/sidebar/lib/useCreateChannelForm.test.mjs new file mode 100644 index 0000000000..825b021464 --- /dev/null +++ b/desktop/src/features/sidebar/lib/useCreateChannelForm.test.mjs @@ -0,0 +1,42 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { resolveInitialChannelTemplate } from "./useCreateChannelForm.ts"; + +const template = { + id: "review-template", + name: "Review", + description: "Review changes", + channelType: "stream", + visibility: "private", + canvasTemplate: null, + projectFolders: [], + projectFolder: null, + worktree: null, + agents: { personas: [], teams: [] }, + isBuiltin: false, + createdAt: "now", + updatedAt: "now", +}; + +test("keeps a section template locked while the template query is pending", () => { + assert.equal( + resolveInitialChannelTemplate(template.id, [], "pending"), + undefined, + ); +}); + +test("applies a section template after a successful query", () => { + assert.equal( + resolveInitialChannelTemplate(template.id, [template], "success"), + template, + ); +}); + +test("clears a missing section template after a successful query", () => { + assert.equal(resolveInitialChannelTemplate(template.id, [], "success"), null); +}); + +test("clears a locked section template after a terminal query failure", () => { + assert.equal(resolveInitialChannelTemplate(template.id, [], "error"), null); +}); diff --git a/desktop/src/features/sidebar/lib/useCreateChannelForm.ts b/desktop/src/features/sidebar/lib/useCreateChannelForm.ts index ace43dcbf9..296bbe9223 100644 --- a/desktop/src/features/sidebar/lib/useCreateChannelForm.ts +++ b/desktop/src/features/sidebar/lib/useCreateChannelForm.ts @@ -4,6 +4,26 @@ import { useChannelTemplatesQuery } from "@/features/channel-templates/hooks"; import { DEFAULT_EPHEMERAL_TTL_SECONDS } from "@/features/channels/lib/ephemeralChannel"; import type { ChannelTemplate, ChannelVisibility } from "@/shared/api/types"; +export type TemplateQueryStatus = "pending" | "success" | "error"; + +/** + * Resolve a section's saved template against the template query. + * + * `undefined` means the query is still pending, `null` means the saved + * template must be cleared, and a template means the default can be applied. + */ +export function resolveInitialChannelTemplate( + initialTemplateId: string | undefined, + templates: ChannelTemplate[], + status: TemplateQueryStatus, +): ChannelTemplate | null | undefined { + if (!initialTemplateId || status === "pending") return undefined; + if (status === "error") return null; + return ( + templates.find((template) => template.id === initialTemplateId) ?? null + ); +} + export type CreateChannelKind = "stream" | "forum"; export type CreateChannelInput = { @@ -82,6 +102,8 @@ export function useCreateChannelForm({ const [selectedTemplateId, setSelectedTemplateId] = React.useState< string | null >(initialTemplateId ?? null); + const [sectionDefaultTemplateId, setSectionDefaultTemplateId] = + React.useState(initialTemplateId ?? null); const [typePopoverOpen, setTypePopoverOpen] = React.useState(false); const nameInputRef = React.useRef(null); const visibilityTouchedRef = React.useRef(false); @@ -101,6 +123,7 @@ export function useCreateChannelForm({ setTtlSeconds(DEFAULT_EPHEMERAL_TTL_SECONDS); setErrorMessage(null); setSelectedTemplateId(initialTemplateId ?? null); + setSectionDefaultTemplateId(initialTemplateId ?? null); setTypePopoverOpen(false); visibilityTouchedRef.current = false; initialTemplateAppliedRef.current = null; @@ -134,20 +157,42 @@ export function useCreateChannelForm({ }, []); React.useEffect(() => { - if ( - !active || - !initialTemplateId || - initialTemplateAppliedRef.current === initialTemplateId - ) { + if (!active || !initialTemplateId) return; + + const queryStatus: TemplateQueryStatus = templatesQuery.isError + ? "error" + : templatesQuery.isSuccess + ? "success" + : "pending"; + const template = resolveInitialChannelTemplate( + initialTemplateId, + templates, + queryStatus, + ); + if (template === undefined) return; + if (template === null) { + // Missing templates and terminal query failures both unlock the form. + // Keeping the raw section ID selected would promise defaults that cannot + // be applied after channel creation. + initialTemplateAppliedRef.current = initialTemplateId; + setSelectedTemplateId(null); + setSectionDefaultTemplateId(null); + setDescription(""); + if (!visibilityTouchedRef.current) setVisibility("open"); + setErrorMessage(null); return; } - const template = templates.find( - (candidate) => candidate.id === initialTemplateId, - ); - if (!template) return; + if (initialTemplateAppliedRef.current === initialTemplateId) return; initialTemplateAppliedRef.current = initialTemplateId; applyTemplate(template); - }, [active, applyTemplate, initialTemplateId, templates]); + }, [ + active, + applyTemplate, + initialTemplateId, + templates, + templatesQuery.isError, + templatesQuery.isSuccess, + ]); const handleTemplateCreated = React.useCallback( (template: ChannelTemplate) => { @@ -246,7 +291,7 @@ export function useCreateChannelForm({ handleTemplateChange, handleTemplateCreated, templates, - sectionDefaultTemplateId: initialTemplateId ?? null, + sectionDefaultTemplateId, nameInputRef, isCreating, canSubmit: name.trim().length > 0 && !isCreating, diff --git a/desktop/src/shared/api/channelTemplateTypes.ts b/desktop/src/shared/api/channelTemplateTypes.ts index e78ca04338..e5ccad36c3 100644 --- a/desktop/src/shared/api/channelTemplateTypes.ts +++ b/desktop/src/shared/api/channelTemplateTypes.ts @@ -17,7 +17,12 @@ export type TemplateTeamEntry = { backend: TemplateBackend | null; }; -export type TemplateWorktreeConfig = { location: string; baseBranch: string }; +export type TemplateWorktreeConfig = { + /** Directory where new worktrees are created. */ + location: string; + /** Branch used as the base for each new worktree. */ + baseBranch: string; +}; export type TemplateAgents = { personas: TemplateAgentEntry[]; teams: TemplateTeamEntry[];