Skip to content

Preserve legacy tag configurations #34

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Stardust0831 merged 2 commits into from
Jul 20, 2026
Merged

Preserve legacy tag configurations #34

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
30b89a2
fix: preserve legacy tag configurations
Stardust0831 Jul 20, 2026
5fb782e
style: apply Rust formatting
Stardust0831 Jul 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 70 additions & 9 deletions crates/mountmate-app/src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4180,13 +4180,20 @@ impl App {
let Some(id) = draft.editing_id.clone() else {
return Task::none();
};
let tags = match validated_connection_tags(&draft.tags, self.locale()) {
Ok(tags) => tags,
Err(error) => {
self.status = error;
return Task::none();
}
};
let existing_tags = self
.servers
.iter()
.find(|server| server.id == id)
.map(|server| server.tags.as_slice());
let tags =
match validated_connection_tags_for_existing(&draft.tags, existing_tags, self.locale())
{
Ok(tags) => tags,
Err(error) => {
self.status = error;
return Task::none();
}
};
let auto_mount_at_login =
draft.auto_mount_at_login && draft.connection_method != ConnectionMethod::Interactive;
self.editor_saving = true;
Expand Down Expand Up @@ -8344,19 +8351,54 @@ fn connection_preference_updates(
}

fn validated_connection_tags(tags: &[String], locale: Locale) -> Result<Vec<String>, String> {
validated_connection_tags_for_existing(tags, None, locale)
}
Comment on lines 8353 to +8355

@coderabbitai coderabbitai Bot Jul 20, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Apply existing-aware validation to batch no-op updates.

Message::BatchAddTag still calls this strict wrapper at Line 2048 even when Lines 2044-2046 leave a selected server’s tags unchanged. A legacy-over-limit server that already has the chosen tag then rejects the entire batch, including valid changes for other selected servers. Use the existing-aware validator there (or omit no-op updates).

Proposed fix 
- let tags = validated_connection_tags(&tags, locale)?;
+ let tags = validated_connection_tags_for_existing(
+     &tags,
+     Some(server.tags.as_slice()),
+     locale,
+ )?;
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/mountmate-app/src/main.rs` around lines 8355 - 8357, Update the
Message::BatchAddTag handling to use existing-aware validation via
validated_connection_tags_for_existing when selected servers’ tags remain
unchanged, or skip those no-op updates. Preserve validation for actual tag
changes so legacy over-limit servers already containing the tag do not reject
valid updates for other servers.


fn validated_connection_tags_for_existing(
tags: &[String],
existing: Option<&[String]>,
locale: Locale,
) -> Result<Vec<String>, String> {
let mut normalized = Vec::new();
for tag in tags {
let tag = normalized_tag_name(tag, locale)?;
let tag = match normalized_tag_name(tag, locale) {
Ok(tag) => tag,
Err(error) => {
let tag = tag.trim();
let is_existing_overlong = tag.chars().count() > MAX_TAG_CHARS
&& !tag.chars().any(char::is_control)
&& !tag.contains(',')
&& !tag.contains('，')
&& existing.is_some_and(|existing| existing.iter().any(|item| item == tag));
if !is_existing_overlong {
return Err(error);
}
tag.to_owned()
}
};
if !normalized.iter().any(|candidate| candidate == &tag) {
normalized.push(tag);
}
}
if normalized.len() > MAX_CONNECTION_TAGS {
let preserves_existing = existing.is_some_and(|existing| {
mountmate_core::model::tag_update_only_preserves_existing(&normalized, existing)
});
if normalized.len() > MAX_CONNECTION_TAGS && !preserves_existing {
return Err(match locale {
Locale::English => format!("A connection may have at most {MAX_CONNECTION_TAGS} tags"),
Locale::Chinese => format!("一个连接最多只能有 {MAX_CONNECTION_TAGS} 个标签"),
});
}
if normalized
.iter()
.any(|tag| tag.chars().count() > MAX_TAG_CHARS)
&& !preserves_existing
{
return Err(match locale {
Locale::English => format!("A tag must be at most {MAX_TAG_CHARS} characters"),
Locale::Chinese => format!("标签最多只能有 {MAX_TAG_CHARS} 个字符"),
});
}
Ok(normalized)
}

Expand Down Expand Up @@ -11385,6 +11427,25 @@ mod localization_tests {
)
.is_err()
);

let legacy_long = "界".repeat(MAX_TAG_CHARS + 1);
assert_eq!(
validated_connection_tags_for_existing(
std::slice::from_ref(&legacy_long),
Some(std::slice::from_ref(&legacy_long)),
Locale::English,
)
.unwrap(),
vec![legacy_long.clone()]
);
assert!(
validated_connection_tags_for_existing(
&[legacy_long, "new-tag".into()],
Some(std::slice::from_ref(&"界".repeat(MAX_TAG_CHARS + 1))),
Locale::English,
)
.is_err()
);
}

#[test]
Expand Down
62 changes: 57 additions & 5 deletions crates/mountmate-core/src/connection.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use thiserror::Error;

use crate::model::{
MAX_CONNECTION_TAGS, MAX_TAG_CHARS, normalize_port, normalize_tags, sanitize_id,
tag_update_only_preserves_existing,
};
use crate::mountpoint::HOME_MOUNTPOINT_VALUE;
use crate::{AuthMethod, ConnectionMethod, ServerConfig};
Expand Down Expand Up @@ -306,7 +307,13 @@ impl ConnectionDraft {
pub fn validate(&self, servers: &[ServerConfig]) -> Result<ValidatedConnection, DraftError> {
let requirements = self.requirements();
let name = required_display_name(&self.name)?;
let tags = validate_tags(&self.tags, &self.folder)?;
let tags = validate_tags(
&self.tags,
&self.folder,
self.existing
.as_ref()
.map(|server| (server.tags.as_slice(), server.folder.as_str())),
)?;
let folder = tags.first().cloned().unwrap_or_default();
let host = required_scalar(&self.host, "IP/Host")?;
let user = required_scalar(&self.user, "User")?;
Expand Down Expand Up @@ -575,20 +582,33 @@ fn required_display_name(value: &str) -> Result<String, DraftError> {
Ok(value.into())
}

fn validate_tags(tags: &[String], legacy_folder: &str) -> Result<Vec<String>, DraftError> {
fn validate_tags(
tags: &[String],
legacy_folder: &str,
existing: Option<(&[String], &str)>,
) -> Result<Vec<String>, DraftError> {
if tags.iter().any(|tag| tag.chars().any(char::is_control))
|| legacy_folder.chars().any(char::is_control)
{
return Err(DraftError::InvalidFolder);
}
let mut normalized = tags.to_vec();
normalize_tags(&mut normalized, legacy_folder);
if normalized.len() > MAX_CONNECTION_TAGS {
let existing_normalized = existing.map(|(tags, folder)| {
let mut tags = tags.to_vec();
normalize_tags(&mut tags, folder);
tags
});
let preserves_existing = existing_normalized
.as_ref()
.is_some_and(|existing| tag_update_only_preserves_existing(&normalized, existing));
if normalized.len() > MAX_CONNECTION_TAGS && !preserves_existing {
return Err(DraftError::TooManyTags(MAX_CONNECTION_TAGS));
}
if normalized
.iter()
.any(|tag| tag.chars().count() > MAX_TAG_CHARS)
&& !preserves_existing
{
return Err(DraftError::TagTooLong(MAX_TAG_CHARS));
}
Expand Down Expand Up @@ -1232,16 +1252,48 @@ mod tests {
.map(|index| format!("tag-{index}"))
.collect::<Vec<_>>();
assert_eq!(
validate_tags(&too_many, ""),
validate_tags(&too_many, "", None),
Err(DraftError::TooManyTags(crate::model::MAX_CONNECTION_TAGS))
);
let too_long = vec!["界".repeat(crate::model::MAX_TAG_CHARS + 1)];
assert_eq!(
validate_tags(&too_long, ""),
validate_tags(&too_long, "", None),
Err(DraftError::TagTooLong(crate::model::MAX_TAG_CHARS))
);
}

#[test]
fn legacy_tag_limits_allow_unrelated_edits_and_progressive_cleanup() {
let mut existing = password_server();
existing.tags = (0..=crate::model::MAX_CONNECTION_TAGS)
.map(|index| format!("tag-{index}"))
.collect();
existing.folder = existing.tags[0].clone();

let unchanged = ConnectionDraft::from_server(&existing);
assert!(unchanged.validate(std::slice::from_ref(&existing)).is_ok());

let mut cleanup = ConnectionDraft::from_server(&existing);
cleanup.tags.pop();
assert!(cleanup.validate(std::slice::from_ref(&existing)).is_ok());

let mut addition = ConnectionDraft::from_server(&existing);
addition.tags.push("new-tag".into());
assert_eq!(
addition.validate(std::slice::from_ref(&existing)),
Err(DraftError::TooManyTags(crate::model::MAX_CONNECTION_TAGS))
);

let mut overlong = password_server();
overlong.tags = vec!["界".repeat(crate::model::MAX_TAG_CHARS + 1)];
overlong.folder = overlong.tags[0].clone();
assert!(
ConnectionDraft::from_server(&overlong)
.validate(std::slice::from_ref(&overlong))
.is_ok()
);
}

#[test]
fn refreshing_an_ssh_import_preserves_user_folder() {
let mut draft = ConnectionDraft {
Expand Down
4 changes: 4 additions & 0 deletions crates/mountmate-core/src/model.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ pub const MAX_VFS_UPLOAD_TRANSFERS: u16 = 32;
pub const MAX_CONNECTION_TAGS: usize = 8;
pub const MAX_TAG_CHARS: usize = 24;

pub fn tag_update_only_preserves_existing(candidate: &[String], existing: &[String]) -> bool {
candidate.iter().all(|tag| existing.contains(tag))
}

fn default_port() -> String {
"22".into()
}
Expand Down
65 changes: 60 additions & 5 deletions crates/mountmate-core/src/storage.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,10 @@ pub fn update_server_preferences_batch(
}
}
for update in updates {
let server = servers
.iter_mut()
.find(|server| server.id == update.id)
.expect("validated connection ID");
if update
.tags
.as_ref()
Expand All @@ -214,7 +218,11 @@ pub fn update_server_preferences_batch(
if let Some(tags) = &update.tags {
let mut normalized_tags = tags.clone();
crate::model::normalize_tags(&mut normalized_tags, "");
if normalized_tags.len() > crate::model::MAX_CONNECTION_TAGS {
let mut existing_tags = server.tags.clone();
crate::model::normalize_tags(&mut existing_tags, &server.folder);
let preserves_existing =
crate::model::tag_update_only_preserves_existing(&normalized_tags, &existing_tags);
if normalized_tags.len() > crate::model::MAX_CONNECTION_TAGS && !preserves_existing {
return Err(StorageError::InvalidPreferenceUpdate(format!(
"connection {} may have at most {} tags",
update.id,
Expand All @@ -224,6 +232,7 @@ pub fn update_server_preferences_batch(
if normalized_tags
.iter()
.any(|tag| tag.chars().count() > crate::model::MAX_TAG_CHARS)
&& !preserves_existing
{
return Err(StorageError::InvalidPreferenceUpdate(format!(
"tags for connection {} must be at most {} Unicode characters each",
Expand All @@ -232,10 +241,6 @@ pub fn update_server_preferences_batch(
)));
}
}
let server = servers
.iter_mut()
.find(|server| server.id == update.id)
.expect("validated connection ID");
if let Some(mut tags) = update.tags.clone() {
crate::model::normalize_tags(&mut tags, "");
server.tags = tags;
Expand Down Expand Up @@ -1125,6 +1130,56 @@ mod tests {
);
}

#[test]
fn batch_preferences_allow_progressive_cleanup_of_legacy_tag_limits() {
let temp = tempdir().unwrap();
let paths = AppPaths {
config_dir: temp.path().join("config"),
cache_dir: temp.path().join("cache"),
state_dir: temp.path().join("state"),
data_dir: temp.path().join("data"),
};
let legacy_tags = (0..=crate::model::MAX_CONNECTION_TAGS)
.map(|index| format!("tag-{index}"))
.collect::<Vec<_>>();
save_servers(
&paths,
&[ServerConfig {
id: "alpha".into(),
folder: legacy_tags[0].clone(),
tags: legacy_tags.clone(),
..ServerConfig::default()
}],
)
.unwrap();

let reduced = legacy_tags[..legacy_tags.len() - 1].to_vec();
let updated = update_server_preferences_batch(
&paths,
&[ServerPreferenceUpdate {
id: "alpha".into(),
tags: Some(reduced.clone()),
auto_mount_at_login: None,
}],
)
.unwrap();
assert_eq!(updated[0].tags, reduced);

let mut invalid_addition = updated[0].tags.clone();
invalid_addition.push("new-tag".into());
assert!(
update_server_preferences_batch(
&paths,
&[ServerPreferenceUpdate {
id: "alpha".into(),
tags: Some(invalid_addition),
auto_mount_at_login: None,
}]
)
.is_err()
);
}

#[test]
fn batch_remove_requires_existing_unique_ids_and_preserves_order() {
let temp = tempdir().unwrap();
Expand Down
Loading