Skip to content

fix(shared-writer): make blocker/label/relation mutations idempotent (#600) #605

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
dollspace-gay merged 1 commit into from
May 15, 2026
Merged

fix(shared-writer): make blocker/label/relation mutations idempotent (#600) #605

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 2 additions & 3 deletions crosslink/src/commands/container.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -645,7 +645,7 @@ mod tests {

/// The container subcommands MUST address the same registry-qualified
/// image name as `crosslink kickoff run --container docker|podman`.
/// Regressing IMAGE_NAME back to the bare `crosslink-agent` form
/// Regressing `IMAGE_NAME` back to the bare `crosslink-agent` form
/// silently un-composes the two code paths and re-opens GH#576.
#[test]
fn image_name_is_ghcr_namespaced() {
Expand All @@ -654,8 +654,7 @@ mod tests {
IMAGE_NAME,
crate::commands::kickoff::DEFAULT_AGENT_IMAGE
.rsplit_once(':')
.map(|(name, _)| name)
.unwrap_or(IMAGE_NAME),
.map_or(IMAGE_NAME, |(name, _)| name),
"container.rs IMAGE_NAME diverged from kickoff DEFAULT_AGENT_IMAGE — \
re-opens the GH#576 compose-failure between `crosslink container build` \
and `crosslink kickoff run --container …`"
Expand Down
30 changes: 18 additions & 12 deletions crosslink/src/commands/deps.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,15 @@ pub fn block(
}

if let Some(w) = writer {
w.add_blocker(db, issue_id, blocker_id)?;
println!(
"Issue {} is now blocked by {}",
format_issue_id(issue_id),
format_issue_id(blocker_id)
);
if w.add_blocker(db, issue_id, blocker_id)? {
println!(
"Issue {} is now blocked by {}",
format_issue_id(issue_id),
format_issue_id(blocker_id)
);
} else {
println!("Dependency already exists");
}
} else if db.add_dependency(issue_id, blocker_id)? {
println!(
"Issue {} is now blocked by {}",
Expand All @@ -45,12 +48,15 @@ pub fn unblock(
blocker_id: i64,
) -> Result<()> {
if let Some(w) = writer {
w.remove_blocker(db, issue_id, blocker_id)?;
println!(
"Removed: {} no longer blocked by {}",
format_issue_id(issue_id),
format_issue_id(blocker_id)
);
if w.remove_blocker(db, issue_id, blocker_id)? {
println!(
"Removed: {} no longer blocked by {}",
format_issue_id(issue_id),
format_issue_id(blocker_id)
);
} else {
println!("No such dependency found");
}
} else if db.remove_dependency(issue_id, blocker_id)? {
println!(
"Removed: {} no longer blocked by {}",
Expand Down
38 changes: 26 additions & 12 deletions crosslink/src/commands/label.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,19 @@ pub fn add(db: &Database, writer: Option<&SharedWriter>, issue_id: i64, label: &
db.require_issue(issue_id)?;

if let Some(w) = writer {
w.add_label(db, issue_id, label)?;
println!(
"Added label '{}' to issue {}",
label,
format_issue_id(issue_id)
);
if w.add_label(db, issue_id, label)? {
println!(
"Added label '{}' to issue {}",
label,
format_issue_id(issue_id)
);
} else {
println!(
"Label '{}' already exists on issue {}",
label,
format_issue_id(issue_id)
);
}
} else if db.add_label(issue_id, label)? {
println!(
"Added label '{}' to issue {}",
Expand All @@ -39,12 +46,19 @@ pub fn remove(
db.require_issue(issue_id)?;

if let Some(w) = writer {
w.remove_label(db, issue_id, label)?;
println!(
"Removed label '{}' from issue {}",
label,
format_issue_id(issue_id)
);
if w.remove_label(db, issue_id, label)? {
println!(
"Removed label '{}' from issue {}",
label,
format_issue_id(issue_id)
);
} else {
println!(
"Label '{}' not found on issue {}",
label,
format_issue_id(issue_id)
);
}
} else if db.remove_label(issue_id, label)? {
println!(
"Removed label '{}' from issue {}",
Expand Down
38 changes: 26 additions & 12 deletions crosslink/src/commands/relate.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,19 @@ pub fn add(
db.require_issue(related_id)?;

if let Some(w) = writer {
w.add_relation(db, issue_id, related_id)?;
println!(
"Linked {} ↔ {}",
format_issue_id(issue_id),
format_issue_id(related_id)
);
if w.add_relation(db, issue_id, related_id)? {
println!(
"Linked {} ↔ {}",
format_issue_id(issue_id),
format_issue_id(related_id)
);
} else {
println!(
"Issues {} and {} are already related",
format_issue_id(issue_id),
format_issue_id(related_id)
);
}
} else if db.add_relation(issue_id, related_id)? {
println!(
"Linked {} ↔ {}",
Expand All @@ -44,12 +51,19 @@ pub fn remove(
related_id: i64,
) -> Result<()> {
if let Some(w) = writer {
w.remove_relation(db, issue_id, related_id)?;
println!(
"Unlinked {} ↔ {}",
format_issue_id(issue_id),
format_issue_id(related_id)
);
if w.remove_relation(db, issue_id, related_id)? {
println!(
"Unlinked {} ↔ {}",
format_issue_id(issue_id),
format_issue_id(related_id)
);
} else {
println!(
"No relation found between {} and {}",
format_issue_id(issue_id),
format_issue_id(related_id)
);
}
} else if db.remove_relation(issue_id, related_id)? {
println!(
"Unlinked {} ↔ {}",
Expand Down
42 changes: 21 additions & 21 deletions crosslink/src/commands/sentinel/collect.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -507,6 +507,27 @@ fn build_fix_template(ctx: &TemplateContext<'_>) -> String {
)
}

/// Post a comment to a GitHub issue via `gh`.
fn post_gh_comment(gh_issue_number: i64, body: &str) -> Result<()> {
let output = Command::new("gh")
.args([
"issue",
"comment",
&gh_issue_number.to_string(),
"--body",
body,
])
.output()
.context("Failed to run `gh issue comment`")?;

if !output.status.success() {
let stderr = String::from_utf8_lossy(&output.stderr);
anyhow::bail!("gh issue comment failed: {}", stderr.trim());
}

Ok(())
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down Expand Up @@ -554,24 +575,3 @@ mod tests {
assert_eq!(classify_status(" ", 1), None);
}
}

/// Post a comment to a GitHub issue via `gh`.
fn post_gh_comment(gh_issue_number: i64, body: &str) -> Result<()> {
let output = Command::new("gh")
.args([
"issue",
"comment",
&gh_issue_number.to_string(),
"--body",
body,
])
.output()
.context("Failed to run `gh issue comment`")?;

if !output.status.success() {
let stderr = String::from_utf8_lossy(&output.stderr);
anyhow::bail!("gh issue comment failed: {}", stderr.trim());
}

Ok(())
}
3 changes: 1 addition & 2 deletions crosslink/src/compaction.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -792,8 +792,7 @@ mod tests {
.stdout(std::process::Stdio::null())
.stderr(std::process::Stdio::null())
.status()
.map(|s| s.success())
.unwrap_or(false)
.is_ok_and(|s| s.success())
}

/// Release the compaction lease.
Expand Down
22 changes: 11 additions & 11 deletions crosslink/src/lock_check.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -697,7 +697,7 @@ mod tests {
.args(["init", "-q"])
.current_dir(repo_root)
.status();
if init_status.map(|s| !s.success()).unwrap_or(true) {
if init_status.map_or(true, |s| !s.success()) {
// git not available in this environment; skip gracefully
return;
}
Expand Down Expand Up @@ -772,7 +772,7 @@ mod tests {
.args(["init", "-q"])
.current_dir(repo_root)
.status();
if init_status.map(|s| !s.success()).unwrap_or(true) {
if init_status.map_or(true, |s| !s.success()) {
return;
}

Expand Down Expand Up @@ -844,7 +844,7 @@ mod tests {
.args(["init", "-q"])
.current_dir(repo_root)
.status();
if init_status.map(|s| !s.success()).unwrap_or(true) {
if init_status.map_or(true, |s| !s.success()) {
return;
}

Expand Down Expand Up @@ -892,7 +892,7 @@ mod tests {
.args(["init", "-q"])
.current_dir(repo_root)
.status();
if init_status.map(|s| !s.success()).unwrap_or(true) {
if init_status.map_or(true, |s| !s.success()) {
return;
}

Expand Down Expand Up @@ -933,7 +933,7 @@ mod tests {
.args(["init", "-q"])
.current_dir(repo_root)
.status();
if init_status.map(|s| !s.success()).unwrap_or(true) {
if init_status.map_or(true, |s| !s.success()) {
return;
}

Expand Down Expand Up @@ -977,7 +977,7 @@ mod tests {
.args(["init", "-q"])
.current_dir(repo_root)
.status();
if init_status.map(|s| !s.success()).unwrap_or(true) {
if init_status.map_or(true, |s| !s.success()) {
return;
}

Expand Down Expand Up @@ -1015,7 +1015,7 @@ mod tests {
.args(["init", "-q"])
.current_dir(repo_root)
.status();
if init_status.map(|s| !s.success()).unwrap_or(true) {
if init_status.map_or(true, |s| !s.success()) {
return;
}

Expand Down Expand Up @@ -1081,7 +1081,7 @@ mod tests {
.args(["init", "-q"])
.current_dir(repo_root)
.status();
if init_status.map(|s| !s.success()).unwrap_or(true) {
if init_status.map_or(true, |s| !s.success()) {
return;
}

Expand Down Expand Up @@ -1237,7 +1237,7 @@ mod tests {
.args(["init", "-q"])
.current_dir(repo_root)
.status();
if init_status.map(|s| !s.success()).unwrap_or(true) {
if init_status.map_or(true, |s| !s.success()) {
return;
}

Expand Down Expand Up @@ -1273,7 +1273,7 @@ mod tests {
.args(["init", "-q"])
.current_dir(repo_root)
.status();
if init_status.map(|s| !s.success()).unwrap_or(true) {
if init_status.map_or(true, |s| !s.success()) {
return;
}

Expand Down Expand Up @@ -1440,7 +1440,7 @@ mod tests {
.args(["init", "-q"])
.current_dir(repo_root)
.status();
if init_status.map(|s| !s.success()).unwrap_or(true) {
if init_status.map_or(true, |s| !s.success()) {
return;
}

Expand Down
Loading
Loading