Skip to content

fix: rename resolve-pr-review-thread to resolve-pr-thread #231

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
jamesadevine merged 1 commit into from
Apr 16, 2026
Merged

fix: rename resolve-pr-review-thread to resolve-pr-thread #231

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
20 changes: 10 additions & 10 deletions src/compile/common.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1176,13 +1176,13 @@ pub fn validate_update_pr_votes(front_matter: &FrontMatter) -> Result<()> {
Ok(())
}

/// Validate that resolve-pr-review-thread has a required `allowed-statuses` field when configured.
/// Validate that resolve-pr-thread has a required `allowed-statuses` field when configured.
///
/// An empty or missing `allowed-statuses` list would let agents set any thread status,
/// including "fixed" or "wontFix" on security-critical review threads. Operators must
/// explicitly opt in to each allowed status transition.
pub fn validate_resolve_pr_thread_statuses(front_matter: &FrontMatter) -> Result<()> {
if let Some(config_value) = front_matter.safe_outputs.get("resolve-pr-review-thread") {
if let Some(config_value) = front_matter.safe_outputs.get("resolve-pr-thread") {
if let Some(obj) = config_value.as_object() {
let allowed_statuses = obj.get("allowed-statuses");
let is_empty = match allowed_statuses {
Expand All @@ -1191,19 +1191,19 @@ pub fn validate_resolve_pr_thread_statuses(front_matter: &FrontMatter) -> Result
};
if is_empty {
anyhow::bail!(
"safe-outputs.resolve-pr-review-thread requires a non-empty \
"safe-outputs.resolve-pr-thread requires a non-empty \
'allowed-statuses' list to prevent agents from manipulating thread \
statuses without explicit operator consent. Example:\n\n \
safe-outputs:\n resolve-pr-review-thread:\n allowed-statuses:\n\
safe-outputs:\n resolve-pr-thread:\n allowed-statuses:\n\
\x20 - fixed\n\n\
Valid statuses: active, fixed, wont-fix, closed, by-design\n"
);
}
} else {
anyhow::bail!(
"safe-outputs.resolve-pr-review-thread must be a configuration object \
"safe-outputs.resolve-pr-thread must be a configuration object \
with an 'allowed-statuses' list. Example:\n\n \
safe-outputs:\n resolve-pr-review-thread:\n allowed-statuses:\n\
safe-outputs:\n resolve-pr-thread:\n allowed-statuses:\n\
\x20 - fixed\n"
);
}
Expand Down Expand Up @@ -2854,7 +2854,7 @@ mod tests {
#[test]
fn test_resolve_pr_thread_fails_when_allowed_statuses_missing() {
let (fm, _) = parse_markdown(
"---\nname: test\ndescription: test\nsafe-outputs:\n resolve-pr-review-thread:\n allowed-repositories:\n - self\n---\n"
"---\nname: test\ndescription: test\nsafe-outputs:\n resolve-pr-thread:\n allowed-repositories:\n - self\n---\n"
).unwrap();
let result = validate_resolve_pr_thread_statuses(&fm);
assert!(result.is_err());
Expand All @@ -2865,7 +2865,7 @@ mod tests {
#[test]
fn test_resolve_pr_thread_fails_when_allowed_statuses_empty() {
let (fm, _) = parse_markdown(
"---\nname: test\ndescription: test\nsafe-outputs:\n resolve-pr-review-thread:\n allowed-statuses: []\n---\n"
"---\nname: test\ndescription: test\nsafe-outputs:\n resolve-pr-thread:\n allowed-statuses: []\n---\n"
).unwrap();
let result = validate_resolve_pr_thread_statuses(&fm);
assert!(result.is_err());
Expand All @@ -2876,7 +2876,7 @@ mod tests {
#[test]
fn test_resolve_pr_thread_fails_when_value_is_scalar() {
let (fm, _) = parse_markdown(
"---\nname: test\ndescription: test\nsafe-outputs:\n resolve-pr-review-thread: true\n---\n"
"---\nname: test\ndescription: test\nsafe-outputs:\n resolve-pr-thread: true\n---\n"
).unwrap();
let result = validate_resolve_pr_thread_statuses(&fm);
assert!(result.is_err());
Expand All @@ -2885,7 +2885,7 @@ mod tests {
#[test]
fn test_resolve_pr_thread_passes_when_statuses_provided() {
let (fm, _) = parse_markdown(
"---\nname: test\ndescription: test\nsafe-outputs:\n resolve-pr-review-thread:\n allowed-statuses:\n - fixed\n - wont-fix\n---\n"
"---\nname: test\ndescription: test\nsafe-outputs:\n resolve-pr-thread:\n allowed-statuses:\n - fixed\n - wont-fix\n---\n"
).unwrap();
assert!(validate_resolve_pr_thread_statuses(&fm).is_ok());
}
Expand Down
8 changes: 4 additions & 4 deletions src/execute.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -378,12 +378,12 @@ pub async fn execute_safe_output(
);
output.execute_sanitized(ctx).await?
}
"resolve-pr-review-thread" => {
debug!("Parsing resolve-pr-review-thread payload");
"resolve-pr-thread" => {
debug!("Parsing resolve-pr-thread payload");
let mut output: ResolvePrThreadResult = serde_json::from_value(entry.clone())
.map_err(|e| anyhow::anyhow!("Failed to parse resolve-pr-review-thread: {}", e))?;
.map_err(|e| anyhow::anyhow!("Failed to parse resolve-pr-thread: {}", e))?;
debug!(
"resolve-pr-review-thread: pr_id={}, thread_id={}, status='{}'",
"resolve-pr-thread: pr_id={}, thread_id={}, status='{}'",
output.pull_request_id, output.thread_id, output.status
);
output.execute_sanitized(ctx).await?
Expand Down
6 changes: 3 additions & 3 deletions src/mcp.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1033,17 +1033,17 @@ Provide the PR ID, thread ID, and reply content. The reply will be posted during
}

#[tool(
name = "resolve-pr-review-thread",
name = "resolve-pr-thread",
description = "Resolve or change the status of a review thread on an Azure DevOps pull request. \
Valid statuses: fixed, wont-fix, closed, by-design, active. \
The status change will be applied during safe output processing."
)]
async fn resolve_pr_review_thread(
async fn resolve_pr_thread(
&self,
params: Parameters<ResolvePrThreadParams>,
) -> Result<CallToolResult, McpError> {
info!(
"Tool called: resolve-pr-review-thread - PR #{} thread #{} → '{}'",
"Tool called: resolve-pr-thread - PR #{} thread #{} → '{}'",
params.0.pull_request_id, params.0.thread_id, params.0.status
);
let result: ResolvePrThreadResult = params.0.try_into()?;
Expand Down
20 changes: 10 additions & 10 deletions src/safeoutputs/resolve_pr_thread.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ impl Validate for ResolvePrThreadParams {
}

tool_result! {
name = "resolve-pr-review-thread",
name = "resolve-pr-thread",
write = true,
params = ResolvePrThreadParams,
/// Result of resolving or reactivating a PR review thread
Expand All @@ -96,12 +96,12 @@ impl SanitizeContent for ResolvePrThreadResult {
}
}

/// Configuration for the resolve-pr-review-thread tool (specified in front matter)
/// Configuration for the resolve-pr-thread tool (specified in front matter)
///
/// Example front matter:
/// ```yaml
/// safe-outputs:
/// resolve-pr-review-thread:
/// resolve-pr-thread:
/// allowed-repositories:
/// - self
/// - other-repo
Expand Down Expand Up @@ -153,7 +153,7 @@ impl Executor for ResolvePrThreadResult {
.context("No access token available (SYSTEM_ACCESSTOKEN or AZURE_DEVOPS_EXT_PAT)")?;
debug!("ADO org: {}, project: {}", org_url, project);

let config: ResolvePrThreadConfig = ctx.get_tool_config("resolve-pr-review-thread");
let config: ResolvePrThreadConfig = ctx.get_tool_config("resolve-pr-thread");
debug!("Config: {:?}", config);

// Validate status against allowed-statuses — REQUIRED.
Expand All @@ -162,10 +162,10 @@ impl Executor for ResolvePrThreadResult {
// concerns as "fixed") without explicit operator consent.
if config.allowed_statuses.is_empty() {
return Ok(ExecutionResult::failure(
"resolve-pr-review-thread requires 'allowed-statuses' to be configured in \
safe-outputs.resolve-pr-review-thread. This prevents agents from \
"resolve-pr-thread requires 'allowed-statuses' to be configured in \
safe-outputs.resolve-pr-thread. This prevents agents from \
manipulating thread statuses without explicit operator consent. Example:\n \
safe-outputs:\n resolve-pr-review-thread:\n allowed-statuses:\n \
safe-outputs:\n resolve-pr-thread:\n allowed-statuses:\n \
- fixed\n\nValid statuses: active, fixed, wont-fix, closed, by-design"
.to_string(),
));
Expand Down Expand Up @@ -291,7 +291,7 @@ mod tests {

#[test]
fn test_result_has_correct_name() {
assert_eq!(ResolvePrThreadResult::NAME, "resolve-pr-review-thread");
assert_eq!(ResolvePrThreadResult::NAME, "resolve-pr-thread");
}

#[test]
Expand All @@ -314,7 +314,7 @@ mod tests {
repository: Some("self".to_string()),
};
let result: ResolvePrThreadResult = params.try_into().unwrap();
assert_eq!(result.name, "resolve-pr-review-thread");
assert_eq!(result.name, "resolve-pr-thread");
assert_eq!(result.pull_request_id, 42);
assert_eq!(result.thread_id, 7);
assert_eq!(result.status, "fixed");
Expand Down Expand Up @@ -367,7 +367,7 @@ mod tests {
let result: ResolvePrThreadResult = params.try_into().unwrap();
let json = serde_json::to_string(&result).unwrap();

assert!(json.contains(r#""name":"resolve-pr-review-thread""#));
assert!(json.contains(r#""name":"resolve-pr-thread""#));
assert!(json.contains(r#""pull_request_id":42"#));
assert!(json.contains(r#""thread_id":7"#));
}
Expand Down
Loading