Skip to content

Commit 577b92d

Browse files
jamesadevineCopilot
andcommitted
fix: rename resolve-pr-review-thread to resolve-pr-thread (#228)
Standardise on the shorter, cleaner name matching AGENTS.md and README.md documentation. Updated across: - tool_result! macro name and config key lookup - MCP tool registration and handler function - Executor dispatch match arm - Compile-time validation function and error messages - All related tests Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 1e396a0 commit 577b92d

4 files changed

Lines changed: 27 additions & 27 deletions

File tree

src/compile/common.rs

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1176,13 +1176,13 @@ pub fn validate_update_pr_votes(front_matter: &FrontMatter) -> Result<()> {
11761176
Ok(())
11771177
}
11781178

1179-
/// Validate that resolve-pr-review-thread has a required `allowed-statuses` field when configured.
1179+
/// Validate that resolve-pr-thread has a required `allowed-statuses` field when configured.
11801180
///
11811181
/// An empty or missing `allowed-statuses` list would let agents set any thread status,
11821182
/// including "fixed" or "wontFix" on security-critical review threads. Operators must
11831183
/// explicitly opt in to each allowed status transition.
11841184
pub fn validate_resolve_pr_thread_statuses(front_matter: &FrontMatter) -> Result<()> {
1185-
if let Some(config_value) = front_matter.safe_outputs.get("resolve-pr-review-thread") {
1185+
if let Some(config_value) = front_matter.safe_outputs.get("resolve-pr-thread") {
11861186
if let Some(obj) = config_value.as_object() {
11871187
let allowed_statuses = obj.get("allowed-statuses");
11881188
let is_empty = match allowed_statuses {
@@ -1191,19 +1191,19 @@ pub fn validate_resolve_pr_thread_statuses(front_matter: &FrontMatter) -> Result
11911191
};
11921192
if is_empty {
11931193
anyhow::bail!(
1194-
"safe-outputs.resolve-pr-review-thread requires a non-empty \
1194+
"safe-outputs.resolve-pr-thread requires a non-empty \
11951195
'allowed-statuses' list to prevent agents from manipulating thread \
11961196
statuses without explicit operator consent. Example:\n\n \
1197-
safe-outputs:\n resolve-pr-review-thread:\n allowed-statuses:\n\
1197+
safe-outputs:\n resolve-pr-thread:\n allowed-statuses:\n\
11981198
\x20 - fixed\n\n\
11991199
Valid statuses: active, fixed, wont-fix, closed, by-design\n"
12001200
);
12011201
}
12021202
} else {
12031203
anyhow::bail!(
1204-
"safe-outputs.resolve-pr-review-thread must be a configuration object \
1204+
"safe-outputs.resolve-pr-thread must be a configuration object \
12051205
with an 'allowed-statuses' list. Example:\n\n \
1206-
safe-outputs:\n resolve-pr-review-thread:\n allowed-statuses:\n\
1206+
safe-outputs:\n resolve-pr-thread:\n allowed-statuses:\n\
12071207
\x20 - fixed\n"
12081208
);
12091209
}
@@ -2854,7 +2854,7 @@ mod tests {
28542854
#[test]
28552855
fn test_resolve_pr_thread_fails_when_allowed_statuses_missing() {
28562856
let (fm, _) = parse_markdown(
2857-
"---\nname: test\ndescription: test\nsafe-outputs:\n resolve-pr-review-thread:\n allowed-repositories:\n - self\n---\n"
2857+
"---\nname: test\ndescription: test\nsafe-outputs:\n resolve-pr-thread:\n allowed-repositories:\n - self\n---\n"
28582858
).unwrap();
28592859
let result = validate_resolve_pr_thread_statuses(&fm);
28602860
assert!(result.is_err());
@@ -2865,7 +2865,7 @@ mod tests {
28652865
#[test]
28662866
fn test_resolve_pr_thread_fails_when_allowed_statuses_empty() {
28672867
let (fm, _) = parse_markdown(
2868-
"---\nname: test\ndescription: test\nsafe-outputs:\n resolve-pr-review-thread:\n allowed-statuses: []\n---\n"
2868+
"---\nname: test\ndescription: test\nsafe-outputs:\n resolve-pr-thread:\n allowed-statuses: []\n---\n"
28692869
).unwrap();
28702870
let result = validate_resolve_pr_thread_statuses(&fm);
28712871
assert!(result.is_err());
@@ -2876,7 +2876,7 @@ mod tests {
28762876
#[test]
28772877
fn test_resolve_pr_thread_fails_when_value_is_scalar() {
28782878
let (fm, _) = parse_markdown(
2879-
"---\nname: test\ndescription: test\nsafe-outputs:\n resolve-pr-review-thread: true\n---\n"
2879+
"---\nname: test\ndescription: test\nsafe-outputs:\n resolve-pr-thread: true\n---\n"
28802880
).unwrap();
28812881
let result = validate_resolve_pr_thread_statuses(&fm);
28822882
assert!(result.is_err());
@@ -2885,7 +2885,7 @@ mod tests {
28852885
#[test]
28862886
fn test_resolve_pr_thread_passes_when_statuses_provided() {
28872887
let (fm, _) = parse_markdown(
2888-
"---\nname: test\ndescription: test\nsafe-outputs:\n resolve-pr-review-thread:\n allowed-statuses:\n - fixed\n - wont-fix\n---\n"
2888+
"---\nname: test\ndescription: test\nsafe-outputs:\n resolve-pr-thread:\n allowed-statuses:\n - fixed\n - wont-fix\n---\n"
28892889
).unwrap();
28902890
assert!(validate_resolve_pr_thread_statuses(&fm).is_ok());
28912891
}

src/execute.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -378,12 +378,12 @@ pub async fn execute_safe_output(
378378
);
379379
output.execute_sanitized(ctx).await?
380380
}
381-
"resolve-pr-review-thread" => {
382-
debug!("Parsing resolve-pr-review-thread payload");
381+
"resolve-pr-thread" => {
382+
debug!("Parsing resolve-pr-thread payload");
383383
let mut output: ResolvePrThreadResult = serde_json::from_value(entry.clone())
384-
.map_err(|e| anyhow::anyhow!("Failed to parse resolve-pr-review-thread: {}", e))?;
384+
.map_err(|e| anyhow::anyhow!("Failed to parse resolve-pr-thread: {}", e))?;
385385
debug!(
386-
"resolve-pr-review-thread: pr_id={}, thread_id={}, status='{}'",
386+
"resolve-pr-thread: pr_id={}, thread_id={}, status='{}'",
387387
output.pull_request_id, output.thread_id, output.status
388388
);
389389
output.execute_sanitized(ctx).await?

src/mcp.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1033,17 +1033,17 @@ Provide the PR ID, thread ID, and reply content. The reply will be posted during
10331033
}
10341034

10351035
#[tool(
1036-
name = "resolve-pr-review-thread",
1036+
name = "resolve-pr-thread",
10371037
description = "Resolve or change the status of a review thread on an Azure DevOps pull request. \
10381038
Valid statuses: fixed, wont-fix, closed, by-design, active. \
10391039
The status change will be applied during safe output processing."
10401040
)]
1041-
async fn resolve_pr_review_thread(
1041+
async fn resolve_pr_thread(
10421042
&self,
10431043
params: Parameters<ResolvePrThreadParams>,
10441044
) -> Result<CallToolResult, McpError> {
10451045
info!(
1046-
"Tool called: resolve-pr-review-thread - PR #{} thread #{} → '{}'",
1046+
"Tool called: resolve-pr-thread - PR #{} thread #{} → '{}'",
10471047
params.0.pull_request_id, params.0.thread_id, params.0.status
10481048
);
10491049
let result: ResolvePrThreadResult = params.0.try_into()?;

src/safeoutputs/resolve_pr_thread.rs

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ impl Validate for ResolvePrThreadParams {
7575
}
7676

7777
tool_result! {
78-
name = "resolve-pr-review-thread",
78+
name = "resolve-pr-thread",
7979
write = true,
8080
params = ResolvePrThreadParams,
8181
/// Result of resolving or reactivating a PR review thread
@@ -96,12 +96,12 @@ impl SanitizeContent for ResolvePrThreadResult {
9696
}
9797
}
9898

99-
/// Configuration for the resolve-pr-review-thread tool (specified in front matter)
99+
/// Configuration for the resolve-pr-thread tool (specified in front matter)
100100
///
101101
/// Example front matter:
102102
/// ```yaml
103103
/// safe-outputs:
104-
/// resolve-pr-review-thread:
104+
/// resolve-pr-thread:
105105
/// allowed-repositories:
106106
/// - self
107107
/// - other-repo
@@ -153,7 +153,7 @@ impl Executor for ResolvePrThreadResult {
153153
.context("No access token available (SYSTEM_ACCESSTOKEN or AZURE_DEVOPS_EXT_PAT)")?;
154154
debug!("ADO org: {}, project: {}", org_url, project);
155155

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

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

292292
#[test]
293293
fn test_result_has_correct_name() {
294-
assert_eq!(ResolvePrThreadResult::NAME, "resolve-pr-review-thread");
294+
assert_eq!(ResolvePrThreadResult::NAME, "resolve-pr-thread");
295295
}
296296

297297
#[test]
@@ -314,7 +314,7 @@ mod tests {
314314
repository: Some("self".to_string()),
315315
};
316316
let result: ResolvePrThreadResult = params.try_into().unwrap();
317-
assert_eq!(result.name, "resolve-pr-review-thread");
317+
assert_eq!(result.name, "resolve-pr-thread");
318318
assert_eq!(result.pull_request_id, 42);
319319
assert_eq!(result.thread_id, 7);
320320
assert_eq!(result.status, "fixed");
@@ -367,7 +367,7 @@ mod tests {
367367
let result: ResolvePrThreadResult = params.try_into().unwrap();
368368
let json = serde_json::to_string(&result).unwrap();
369369

370-
assert!(json.contains(r#""name":"resolve-pr-review-thread""#));
370+
assert!(json.contains(r#""name":"resolve-pr-thread""#));
371371
assert!(json.contains(r#""pull_request_id":42"#));
372372
assert!(json.contains(r#""thread_id":7"#));
373373
}

0 commit comments

Comments
 (0)