Hare/yoi
1
1
Fork 0
Code Actions Packages Releases Activity

merge: integrate selector-thread merge request authority

Browse Source
This commit is contained in:
Hare
2026-08-17 10:34:01 +09:00
parent 298c20012a 5f44b10ff9
commit 063f203efe
22 changed files with 2611 additions and 3932 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/worker/src/feature/builtin/manage_worker.rs
+1 -129
View File
@@ -435,12 +435,6 @@ impl FeatureModule for ManageWorkerFeature {
WorkerOperation::Remove => {
definition::<WorkerRemoveInput>(operation, self.control.clone())
}
WorkerOperation::Share | WorkerOperation::Transfer => {
definition::<WorkerDelegateInput>(operation, self.control.clone())
}
WorkerOperation::Revoke => {
definition::<WorkerRevokeInput>(operation, self.control.clone())
}
};
context
.tools()
@@ -540,19 +534,6 @@ struct WorkerRemoveInput {
reason: String,
}
#[derive(Debug, Deserialize, JsonSchema)]
#[serde(deny_unknown_fields)]
struct WorkerRevokeInput {
grant_id: String,
}
#[derive(Debug, Deserialize, JsonSchema)]
#[serde(deny_unknown_fields)]
struct WorkerDelegateInput {
grant_id: String,
target_controller: WorkerSubjectInput,
}
struct WorkspaceWorkerTool {
operation: WorkerOperation,
control: Arc<dyn WorkerControlService>,
@@ -568,13 +549,10 @@ enum WorkerOperation {
Stop,
Restore,
Remove,
Share,
Transfer,
Revoke,
}
impl WorkerOperation {
const ALL: [Self; 11] = [
const ALL: [Self; 8] = [
Self::List,
Self::Spawn,
Self::SendInput,
@@ -583,9 +561,6 @@ impl WorkerOperation {
Self::Stop,
Self::Restore,
Self::Remove,
Self::Share,
Self::Transfer,
Self::Revoke,
];
fn tool_name(self) -> &'static str {
@@ -598,9 +573,6 @@ impl WorkerOperation {
Self::Stop => "WorkerStop",
Self::Restore => "WorkerRestore",
Self::Remove => "WorkerRemove",
Self::Share => "WorkerShare",
Self::Transfer => "WorkerTransfer",
Self::Revoke => "WorkerRevoke",
}
}
@@ -622,11 +594,6 @@ impl WorkerOperation {
Self::Remove => {
"Remove an eligible stopped, unassigned, non-internal Worker. Supply the current Worker revision and a bounded reason; Backend validation and retention are authoritative."
}
Self::Share => "Share one controlled Runtime Worker with another known Runtime Worker.",
Self::Transfer => {
"Transfer one controlled Runtime Worker to another known Runtime Worker."
}
Self::Revoke => "Revoke one durable Runtime Worker control grant owned by this Worker.",
}
}
}
@@ -792,59 +759,6 @@ impl Tool for WorkspaceWorkerTool {
)
.map_err(control_tool_error)?
}
WorkerOperation::Share | WorkerOperation::Transfer => {
let input = parse::<WorkerDelegateInput>(input_json, self.operation.tool_name())?;
let grant_id = authority_id(&input.grant_id, "grant_id")?;
let (runtime_id, worker_id) =
runtime_subject_ids(&input.target_controller, self.operation)?;
let operation_id = format!(
"worker-control-{}:{}",
if self.operation == WorkerOperation::Transfer {
"transfer"
} else {
"share"
},
non_empty(ctx.call_id.clone(), "tool call_id")?
);
let action = if self.operation == WorkerOperation::Transfer {
"transfer"
} else {
"share"
};
self.control
.execute_runtime(WorkspaceRequest::json(
WorkspaceRequestMethod::Post,
format!(
"/api/w/{}/worker-control/grants/{grant_id}/{action}",
self.control.workspace_id()
),
serde_json::json!({
"target_controller": {
"runtime_id": runtime_id,
"worker_id": worker_id,
},
"operation_id": operation_id,
})
.to_string(),
))
.await
.map_err(control_tool_error)?
}
WorkerOperation::Revoke => {
let input = parse::<WorkerRevokeInput>(input_json, "WorkerRevoke")?;
let grant_id = authority_id(&input.grant_id, "grant_id")?;
self.control
.execute_runtime(WorkspaceRequest::json(
WorkspaceRequestMethod::Post,
format!(
"/api/w/{}/worker-control/grants/{grant_id}/revoke",
self.control.workspace_id()
),
"{}",
))
.await
.map_err(control_tool_error)?
}
};
tool_output(self.operation, response)
}
@@ -1131,9 +1045,6 @@ mod tests {
"WorkerStop",
"WorkerRestore",
"WorkerRemove",
"WorkerShare",
"WorkerTransfer",
"WorkerRevoke",
]
);
}
@@ -1248,45 +1159,6 @@ mod tests {
}
}
#[tokio::test]
async fn worker_share_and_transfer_use_typed_runtime_subjects_and_operation_ids() {
let client = Arc::new(RecordingWorkspaceClient::default());
for (operation, action) in [
(WorkerOperation::Share, "share"),
(WorkerOperation::Transfer, "transfer"),
] {
WorkspaceWorkerTool {
operation,
control: test_control(client.clone()),
}
.execute(
&serde_json::json!({
"grant_id": "grant-1",
"target_controller": {
"kind": "runtime_worker",
"runtime_id": "runtime-2",
"worker_id": "worker-9",
},
})
.to_string(),
ToolExecutionContext::new("call-delegate", "batch-delegate", 0),
)
.await
.unwrap();
let request = client.requests.lock().unwrap().last().cloned().unwrap();
assert!(request.path.ends_with(&format!("/grant-1/{action}")));
let body: serde_json::Value =
serde_json::from_str(request.body.as_deref().unwrap()).unwrap();
assert_eq!(body["target_controller"]["runtime_id"], "runtime-2");
assert!(
body["operation_id"]
.as_str()
.unwrap()
.contains("call-delegate")
);
}
}
#[tokio::test]
async fn worker_remove_forwards_only_target_revision_and_bounded_reason() {
let client = Arc::new(RecordingWorkspaceClient::default());
crates/worker/src/feature/builtin/merge_request.rs
+84 -144
View File
@@ -11,7 +11,6 @@ pub const MERGE_REQUEST_COMMON_TOOL_NAMES: &[&str] = &[
"MergeRequestShow",
"MergeRequestReadinessCheck",
"MergeRequestOpen",
"MergeRequestAddRevision",
"MergeRequestComplete",
];
pub const MERGE_REQUEST_REVIEW_TOOL_NAME: &str = "MergeRequestReviewSubmit";
@@ -20,7 +19,6 @@ enum Kind {
Show,
Readiness,
Open,
AddRevision,
Complete,
Review,
}
@@ -29,7 +27,6 @@ struct MergeRequestTool {
client: Arc<dyn WorkspaceClient>,
kind: Kind,
}
#[derive(Debug, Deserialize, JsonSchema)]
struct ShowInput {
ticket: String,
@@ -38,25 +35,8 @@ struct ShowInput {
struct OpenInput {
ticket: String,
repository_id: String,
revision_id: String,
base_commit: String,
head_commit: String,
diff_digest: String,
#[serde(default)]
changed_paths: Vec<String>,
#[serde(default)]
summary: String,
}
#[derive(Debug, Deserialize, JsonSchema)]
struct AddRevisionInput {
ticket: String,
expected_current_revision_id: String,
revision_id: String,
base_commit: String,
head_commit: String,
diff_digest: String,
#[serde(default)]
changed_paths: Vec<String>,
selector_from: String,
selector_to: String,
#[serde(default)]
summary: String,
}
@@ -64,10 +44,9 @@ struct AddRevisionInput {
struct CompleteInput {
ticket: String,
operation_id: String,
expected_revision_id: String,
target_commit: String,
source_commit: String,
result_commit: String,
approval_event_id: String,
target_ref_before: String,
target_ref_after: String,
strategy: MergeStrategyInput,
resolution: MergeResolutionInput,
}
@@ -106,63 +85,48 @@ struct ReviewFindingInput {
#[serde(default)]
path: Option<String>,
#[serde(default)]
line: Option<u64>,
line: Option<u32>,
body: String,
}
impl Kind {
fn name(self) -> &'static str {
match self {
Self::Show => "MergeRequestShow",
Self::Readiness => "MergeRequestReadinessCheck",
Self::Open => "MergeRequestOpen",
Self::AddRevision => "MergeRequestAddRevision",
Self::Complete => "MergeRequestComplete",
Self::Review => "MergeRequestReviewSubmit",
}
}
fn description(self) -> &'static str {
description(self.name()).unwrap_or("Merge Request operation.")
}
fn schema(self) -> serde_json::Value {
match self {
Self::Show | Self::Readiness => json!(schemars::schema_for!(ShowInput)),
Self::Open => json!(schemars::schema_for!(OpenInput)),
Self::AddRevision => json!(schemars::schema_for!(AddRevisionInput)),
Self::Complete => json!(schemars::schema_for!(CompleteInput)),
Self::Review => json!(schemars::schema_for!(ReviewInput)),
}
}
}
#[async_trait]
impl Tool for MergeRequestTool {
async fn execute(
&self,
input: &str,
_context: ToolExecutionContext,
) -> Result<ToolOutput, ToolError> {
let workspace_id = self.client.workspace_id().ok_or_else(|| {
async fn execute(&self, input: &str, _: ToolExecutionContext) -> Result<ToolOutput, ToolError> {
let ws = self.client.workspace_id().ok_or_else(|| {
ToolError::ExecutionFailed("Merge Request tools require Workspace identity".into())
})?;
let (method, path, body) = match self.kind {
Kind::Show => {
let v: ShowInput = parse(input)?;
nonempty(&v.ticket)?;
(
WorkspaceRequestMethod::Get,
format!("/api/w/{workspace_id}/tickets/{}/merge-request", v.ticket),
None,
)
}
Kind::Readiness => {
Kind::Show | Kind::Readiness => {
let v: ShowInput = parse(input)?;
nonempty(&v.ticket)?;
(
WorkspaceRequestMethod::Get,
format!(
"/api/w/{workspace_id}/tickets/{}/merge-request/readiness",
v.ticket
"/api/w/{ws}/tickets/{}/merge-request{}",
v.ticket,
if matches!(self.kind, Kind::Readiness) {
"/readiness"
} else {
""
}
),
None,
)
@@ -172,62 +136,35 @@ impl Tool for MergeRequestTool {
nonempty(&v.ticket)?;
(
WorkspaceRequestMethod::Post,
format!("/api/w/{workspace_id}/tickets/{}/merge-request", v.ticket),
format!("/api/w/{ws}/tickets/{}/merge-request", v.ticket),
Some(
json!({"repository_id":v.repository_id,"revision_id":v.revision_id,"base_commit":v.base_commit,"head_commit":v.head_commit,"diff_digest":v.diff_digest,"changed_paths":v.changed_paths,"summary":v.summary}),
),
)
}
Kind::AddRevision => {
let v: AddRevisionInput = parse(input)?;
nonempty(&v.ticket)?;
(
WorkspaceRequestMethod::Post,
format!(
"/api/w/{workspace_id}/tickets/{}/merge-request/revisions",
v.ticket
),
Some(
json!({"expected_current_revision_id":v.expected_current_revision_id,"revision_id":v.revision_id,"base_commit":v.base_commit,"head_commit":v.head_commit,"diff_digest":v.diff_digest,"changed_paths":v.changed_paths,"summary":v.summary}),
json!({"repository_id":v.repository_id,"selector_from":v.selector_from,"selector_to":v.selector_to,"summary":v.summary}),
),
)
}
Kind::Complete => {
let v: CompleteInput = parse(input)?;
nonempty(&v.ticket)?;
let strategy = match v.strategy {
MergeStrategyInput::FastForward => "fast_forward",
MergeStrategyInput::Merge => "merge",
};
let resolution = match v.resolution {
MergeResolutionInput::None => "none",
MergeResolutionInput::Clean => "clean",
MergeResolutionInput::ConflictsResolved => "conflicts_resolved",
};
(
WorkspaceRequestMethod::Post,
format!(
"/api/w/{workspace_id}/tickets/{}/merge-request/complete",
v.ticket
),
format!("/api/w/{ws}/tickets/{}/merge-request/complete", v.ticket),
Some(
json!({"operation_id":v.operation_id,"expected_revision_id":v.expected_revision_id,"target_commit":v.target_commit,"source_commit":v.source_commit,"result_commit":v.result_commit,"strategy":strategy,"resolution":resolution}),
json!({"operation_id":v.operation_id,"approval_event_id":v.approval_event_id,"target_ref_before":v.target_ref_before,"target_ref_after":v.target_ref_after,"strategy":match v.strategy{MergeStrategyInput::FastForward=>"fast_forward",MergeStrategyInput::Merge=>"merge"},"resolution":match v.resolution{MergeResolutionInput::None=>"none",MergeResolutionInput::Clean=>"clean",MergeResolutionInput::ConflictsResolved=>"conflicts_resolved"}}),
),
)
}
Kind::Review => {
let v: ReviewInput = parse(input)?;
let context = self.client.reviewer_attempt_context().ok_or_else(|| {
let ctx = self.client.reviewer_context().ok_or_else(|| {
ToolError::ExecutionFailed(
"MergeRequestReviewSubmit is available only to an attested Reviewer child"
.into(),
"Review submit requires injected Reviewer capability".into(),
)
})?;
(
WorkspaceRequestMethod::Post,
format!(
"/api/w/{workspace_id}/tickets/{}/merge-request/reviews",
context.ticket_id
"/api/w/{ws}/tickets/{}/merge-request/reviews",
ctx.ticket_id
),
Some(
json!({"decision":match v.decision{ReviewDecisionInput::Approve=>"approve",ReviewDecisionInput::RequestChanges=>"request_changes"},"body":v.body,"findings":v.findings.into_iter().map(|f|json!({"severity":f.severity,"code":f.code,"path":f.path,"line":f.line,"body":f.body})).collect::<Vec<_>>() }),
@@ -235,32 +172,32 @@ impl Tool for MergeRequestTool {
)
}
};
let request = match body {
Some(body) => WorkspaceRequest::json(method, path, body.to_string()),
let req = match body {
Some(v) => WorkspaceRequest::json(method, path, v.to_string()),
None => WorkspaceRequest::get(path),
};
let response = self
let res = self
.client
.execute(request)
.execute(req)
.map_err(|e| ToolError::ExecutionFailed(e.to_string()))?;
if !response.is_success() {
if !res.is_success() {
return Err(ToolError::ExecutionFailed(format!(
"Merge Request API returned HTTP {}: {}",
response.status, response.body
res.status, res.body
)));
}
Ok(ToolOutput {
summary: self.kind.name().to_string(),
content: Some(response.body),
attachments: Vec::new(),
summary: self.kind.name().into(),
content: Some(res.body),
attachments: vec![],
})
}
}
fn parse<T: serde::de::DeserializeOwned>(value: &str) -> Result<T, ToolError> {
serde_json::from_str(value).map_err(|e| ToolError::InvalidArgument(e.to_string()))
fn parse<T: serde::de::DeserializeOwned>(v: &str) -> Result<T, ToolError> {
serde_json::from_str(v).map_err(|e| ToolError::InvalidArgument(e.to_string()))
}
fn nonempty(value: &str) -> Result<(), ToolError> {
if value.trim().is_empty() {
fn nonempty(v: &str) -> Result<(), ToolError> {
if v.trim().is_empty() {
Err(ToolError::InvalidArgument(
"ticket must not be empty".into(),
))
@@ -270,72 +207,75 @@ fn nonempty(value: &str) -> Result<(), ToolError> {
}
fn definition(client: Arc<dyn WorkspaceClient>, kind: Kind) -> ToolDefinition {
Arc::new(move || {
let meta = ToolMeta::new(kind.name())
.description(kind.description())
.input_schema(kind.schema());
let tool: Arc<dyn Tool> = Arc::new(MergeRequestTool {
client: client.clone(),
kind,
});
(meta, tool)
(
ToolMeta::new(kind.name())
.description(description(kind.name()).unwrap_or("Merge Request operation."))
.input_schema(kind.schema()),
Arc::new(MergeRequestTool {
client: client.clone(),
kind,
}) as Arc<dyn Tool>,
)
})
}
pub fn common_tools(client: Arc<dyn WorkspaceClient>) -> Vec<ToolDefinition> {
pub fn common_tools(c: Arc<dyn WorkspaceClient>) -> Vec<ToolDefinition> {
vec![
definition(client.clone(), Kind::Show),
definition(client.clone(), Kind::Readiness),
definition(client.clone(), Kind::Open),
definition(client.clone(), Kind::AddRevision),
definition(client, Kind::Complete),
definition(c.clone(), Kind::Show),
definition(c.clone(), Kind::Readiness),
definition(c.clone(), Kind::Open),
definition(c, Kind::Complete),
]
}
pub fn reviewer_tools(client: Arc<dyn WorkspaceClient>) -> Vec<ToolDefinition> {
if client.reviewer_attempt_context().is_some() {
pub fn reviewer_tools(c: Arc<dyn WorkspaceClient>) -> Vec<ToolDefinition> {
if c.reviewer_context().is_some() {
vec![
definition(client.clone(), Kind::Show),
definition(client, Kind::Review),
definition(c.clone(), Kind::Show),
definition(c, Kind::Review),
]
} else {
Vec::new()
vec![]
}
}
pub fn description(name: &str) -> Option<&'static str> {
match name {
"MergeRequestShow" => Some(
"Read the authoritative Merge Request, immutable current revision, and structured review status.",
),
pub fn description(n: &str) -> Option<&'static str> {
match n {
"MergeRequestShow" => Some("Read the selector-based Merge Request and append-only thread."),
"MergeRequestReadinessCheck" => {
Some("Check derived merge readiness for the current immutable revision.")
Some("Resolve current provider refs and derive readiness from valid review events.")
}
"MergeRequestOpen" => {
Some("Open an immutable Merge Request revision for the current assigned Coder.")
}
"MergeRequestAddRevision" => {
Some("Append an immutable revision; prior approval cannot carry to the new revision.")
Some("Open a Merge Request with immutable source and target selectors.")
}
"MergeRequestComplete" => {
Some("CAS-complete an approved revision with operation-id replay and crash fencing.")
Some("Complete using an approved review event and final target-ref evidence.")
}
"MergeRequestReviewSubmit" => {
Some("Submit the injected Reviewer capability result for its captured subject ref.")
}
"MergeRequestReviewSubmit" => Some(
"Submit the attested direct-child Reviewer result bound to its immutable revision.",
),
_ => None,
}
}
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn merge_request_tool_contract_omits_tree_hashes_and_candidate_result_tool() {
let open = serde_json::to_string(&schemars::schema_for!(OpenInput)).unwrap();
let add = serde_json::to_string(&schemars::schema_for!(AddRevisionInput)).unwrap();
let complete = serde_json::to_string(&schemars::schema_for!(CompleteInput)).unwrap();
assert!(!open.contains("head_tree"));
assert!(!add.contains("head_tree"));
assert!(complete.contains("result_commit"));
assert!(complete.contains("conflicts_resolved"));
assert!(!MERGE_REQUEST_COMMON_TOOL_NAMES.contains(&"MergeRequestRecordMergeResult"));
fn schemas_hide_revision_and_commit_authority() {
let schemas = [
schemars::schema_for!(OpenInput),
schemars::schema_for!(CompleteInput),
];
for s in schemas {
let j = serde_json::to_string(&s).unwrap();
for banned in [
"revision_id",
"attempt_id",
"base_commit",
"head_commit",
"source_commit",
"result_commit",
] {
assert!(!j.contains(banned), "{banned} in {j}")
}
}
assert!(!MERGE_REQUEST_COMMON_TOOL_NAMES.contains(&"MergeRequestRequestReview"));
}
}
crates/worker/src/feature/builtin/ticket.rs
+2 -2
View File
@@ -589,7 +589,7 @@ impl FeatureModule for TicketFeature {
));
}
if let TicketFeatureBackend::WorkspaceClient(client) = &self.backend {
let names: Vec<&str> = if client.reviewer_attempt_context().is_some() {
let names: Vec<&str> = if client.reviewer_context().is_some() {
vec![
"MergeRequestShow",
merge_request::MERGE_REQUEST_REVIEW_TOOL_NAME,
@@ -662,7 +662,7 @@ impl FeatureModule for TicketFeature {
tools.register(ToolContribution::new(name, definition))?;
}
if let TicketFeatureBackend::WorkspaceClient(client) = &self.backend {
let definitions = if client.reviewer_attempt_context().is_some() {
let definitions = if client.reviewer_context().is_some() {
merge_request::reviewer_tools(client.clone())
} else {
merge_request::common_tools(client.clone())
crates/worker/src/prompt/catalog.rs
+25
View File
@@ -527,6 +527,31 @@ mod tests {
);
}
#[test]
fn commit_capable_roles_classify_commits_by_change_type() {
let catalog = PromptCatalog::builtins_only().unwrap();
assert!(catalog.projection.templates.contains_key("common.git"));
let context = Value::from_serialize(serde_json::json!({
"cwd": "/workspace",
"date": "2026-08-16",
"language": "match the user's language",
"tool_capabilities": {
"memory_any": false,
"memory_mutation": false,
"memory_query": false,
"memory_read_document": false
}
}));
for prompt in ["default", "role.coder", "role.orchestrator"] {
let rendered = catalog.render_name(prompt, context.clone()).unwrap();
assert!(rendered.contains("use the change type as the subject prefix"));
assert!(rendered.contains("A change made because review"));
assert!(rendered.contains("Do not keep reusing a domain prefix"));
assert!(rendered.contains("fix: scope merge request foreign key checks"));
}
}
#[test]
fn builtin_render_resolves_catalog_root_dotted_includes() {
let catalog = PromptCatalog::builtins_only().unwrap();
crates/worker/src/spawn/tool.rs
+16 -22
View File
@@ -28,7 +28,7 @@ use crate::internal_worker::{
use crate::prompt::catalog::PromptCatalog;
use crate::spawn::registry::SpawnedWorkerRegistry;
use crate::worker::{
ReviewerAttemptContext, ReviewerChildWorkspaceClient, Worker, WorkerFilesystemAuthority,
ReviewerChildWorkspaceClient, ReviewerContext, Worker, WorkerFilesystemAuthority,
WorkspaceRequest, WorkspaceRequestMethod,
};
use protocol::Method;
@@ -58,8 +58,8 @@ struct SubWorkerSpawnInput {
/// spawner's explicit delegation authority; direct tool scope alone is not
/// sufficient. Omit `recursive` for normal workspace/worktree delegation; it defaults to true.
scope: Vec<ScopeRuleInput>,
/// Binds an actual read-only builtin Reviewer child to an immutable Merge Request revision.
/// Review attempt identity and capability material are generated by the trusted spawn layer.
/// Binds an actual read-only builtin Reviewer child to the current Merge Request candidate.
/// Review capability material is generated by the trusted spawn layer.
#[serde(default)]
review: Option<ReviewerHandoffInput>,
}
@@ -67,7 +67,6 @@ struct SubWorkerSpawnInput {
#[derive(Debug, Deserialize, schemars::JsonSchema)]
struct ReviewerHandoffInput {
ticket_id: String,
revision_id: String,
}
#[derive(Debug, Deserialize, schemars::JsonSchema)]
@@ -337,9 +336,9 @@ fn validate_reviewer_handoff(input: &SubWorkerSpawnInput) -> Result<(), ToolErro
let Some(review) = &input.review else {
return Ok(());
};
if review.ticket_id.trim().is_empty() || review.revision_id.trim().is_empty() {
if review.ticket_id.trim().is_empty() {
return Err(ToolError::InvalidArgument(
"reviewer handoff requires non-empty ticket_id and revision_id".to_string(),
"reviewer handoff requires non-empty ticket_id".to_string(),
));
}
if input.profile.as_deref() != Some("builtin:reviewer") {
@@ -418,11 +417,9 @@ impl Tool for SubWorkerSpawnTool {
.map_err(|error| {
ToolError::ExecutionFailed(format!("resolve child manifest: {error}"))
})?;
let reviewer_attempt = input.review.as_ref().map(|review| {
let reviewer_capability = input.review.as_ref().map(|review| {
(
review.ticket_id.clone(),
review.revision_id.clone(),
uuid::Uuid::now_v7().to_string(),
format!(
"{}{}",
uuid::Uuid::now_v7().simple(),
@@ -431,7 +428,7 @@ impl Tool for SubWorkerSpawnTool {
)
});
let child_workspace_context =
if let Some((ticket_id, revision_id, _, capability_token)) = &reviewer_attempt {
if let Some((ticket_id, capability_token)) = &reviewer_capability {
let workspace_id =
self.workspace_context
.workspace_id()
@@ -450,9 +447,8 @@ impl Tool for SubWorkerSpawnTool {
let child_client: Arc<dyn crate::worker::WorkspaceClient> =
Arc::new(ReviewerChildWorkspaceClient::new(
parent_client.clone(),
ReviewerAttemptContext {
ReviewerContext {
ticket_id: ticket_id.clone(),
revision_id: revision_id.clone(),
},
capability_token.clone(),
));
@@ -547,9 +543,9 @@ impl Tool for SubWorkerSpawnTool {
}
};
if let Some((ticket_id, revision_id, attempt_id, capability_token)) = &reviewer_attempt {
if let Some((ticket_id, capability_token)) = &reviewer_capability {
let workspace_id = self.workspace_context.workspace_id().ok_or_else(|| {
ToolError::ExecutionFailed("reviewer attempt lost Workspace identity".to_string())
ToolError::ExecutionFailed("review capability lost Workspace identity".to_string())
})?;
let child_session_id = session.session_id_string();
let child_registration = WorkspaceRequest::json(
@@ -577,15 +573,13 @@ impl Tool for SubWorkerSpawnTool {
)));
}
let body = serde_json::json!({
"attempt_id": attempt_id,
"revision_id": revision_id,
"child_session_id": child_session_id,
"capability_token": capability_token,
});
let request = WorkspaceRequest::json(
WorkspaceRequestMethod::Post,
format!(
"/api/w/{}/tickets/{}/merge-request/review-attempts",
"/api/w/{}/tickets/{}/merge-request/review-capabilities",
workspace_id.as_str(),
ticket_id
),
@@ -596,12 +590,12 @@ impl Tool for SubWorkerSpawnTool {
.client()
.execute(request)
.map_err(|error| {
ToolError::ExecutionFailed(format!("register reviewer attempt: {error}"))
ToolError::ExecutionFailed(format!("register review capability: {error}"))
})?;
if !response.is_success() {
let _ = session.stop().await;
return Err(ToolError::ExecutionFailed(format!(
"register reviewer attempt failed with status {}: {}",
"register review capability failed with status {}: {}",
response.status, response.body
)));
}
@@ -1044,21 +1038,21 @@ mod tests {
let valid: SubWorkerSpawnInput = serde_json::from_value(serde_json::json!({
"name":"reviewer","task":"review","profile":"builtin:reviewer",
"scope":[{"target":"/tmp/work","permission":"read"}],
"review":{"ticket_id":"T1","revision_id":"V1"}
"review":{"ticket_id":"T1"}
}))
.unwrap();
assert!(validate_reviewer_handoff(&valid).is_ok());
let wrong_profile: SubWorkerSpawnInput = serde_json::from_value(serde_json::json!({
"name":"reviewer","task":"review","profile":"builtin:coder",
"scope":[{"target":"/tmp/work","permission":"read"}],
"review":{"ticket_id":"T1","revision_id":"V1"}
"review":{"ticket_id":"T1"}
}))
.unwrap();
assert!(validate_reviewer_handoff(&wrong_profile).is_err());
let writable: SubWorkerSpawnInput = serde_json::from_value(serde_json::json!({
"name":"reviewer","task":"review","profile":"builtin:reviewer",
"scope":[{"target":"/tmp/work","permission":"write"}],
"review":{"ticket_id":"T1","revision_id":"V1"}
"review":{"ticket_id":"T1"}
}))
.unwrap();
assert!(validate_reviewer_handoff(&writable).is_err());
crates/worker/src/worker.rs
+7 -13
View File
@@ -238,30 +238,29 @@ pub trait WorkspaceClient: std::fmt::Debug + Send + Sync {
))
}
/// Trusted review-attempt context is injected by the Internal SubWorker spawn layer.
/// Trusted review capability context is injected by the Internal SubWorker spawn layer.
/// It is never accepted from a model-visible tool argument.
fn reviewer_attempt_context(&self) -> Option<&ReviewerAttemptContext> {
fn reviewer_context(&self) -> Option<&ReviewerContext> {
None
}
}
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct ReviewerAttemptContext {
pub struct ReviewerContext {
pub ticket_id: String,
pub revision_id: String,
}
#[derive(Debug)]
pub struct ReviewerChildWorkspaceClient {
inner: Arc<dyn WorkspaceClient>,
context: ReviewerAttemptContext,
context: ReviewerContext,
capability_token: String,
}
impl ReviewerChildWorkspaceClient {
pub fn new(
inner: Arc<dyn WorkspaceClient>,
context: ReviewerAttemptContext,
context: ReviewerContext,
capability_token: String,
) -> Self {
Self {
@@ -282,7 +281,7 @@ impl WorkspaceClient for ReviewerChildWorkspaceClient {
fn is_available(&self) -> bool {
self.inner.is_available()
}
fn reviewer_attempt_context(&self) -> Option<&ReviewerAttemptContext> {
fn reviewer_context(&self) -> Option<&ReviewerContext> {
Some(&self.context)
}
@@ -306,10 +305,6 @@ impl WorkspaceClient for ReviewerChildWorkspaceClient {
"review submission body must be an object".to_string(),
)
})?;
object.insert(
"revision_id".to_string(),
serde_json::Value::String(self.context.revision_id.clone()),
);
object.insert(
"capability_token".to_string(),
serde_json::Value::String(self.capability_token.clone()),
@@ -450,9 +445,8 @@ mod reviewer_client_tests {
});
let client = ReviewerChildWorkspaceClient::new(
inner,
ReviewerAttemptContext {
ReviewerContext {
ticket_id: "T1".into(),
revision_id: "V1".into(),
},
"secret".into(),
);