feat: scope merge request tools by profile flags
This commit is contained in:
@@ -18,9 +18,10 @@ use crate::model::{AuthRef, ModelManifest, ReasoningControl};
|
||||
use crate::plugin::PluginConfig;
|
||||
use crate::{
|
||||
CompactionConfig, EngineManifest, FeatureConfig, FeatureFlagConfig, FileUploadLimits,
|
||||
McpConfig, McpEnvValue, McpStdioCwdPolicy, MemoryConfig, MemoryFeatureConfig, ScopeConfig,
|
||||
SessionConfig, SkillsConfig, TicketFeatureConfig, ToolOutputLimits, ToolPermissionConfig,
|
||||
ToolPermissionRule, WebConfig, WorkerFeatureConfig, WorkerManifest, WorkerMeta,
|
||||
McpConfig, McpEnvValue, McpStdioCwdPolicy, MemoryConfig, MemoryFeatureConfig,
|
||||
MergeRequestFeatureConfig, ScopeConfig, SessionConfig, SkillsConfig, TicketFeatureConfig,
|
||||
ToolOutputLimits, ToolPermissionConfig, ToolPermissionRule, WebConfig, WorkerFeatureConfig,
|
||||
WorkerManifest, WorkerMeta,
|
||||
};
|
||||
|
||||
/// Partial-form Worker manifest. Every field is optional; one or more
|
||||
@@ -97,6 +98,8 @@ pub struct FeatureConfigPartial {
|
||||
#[serde(default)]
|
||||
pub ticket: Option<TicketFeatureConfigPartial>,
|
||||
#[serde(default)]
|
||||
pub merge_request: Option<MergeRequestFeatureConfigPartial>,
|
||||
#[serde(default)]
|
||||
pub orchestration: Option<FeatureFlagConfigPartial>,
|
||||
#[serde(default)]
|
||||
pub plugins: Option<FeatureFlagConfigPartial>,
|
||||
@@ -127,6 +130,11 @@ impl FeatureConfigPartial {
|
||||
FeatureFlagConfigPartial::merge,
|
||||
),
|
||||
ticket: merge_option(self.ticket, other.ticket, TicketFeatureConfigPartial::merge),
|
||||
merge_request: merge_option(
|
||||
self.merge_request,
|
||||
other.merge_request,
|
||||
MergeRequestFeatureConfigPartial::merge,
|
||||
),
|
||||
orchestration: merge_option(
|
||||
self.orchestration,
|
||||
other.orchestration,
|
||||
@@ -216,6 +224,28 @@ impl TicketFeatureConfigPartial {
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Default, Deserialize, Serialize, PartialEq, Eq)]
|
||||
#[serde(default, deny_unknown_fields)]
|
||||
pub struct MergeRequestFeatureConfigPartial {
|
||||
pub show: Option<bool>,
|
||||
pub open: Option<bool>,
|
||||
pub review: Option<bool>,
|
||||
pub readiness_check: Option<bool>,
|
||||
pub complete: Option<bool>,
|
||||
}
|
||||
|
||||
impl MergeRequestFeatureConfigPartial {
|
||||
fn merge(self, other: Self) -> Self {
|
||||
Self {
|
||||
show: other.show.or(self.show),
|
||||
open: other.open.or(self.open),
|
||||
review: other.review.or(self.review),
|
||||
readiness_check: other.readiness_check.or(self.readiness_check),
|
||||
complete: other.complete.or(self.complete),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl From<FeatureConfigPartial> for FeatureConfig {
|
||||
fn from(value: FeatureConfigPartial) -> Self {
|
||||
Self {
|
||||
@@ -247,6 +277,10 @@ impl From<FeatureConfigPartial> for FeatureConfig {
|
||||
.ticket
|
||||
.map(TicketFeatureConfig::from)
|
||||
.unwrap_or_default(),
|
||||
merge_request: value
|
||||
.merge_request
|
||||
.map(MergeRequestFeatureConfig::from)
|
||||
.unwrap_or_default(),
|
||||
orchestration: value
|
||||
.orchestration
|
||||
.map(FeatureFlagConfig::from)
|
||||
@@ -326,6 +360,30 @@ impl From<TicketFeatureConfig> for TicketFeatureConfigPartial {
|
||||
}
|
||||
}
|
||||
|
||||
impl From<MergeRequestFeatureConfigPartial> for MergeRequestFeatureConfig {
|
||||
fn from(value: MergeRequestFeatureConfigPartial) -> Self {
|
||||
Self {
|
||||
show: value.show.unwrap_or_default(),
|
||||
open: value.open.unwrap_or_default(),
|
||||
review: value.review.unwrap_or_default(),
|
||||
readiness_check: value.readiness_check.unwrap_or_default(),
|
||||
complete: value.complete.unwrap_or_default(),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl From<MergeRequestFeatureConfig> for MergeRequestFeatureConfigPartial {
|
||||
fn from(value: MergeRequestFeatureConfig) -> Self {
|
||||
Self {
|
||||
show: Some(value.show),
|
||||
open: Some(value.open),
|
||||
review: Some(value.review),
|
||||
readiness_check: Some(value.readiness_check),
|
||||
complete: Some(value.complete),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl From<FeatureConfig> for FeatureConfigPartial {
|
||||
fn from(value: FeatureConfig) -> Self {
|
||||
Self {
|
||||
@@ -339,6 +397,7 @@ impl From<FeatureConfig> for FeatureConfigPartial {
|
||||
objective: Some(value.objective.into()),
|
||||
manage_workdir: Some(value.manage_workdir.into()),
|
||||
ticket: Some(value.ticket.into()),
|
||||
merge_request: Some(value.merge_request.into()),
|
||||
orchestration: Some(value.orchestration.into()),
|
||||
plugins: Some(value.plugins.into()),
|
||||
}
|
||||
@@ -1880,6 +1939,7 @@ worker_max_turns = 7
|
||||
assert!(!manifest.feature.objective.enabled);
|
||||
assert!(!manifest.feature.manage_workdir.enabled);
|
||||
assert!(!manifest.feature.ticket.enabled);
|
||||
assert!(!manifest.feature.merge_request.any());
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -1899,6 +1959,13 @@ thread = false
|
||||
intake = false
|
||||
workflow = false
|
||||
|
||||
[feature.merge_request]
|
||||
show = true
|
||||
open = false
|
||||
review = true
|
||||
readiness_check = false
|
||||
complete = false
|
||||
|
||||
[feature.orchestration]
|
||||
enabled = false
|
||||
"#,
|
||||
@@ -1934,6 +2001,14 @@ enabled = false
|
||||
assert!(!manifest.feature.ticket.thread);
|
||||
assert!(!manifest.feature.ticket.intake);
|
||||
assert!(!manifest.feature.ticket.workflow);
|
||||
assert_eq!(
|
||||
manifest.feature.merge_request,
|
||||
MergeRequestFeatureConfig {
|
||||
show: true,
|
||||
review: true,
|
||||
..Default::default()
|
||||
}
|
||||
);
|
||||
assert!(!manifest.feature.orchestration.enabled);
|
||||
assert!(!manifest.feature.memory.enabled);
|
||||
assert!(!manifest.feature.memory.staging);
|
||||
@@ -1957,6 +2032,13 @@ thread = false
|
||||
intake = false
|
||||
workflow = false
|
||||
|
||||
[feature.merge_request]
|
||||
show = true
|
||||
open = false
|
||||
review = true
|
||||
readiness_check = false
|
||||
complete = false
|
||||
|
||||
[feature.orchestration]
|
||||
enabled = false
|
||||
"#,
|
||||
@@ -1968,6 +2050,11 @@ enabled = false
|
||||
thread = true
|
||||
workflow = true
|
||||
|
||||
[feature.merge_request]
|
||||
open = true
|
||||
review = false
|
||||
readiness_check = true
|
||||
|
||||
[feature.orchestration]
|
||||
enabled = true
|
||||
|
||||
@@ -2017,6 +2104,16 @@ enabled = true
|
||||
assert!(manifest.feature.ticket.thread);
|
||||
assert!(!manifest.feature.ticket.intake);
|
||||
assert!(manifest.feature.ticket.workflow);
|
||||
assert_eq!(
|
||||
manifest.feature.merge_request,
|
||||
MergeRequestFeatureConfig {
|
||||
show: true,
|
||||
open: true,
|
||||
review: false,
|
||||
readiness_check: true,
|
||||
complete: false,
|
||||
}
|
||||
);
|
||||
assert!(manifest.feature.orchestration.enabled);
|
||||
assert!(manifest.feature.objective.enabled);
|
||||
assert!(manifest.feature.web.enabled);
|
||||
|
||||
@@ -125,6 +125,8 @@ pub struct FeatureConfig {
|
||||
#[serde(default)]
|
||||
pub ticket: TicketFeatureConfig,
|
||||
#[serde(default)]
|
||||
pub merge_request: MergeRequestFeatureConfig,
|
||||
#[serde(default)]
|
||||
pub orchestration: FeatureFlagConfig,
|
||||
#[serde(default)]
|
||||
pub plugins: FeatureFlagConfig,
|
||||
@@ -143,6 +145,7 @@ impl Default for FeatureConfig {
|
||||
objective: FeatureFlagConfig::disabled(),
|
||||
manage_workdir: FeatureFlagConfig::disabled(),
|
||||
ticket: TicketFeatureConfig::default(),
|
||||
merge_request: MergeRequestFeatureConfig::default(),
|
||||
orchestration: FeatureFlagConfig::disabled(),
|
||||
plugins: FeatureFlagConfig::disabled(),
|
||||
}
|
||||
@@ -252,6 +255,27 @@ pub struct TicketFeatureConfig {
|
||||
pub workflow: bool,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq, Default)]
|
||||
#[serde(deny_unknown_fields)]
|
||||
pub struct MergeRequestFeatureConfig {
|
||||
#[serde(default)]
|
||||
pub show: bool,
|
||||
#[serde(default)]
|
||||
pub open: bool,
|
||||
#[serde(default)]
|
||||
pub review: bool,
|
||||
#[serde(default)]
|
||||
pub readiness_check: bool,
|
||||
#[serde(default)]
|
||||
pub complete: bool,
|
||||
}
|
||||
|
||||
impl MergeRequestFeatureConfig {
|
||||
pub fn any(self) -> bool {
|
||||
self.show || self.open || self.review || self.readiness_check || self.complete
|
||||
}
|
||||
}
|
||||
|
||||
/// External Agent Skills (`SKILL.md`) ingest configuration. Skills are
|
||||
/// loaded *only* from the directories listed here — there is no
|
||||
/// implicit `$config_dir/skills/` or builtin probe. Profile and Manifest
|
||||
|
||||
@@ -919,6 +919,37 @@ fn apply_role_profile(
|
||||
_ => serde_json::json!({ "enabled": true, "authoring": true, "thread": true }),
|
||||
};
|
||||
value["feature"]["ticket"] = ticket;
|
||||
let merge_request = match slug {
|
||||
"coder" => serde_json::json!({
|
||||
"show": true,
|
||||
"open": true,
|
||||
"review": false,
|
||||
"readiness_check": false,
|
||||
"complete": false
|
||||
}),
|
||||
"reviewer" => serde_json::json!({
|
||||
"show": true,
|
||||
"open": false,
|
||||
"review": true,
|
||||
"readiness_check": false,
|
||||
"complete": false
|
||||
}),
|
||||
"orchestrator" => serde_json::json!({
|
||||
"show": true,
|
||||
"open": false,
|
||||
"review": false,
|
||||
"readiness_check": true,
|
||||
"complete": true
|
||||
}),
|
||||
_ => serde_json::json!({
|
||||
"show": false,
|
||||
"open": false,
|
||||
"review": false,
|
||||
"readiness_check": false,
|
||||
"complete": false
|
||||
}),
|
||||
};
|
||||
value["feature"]["merge_request"] = merge_request;
|
||||
}
|
||||
|
||||
fn reject_manifest_shaped_profile(value: &serde_json::Value) -> Result<(), ProfileError> {
|
||||
@@ -1361,6 +1392,7 @@ mod tests {
|
||||
assert!(companion.feature.objective.enabled);
|
||||
assert!(!companion.feature.ticket.intake);
|
||||
assert!(!companion.feature.orchestration.enabled);
|
||||
assert!(!companion.feature.merge_request.any());
|
||||
assert_eq!(
|
||||
companion.compaction.as_ref().unwrap().threshold,
|
||||
Some(240000)
|
||||
@@ -1401,6 +1433,15 @@ mod tests {
|
||||
assert!(!orchestrator.feature.sub_worker.enabled);
|
||||
assert!(orchestrator.feature.worker.enabled);
|
||||
assert!(!orchestrator.feature.worker.direct_spawn);
|
||||
assert_eq!(
|
||||
orchestrator.feature.merge_request,
|
||||
crate::MergeRequestFeatureConfig {
|
||||
show: true,
|
||||
readiness_check: true,
|
||||
complete: true,
|
||||
..Default::default()
|
||||
}
|
||||
);
|
||||
assert!(orchestrator.feature.ticket.enabled);
|
||||
assert!(orchestrator.feature.ticket.enabled);
|
||||
assert!(!orchestrator.feature.ticket.authoring);
|
||||
@@ -1424,6 +1465,14 @@ mod tests {
|
||||
assert!(coder.feature.sub_worker.enabled);
|
||||
assert!(coder.feature.flow.enabled);
|
||||
assert!(!coder.feature.worker.enabled);
|
||||
assert_eq!(
|
||||
coder.feature.merge_request,
|
||||
crate::MergeRequestFeatureConfig {
|
||||
show: true,
|
||||
open: true,
|
||||
..Default::default()
|
||||
}
|
||||
);
|
||||
assert!(coder.scope.allow.is_empty());
|
||||
assert!(coder.delegation_scope.allow.is_empty());
|
||||
assert_eq!(coder.model.ref_.as_deref(), Some("codex-oauth/gpt-5.5"));
|
||||
@@ -1442,6 +1491,14 @@ mod tests {
|
||||
assert!(!reviewer.feature.sub_worker.enabled);
|
||||
assert!(!reviewer.feature.flow.enabled);
|
||||
assert!(!reviewer.feature.worker.enabled);
|
||||
assert_eq!(
|
||||
reviewer.feature.merge_request,
|
||||
crate::MergeRequestFeatureConfig {
|
||||
show: true,
|
||||
review: true,
|
||||
..Default::default()
|
||||
}
|
||||
);
|
||||
assert!(reviewer.feature.ticket.enabled);
|
||||
assert!(reviewer.feature.ticket.enabled);
|
||||
assert!(!reviewer.feature.ticket.authoring);
|
||||
@@ -1604,6 +1661,14 @@ enabled = true
|
||||
authoring = false
|
||||
thread = false
|
||||
intake = false
|
||||
|
||||
[feature.merge_request]
|
||||
show = true
|
||||
open = false
|
||||
review = true
|
||||
readiness_check = false
|
||||
complete = false
|
||||
|
||||
[feature.orchestration]
|
||||
enabled = false
|
||||
"#,
|
||||
@@ -1626,6 +1691,14 @@ enabled = false
|
||||
assert!(!resolved.manifest.feature.ticket.authoring);
|
||||
assert!(!resolved.manifest.feature.ticket.thread);
|
||||
assert!(!resolved.manifest.feature.ticket.intake);
|
||||
assert_eq!(
|
||||
resolved.manifest.feature.merge_request,
|
||||
crate::MergeRequestFeatureConfig {
|
||||
show: true,
|
||||
review: true,
|
||||
..Default::default()
|
||||
}
|
||||
);
|
||||
assert!(!resolved.manifest.feature.orchestration.enabled);
|
||||
assert_eq!(
|
||||
resolved.manifest.delegation_scope.allow[0].target,
|
||||
|
||||
@@ -769,6 +769,21 @@ where
|
||||
),
|
||||
);
|
||||
}
|
||||
if feature_config.merge_request.any() {
|
||||
let workspace_client = worker.workspace_client_handle();
|
||||
if !workspace_client.is_available() || workspace_client.workspace_id().is_none() {
|
||||
return Err(std::io::Error::new(
|
||||
std::io::ErrorKind::InvalidInput,
|
||||
"Merge Request tools require Backend Workspace API authority",
|
||||
));
|
||||
}
|
||||
feature_registry.add_module(
|
||||
crate::feature::builtin::merge_request::MergeRequestFeature::new(
|
||||
workspace_client,
|
||||
feature_config.merge_request,
|
||||
),
|
||||
);
|
||||
}
|
||||
if feature_config.manage_workdir.enabled {
|
||||
// Workdir lifecycle is Workspace control-plane authority. The Worker
|
||||
// receives only the injected WorkspaceClient and never Runtime URLs,
|
||||
|
||||
@@ -1,19 +1,41 @@
|
||||
use crate::feature::ToolDefinition;
|
||||
use crate::feature::{
|
||||
FeatureDescriptor, FeatureInstallContext, FeatureInstallError, FeatureInstructionContribution,
|
||||
FeatureInstructionDeclaration, FeatureInstructionId, FeatureModule, ToolContribution,
|
||||
ToolDeclaration, ToolDefinition,
|
||||
};
|
||||
use crate::worker::{WorkspaceClient, WorkspaceRequest, WorkspaceRequestMethod};
|
||||
use async_trait::async_trait;
|
||||
use llm_engine::tool::{Tool, ToolError, ToolExecutionContext, ToolMeta, ToolOutput};
|
||||
use manifest::MergeRequestFeatureConfig;
|
||||
use schemars::JsonSchema;
|
||||
use serde::Deserialize;
|
||||
use serde_json::json;
|
||||
use std::sync::Arc;
|
||||
|
||||
pub const MERGE_REQUEST_COMMON_TOOL_NAMES: &[&str] = &[
|
||||
"MergeRequestShow",
|
||||
"MergeRequestReadinessCheck",
|
||||
"MergeRequestOpen",
|
||||
"MergeRequestComplete",
|
||||
pub const FEATURE_ID: &str = "merge_request";
|
||||
const FEATURE_NAME: &str = "Merge Request tools";
|
||||
const FEATURE_DESCRIPTION: &str =
|
||||
"Operation-specific Merge Request workflow tools over Workspace authority.";
|
||||
const FEATURE_INSTRUCTION_ID: &str = "merge_request.workflow";
|
||||
pub const FEATURE_PROMPT_REF: &str = "common.merge_request";
|
||||
|
||||
fn workflow_instruction() -> FeatureInstructionDeclaration {
|
||||
FeatureInstructionDeclaration::new(
|
||||
FeatureInstructionId::builtin(FEATURE_INSTRUCTION_ID),
|
||||
FEATURE_PROMPT_REF,
|
||||
"Operation-specific Merge Request workflow guidance",
|
||||
)
|
||||
.expect("static Merge Request workflow instruction declaration is valid")
|
||||
}
|
||||
|
||||
const ALL_KINDS: [Kind; 5] = [
|
||||
Kind::Show,
|
||||
Kind::Open,
|
||||
Kind::Review,
|
||||
Kind::Readiness,
|
||||
Kind::Complete,
|
||||
];
|
||||
pub const MERGE_REQUEST_REVIEW_TOOL_NAME: &str = "MergeRequestReviewSubmit";
|
||||
|
||||
#[derive(Clone, Copy)]
|
||||
enum Kind {
|
||||
Show,
|
||||
@@ -89,13 +111,23 @@ struct ReviewFindingInput {
|
||||
body: String,
|
||||
}
|
||||
impl Kind {
|
||||
fn enabled(self, config: MergeRequestFeatureConfig) -> bool {
|
||||
match self {
|
||||
Self::Show => config.show,
|
||||
Self::Open => config.open,
|
||||
Self::Review => config.review,
|
||||
Self::Readiness => config.readiness_check,
|
||||
Self::Complete => config.complete,
|
||||
}
|
||||
}
|
||||
|
||||
fn name(self) -> &'static str {
|
||||
match self {
|
||||
Self::Show => "MergeRequestShow",
|
||||
Self::Readiness => "MergeRequestReadinessCheck",
|
||||
Self::Open => "MergeRequestOpen",
|
||||
Self::Complete => "MergeRequestComplete",
|
||||
Self::Review => "MergeRequestReviewSubmit",
|
||||
Self::Review => "MergeRequestReview",
|
||||
}
|
||||
}
|
||||
fn schema(self) -> serde_json::Value {
|
||||
@@ -218,24 +250,53 @@ fn definition(client: Arc<dyn WorkspaceClient>, kind: Kind) -> ToolDefinition {
|
||||
)
|
||||
})
|
||||
}
|
||||
pub fn common_tools(c: Arc<dyn WorkspaceClient>) -> Vec<ToolDefinition> {
|
||||
vec![
|
||||
definition(c.clone(), Kind::Show),
|
||||
definition(c.clone(), Kind::Readiness),
|
||||
definition(c.clone(), Kind::Open),
|
||||
definition(c, Kind::Complete),
|
||||
]
|
||||
pub struct MergeRequestFeature {
|
||||
client: Arc<dyn WorkspaceClient>,
|
||||
config: MergeRequestFeatureConfig,
|
||||
}
|
||||
pub fn reviewer_tools(c: Arc<dyn WorkspaceClient>) -> Vec<ToolDefinition> {
|
||||
if c.reviewer_context().is_some() {
|
||||
vec![
|
||||
definition(c.clone(), Kind::Show),
|
||||
definition(c, Kind::Review),
|
||||
]
|
||||
} else {
|
||||
vec![]
|
||||
|
||||
impl MergeRequestFeature {
|
||||
pub fn new(client: Arc<dyn WorkspaceClient>, config: MergeRequestFeatureConfig) -> Self {
|
||||
Self { client, config }
|
||||
}
|
||||
|
||||
fn kinds(&self) -> impl Iterator<Item = Kind> + '_ {
|
||||
ALL_KINDS
|
||||
.into_iter()
|
||||
.filter(|kind| kind.enabled(self.config))
|
||||
}
|
||||
}
|
||||
|
||||
impl FeatureModule for MergeRequestFeature {
|
||||
fn descriptor(&self) -> FeatureDescriptor {
|
||||
let mut descriptor = FeatureDescriptor::builtin(FEATURE_ID, FEATURE_NAME)
|
||||
.with_description(FEATURE_DESCRIPTION);
|
||||
if self.config.any() {
|
||||
descriptor = descriptor.with_instruction(workflow_instruction());
|
||||
}
|
||||
for kind in self.kinds() {
|
||||
descriptor = descriptor.with_tool(ToolDeclaration::new(
|
||||
kind.name(),
|
||||
description(kind.name()).unwrap_or("Merge Request operation."),
|
||||
));
|
||||
}
|
||||
descriptor
|
||||
}
|
||||
|
||||
fn install(&self, ctx: &mut FeatureInstallContext<'_>) -> Result<(), FeatureInstallError> {
|
||||
if self.config.any() {
|
||||
ctx.instructions()
|
||||
.register(FeatureInstructionContribution::new(workflow_instruction()))?;
|
||||
}
|
||||
let mut tools = ctx.tools();
|
||||
for kind in self.kinds() {
|
||||
let definition = definition(self.client.clone(), kind);
|
||||
tools.register(ToolContribution::new(kind.name(), definition))?;
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
||||
pub fn description(n: &str) -> Option<&'static str> {
|
||||
match n {
|
||||
"MergeRequestShow" => Some("Read the selector-based Merge Request and append-only thread."),
|
||||
@@ -248,7 +309,7 @@ pub fn description(n: &str) -> Option<&'static str> {
|
||||
"MergeRequestComplete" => {
|
||||
Some("Complete using an approved review event and final target-ref evidence.")
|
||||
}
|
||||
"MergeRequestReviewSubmit" => {
|
||||
"MergeRequestReview" => {
|
||||
Some("Submit the injected Reviewer capability result for its captured subject ref.")
|
||||
}
|
||||
_ => None,
|
||||
@@ -257,6 +318,72 @@ pub fn description(n: &str) -> Option<&'static str> {
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use crate::feature::FeatureRegistryBuilder;
|
||||
use crate::hook::HookRegistryBuilder;
|
||||
use crate::worker::TestWorkspaceHttpClient;
|
||||
|
||||
fn install(config: MergeRequestFeatureConfig) -> (Vec<String>, Vec<String>) {
|
||||
let client: Arc<dyn WorkspaceClient> =
|
||||
Arc::new(TestWorkspaceHttpClient::new("workspace", "http://unused"));
|
||||
let mut pending_tools = Vec::new();
|
||||
let mut hook_builder = HookRegistryBuilder::default();
|
||||
let report = FeatureRegistryBuilder::new()
|
||||
.with_module(MergeRequestFeature::new(client, config))
|
||||
.install_into_pending(&mut pending_tools, &mut hook_builder);
|
||||
assert!(!report.has_errors(), "{}", report.error_message());
|
||||
(
|
||||
report.installed_tool_names(),
|
||||
report
|
||||
.installed_instruction_contributions()
|
||||
.into_iter()
|
||||
.map(|instruction| instruction.prompt_ref)
|
||||
.collect(),
|
||||
)
|
||||
}
|
||||
|
||||
fn tool_names(config: MergeRequestFeatureConfig) -> Vec<String> {
|
||||
install(config).0
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn flags_define_the_exact_registered_tool_surface() {
|
||||
let coder = MergeRequestFeatureConfig {
|
||||
show: true,
|
||||
open: true,
|
||||
..Default::default()
|
||||
};
|
||||
assert_eq!(tool_names(coder), ["MergeRequestShow", "MergeRequestOpen"]);
|
||||
|
||||
let reviewer = MergeRequestFeatureConfig {
|
||||
show: true,
|
||||
review: true,
|
||||
..Default::default()
|
||||
};
|
||||
assert_eq!(
|
||||
tool_names(reviewer),
|
||||
["MergeRequestShow", "MergeRequestReview"]
|
||||
);
|
||||
|
||||
let orchestrator = MergeRequestFeatureConfig {
|
||||
show: true,
|
||||
readiness_check: true,
|
||||
complete: true,
|
||||
..Default::default()
|
||||
};
|
||||
assert_eq!(
|
||||
tool_names(orchestrator),
|
||||
[
|
||||
"MergeRequestShow",
|
||||
"MergeRequestReadinessCheck",
|
||||
"MergeRequestComplete"
|
||||
]
|
||||
);
|
||||
assert_eq!(install(coder).1, [FEATURE_PROMPT_REF]);
|
||||
let unspecified = install(MergeRequestFeatureConfig::default());
|
||||
assert!(unspecified.0.is_empty());
|
||||
assert!(unspecified.1.is_empty());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn schemas_hide_revision_and_commit_authority() {
|
||||
let schemas = [
|
||||
@@ -276,6 +403,5 @@ mod tests {
|
||||
assert!(!j.contains(banned), "{banned} in {j}")
|
||||
}
|
||||
}
|
||||
assert!(!MERGE_REQUEST_COMMON_TOOL_NAMES.contains(&"MergeRequestRequestReview"));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -24,7 +24,6 @@ use ticket::{
|
||||
tool::{TICKET_TOOL_NAMES, TicketToolBackend, ticket_tool_description, ticket_tools},
|
||||
};
|
||||
|
||||
use super::merge_request;
|
||||
use crate::feature::{
|
||||
FeatureDescriptor, FeatureDiagnostic, FeatureInstallContext, FeatureInstallError,
|
||||
FeatureInstructionContribution, FeatureInstructionDeclaration, FeatureInstructionId,
|
||||
@@ -588,22 +587,6 @@ impl FeatureModule for TicketFeature {
|
||||
ticket_tool_description(name, self.record_language.as_deref()),
|
||||
));
|
||||
}
|
||||
if let TicketFeatureBackend::WorkspaceClient(client) = &self.backend {
|
||||
let names: Vec<&str> = if client.reviewer_context().is_some() {
|
||||
vec![
|
||||
"MergeRequestShow",
|
||||
merge_request::MERGE_REQUEST_REVIEW_TOOL_NAME,
|
||||
]
|
||||
} else {
|
||||
merge_request::MERGE_REQUEST_COMMON_TOOL_NAMES.to_vec()
|
||||
};
|
||||
for name in names {
|
||||
descriptor = descriptor.with_tool(ToolDeclaration::new(
|
||||
name,
|
||||
merge_request::description(name).unwrap_or("Merge Request operation."),
|
||||
));
|
||||
}
|
||||
}
|
||||
descriptor
|
||||
}
|
||||
|
||||
@@ -661,17 +644,6 @@ impl FeatureModule for TicketFeature {
|
||||
};
|
||||
tools.register(ToolContribution::new(name, definition))?;
|
||||
}
|
||||
if let TicketFeatureBackend::WorkspaceClient(client) = &self.backend {
|
||||
let definitions = if client.reviewer_context().is_some() {
|
||||
merge_request::reviewer_tools(client.clone())
|
||||
} else {
|
||||
merge_request::common_tools(client.clone())
|
||||
};
|
||||
for definition in definitions {
|
||||
let (meta, _) = definition();
|
||||
tools.register(ToolContribution::new(meta.name.clone(), definition))?;
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
||||
@@ -527,6 +527,39 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn merge_request_instruction_matches_the_exposed_operations() {
|
||||
let catalog = PromptCatalog::builtins_only().unwrap();
|
||||
let coder = catalog
|
||||
.render_name(
|
||||
"common.merge_request",
|
||||
Value::from_serialize(serde_json::json!({
|
||||
"tools": ["MergeRequestShow", "MergeRequestOpen"]
|
||||
})),
|
||||
)
|
||||
.unwrap();
|
||||
assert!(coder.contains("Reread the current Merge Request"));
|
||||
assert!(coder.contains("Open the Merge Request only after"));
|
||||
assert!(!coder.contains("Submit the authoritative verdict"));
|
||||
assert!(!coder.contains("Complete integration only after"));
|
||||
|
||||
let orchestrator = catalog
|
||||
.render_name(
|
||||
"common.merge_request",
|
||||
Value::from_serialize(serde_json::json!({
|
||||
"tools": [
|
||||
"MergeRequestShow",
|
||||
"MergeRequestReadinessCheck",
|
||||
"MergeRequestComplete"
|
||||
]
|
||||
})),
|
||||
)
|
||||
.unwrap();
|
||||
assert!(orchestrator.contains("Use `MergeRequestReadinessCheck`"));
|
||||
assert!(orchestrator.contains("Complete integration only after"));
|
||||
assert!(!orchestrator.contains("Open the Merge Request only after"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn commit_capable_roles_classify_commits_by_change_type() {
|
||||
let catalog = PromptCatalog::builtins_only().unwrap();
|
||||
|
||||
@@ -319,6 +319,7 @@ fn append_trailing_section(
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use crate::feature::FeatureInstructionId;
|
||||
use chrono::TimeZone;
|
||||
use manifest::{Permission, ScopeConfig, ScopeRule};
|
||||
use tempfile::TempDir;
|
||||
@@ -400,6 +401,48 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn merge_request_role_prompts_match_operation_specific_tool_surfaces() {
|
||||
fn render(role: &str, tools: &[&str]) -> String {
|
||||
let tmp = TempDir::new().unwrap();
|
||||
let scope = build_scope(tmp.path());
|
||||
let prompts = PromptCatalog::builtins_only().unwrap();
|
||||
let template =
|
||||
SystemPromptTemplate::parse(role, PromptCatalogSource::builtins_only()).unwrap();
|
||||
let instruction = FeatureInstructionDeclaration::new(
|
||||
FeatureInstructionId::builtin("merge_request.workflow"),
|
||||
"common.merge_request",
|
||||
"Merge Request workflow",
|
||||
)
|
||||
.unwrap();
|
||||
let mut ctx = context(tmp.path(), &scope, &prompts);
|
||||
ctx.tool_names = tools.iter().map(|name| (*name).to_string()).collect();
|
||||
ctx.feature_instructions = std::slice::from_ref(&instruction);
|
||||
template.render(&ctx).unwrap()
|
||||
}
|
||||
|
||||
let coder = render("role.coder", &["MergeRequestShow", "MergeRequestOpen"]);
|
||||
assert!(coder.contains("Open the Merge Request only after"));
|
||||
assert!(!coder.contains("Complete integration only after"));
|
||||
assert!(coder.contains("Do not call `MergeRequestComplete`"));
|
||||
|
||||
let reviewer = render("role.reviewer", &["MergeRequestShow", "MergeRequestReview"]);
|
||||
assert!(reviewer.contains("Submit the authoritative verdict"));
|
||||
assert!(!reviewer.contains("Open the Merge Request only after"));
|
||||
|
||||
let orchestrator = render(
|
||||
"role.orchestrator",
|
||||
&[
|
||||
"MergeRequestShow",
|
||||
"MergeRequestReadinessCheck",
|
||||
"MergeRequestComplete",
|
||||
],
|
||||
);
|
||||
assert!(orchestrator.contains("Use `MergeRequestReadinessCheck`"));
|
||||
assert!(orchestrator.contains("Complete integration only after"));
|
||||
assert!(!orchestrator.contains("Submit the authoritative verdict"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn role_templates_are_selected_without_filesystem_resolution() {
|
||||
let loader = PromptCatalogSource::builtins_only();
|
||||
|
||||
@@ -5890,6 +5890,13 @@ model_id = "claude-sonnet-4-20250514"
|
||||
[engine]
|
||||
instruction = "saved"
|
||||
|
||||
[feature.merge_request]
|
||||
show = true
|
||||
open = false
|
||||
review = true
|
||||
readiness_check = false
|
||||
complete = false
|
||||
|
||||
[[scope.allow]]
|
||||
target = "/snapshot/workspace"
|
||||
permission = "read"
|
||||
@@ -5912,6 +5919,13 @@ model_id = "claude-sonnet-4-20250514"
|
||||
[engine]
|
||||
instruction = "current"
|
||||
|
||||
[feature.merge_request]
|
||||
show = true
|
||||
open = true
|
||||
review = true
|
||||
readiness_check = true
|
||||
complete = true
|
||||
|
||||
[[scope.allow]]
|
||||
target = "/current/workspace"
|
||||
permission = "write"
|
||||
@@ -5931,6 +5945,14 @@ permission = "write"
|
||||
.unwrap();
|
||||
|
||||
assert_eq!(restored.engine.instruction, "saved");
|
||||
assert_eq!(
|
||||
restored.feature.merge_request,
|
||||
manifest::MergeRequestFeatureConfig {
|
||||
show: true,
|
||||
review: true,
|
||||
..Default::default()
|
||||
}
|
||||
);
|
||||
assert_eq!(restored.scope.allow.len(), 1);
|
||||
assert_eq!(
|
||||
restored.scope.allow[0].target,
|
||||
|
||||
Reference in New Issue
Block a user