merge: integrate orchestration merge request lifecycle

# Conflicts:
#	resources/flows/coder-review.dcdl
This commit is contained in:
2026-08-12 18:30:08 +09:00
61 changed files with 13864 additions and 1248 deletions
+29 -4
View File
@@ -211,7 +211,7 @@ impl WorkerController {
C: LlmClient + Clone + 'static,
St: Store + WorkerMetadataStore + Clone + Send + Sync + 'static,
{
Self::spawn_inner(worker, runtime_base, false).await
Self::spawn_inner(worker, runtime_base, false, None).await
}
/// Spawn a Worker owned by `worker-runtime`.
@@ -227,20 +227,37 @@ impl WorkerController {
C: LlmClient + Clone + 'static,
St: Store + WorkerMetadataStore + Clone + Send + Sync + 'static,
{
Self::spawn_inner(worker, runtime_base, true).await
Self::spawn_inner(worker, runtime_base, true, None).await
}
/// Spawn into an exact persistent `runs/<generation>` directory.
pub async fn spawn_runtime_managed_run<C, St>(
worker: Worker<C, St>,
run_dir: &Path,
) -> Result<(WorkerHandle, ShutdownReceiver), std::io::Error>
where
C: LlmClient + Clone + 'static,
St: Store + WorkerMetadataStore + Clone + Send + Sync + 'static,
{
let parent = run_dir
.parent()
.ok_or_else(|| std::io::Error::other("run path has no parent"))?;
Self::spawn_inner(worker, parent, true, Some(run_dir)).await
}
async fn spawn_inner<C, St>(
worker: Worker<C, St>,
runtime_base: &Path,
runtime_managed: bool,
runtime_run: Option<&Path>,
) -> Result<(WorkerHandle, ShutdownReceiver), std::io::Error>
where
C: LlmClient + Clone + 'static,
St: Store + WorkerMetadataStore + Clone + Send + Sync + 'static,
{
let session = worker.workdir_session().cloned();
let result = Self::spawn_initialized(worker, runtime_base, runtime_managed).await;
let result =
Self::spawn_initialized(worker, runtime_base, runtime_managed, runtime_run).await;
if result.is_err()
&& let Some(session) = session
&& let Err(error) = session.close().await
@@ -254,6 +271,7 @@ impl WorkerController {
mut worker: Worker<C, St>,
runtime_base: &Path,
runtime_managed: bool,
runtime_run: Option<&Path>,
) -> Result<(WorkerHandle, ShutdownReceiver), std::io::Error>
where
C: LlmClient + Clone + 'static,
@@ -273,7 +291,9 @@ impl WorkerController {
// the spawn-tool factories need its socket path, and before the
// initial status/history writes consume the greeting we build
// after registration is complete.
let runtime_dir = Arc::new(if runtime_managed {
let runtime_dir = Arc::new(if let Some(run_dir) = runtime_run {
RuntimeDir::create_worker_run(run_dir).await?
} else if runtime_managed {
RuntimeDir::create_transient(runtime_base, &worker.manifest().worker.name).await?
} else {
RuntimeDir::create(runtime_base, &worker.manifest().worker.name).await?
@@ -1298,6 +1318,11 @@ async fn controller_loop<C, St>(
}
}
drop(_socket_server);
if let Err(error) = runtime_dir.close_socket().await {
tracing::warn!(%error, "Worker runtime socket cleanup failed");
}
// Background memory jobs own extract/consolidate workers after a
// turn completes. Join them before closing the Workdir session so no
// Worker-owned task can outlive its operation attachment.
+1
View File
@@ -9,6 +9,7 @@ pub mod manage_workdir;
pub mod manage_worker;
pub mod memory;
pub mod memory_extract;
pub mod merge_request;
pub mod objective;
pub mod session_explore;
pub mod task;
@@ -77,6 +77,11 @@ impl FeatureModule for ManageWorkerFeature {
self.client.clone(),
workspace_id.clone(),
),
WorkerOperation::Remove => definition::<WorkerRemoveInput>(
operation,
self.client.clone(),
workspace_id.clone(),
),
};
context
.tools()
@@ -149,6 +154,15 @@ struct WorkerStopInput {
reason: Option<String>,
}
#[derive(Debug, Deserialize, JsonSchema)]
#[serde(deny_unknown_fields)]
struct WorkerRemoveInput {
runtime_id: String,
worker_id: String,
expected_worker_revision: String,
reason: String,
}
struct WorkspaceWorkerTool {
operation: WorkerOperation,
client: Arc<dyn WorkspaceClient>,
@@ -161,10 +175,17 @@ enum WorkerOperation {
Spawn,
Stop,
Restore,
Remove,
}
impl WorkerOperation {
const ALL: [Self; 4] = [Self::List, Self::Spawn, Self::Stop, Self::Restore];
const ALL: [Self; 5] = [
Self::List,
Self::Spawn,
Self::Stop,
Self::Restore,
Self::Remove,
];
fn tool_name(self) -> &'static str {
match self {
@@ -172,6 +193,7 @@ impl WorkerOperation {
Self::Spawn => "WorkerSpawn",
Self::Stop => "WorkerStop",
Self::Restore => "WorkerRestore",
Self::Remove => "WorkerRemove",
}
}
@@ -187,6 +209,9 @@ impl WorkerOperation {
Self::Restore => {
"Restore a stopped Backend/Runtime Worker session in the current Workspace."
}
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."
}
}
}
}
@@ -198,82 +223,107 @@ impl Tool for WorkspaceWorkerTool {
input_json: &str,
ctx: ToolExecutionContext,
) -> Result<ToolOutput, ToolError> {
let request = match self.operation {
WorkerOperation::List => {
parse::<WorkerListInput>(input_json, "WorkerList")?;
WorkspaceRequest::get(format!("/api/w/{}/workers", self.workspace_id))
let response = match self.operation {
WorkerOperation::Remove => {
let input = parse::<WorkerRemoveInput>(input_json, "WorkerRemove")?;
let runtime_id = authority_id(&input.runtime_id, "runtime_id")?;
let worker_id = authority_id(&input.worker_id, "worker_id")?;
let expected_worker_revision =
non_empty(input.expected_worker_revision, "expected_worker_revision")?;
let reason = non_empty(input.reason, "reason")?;
if reason.len() > 512 {
return Err(ToolError::ExecutionFailed(
"reason must contain at most 512 bytes".to_string(),
));
}
self.client
.execute_worker_remove(
&runtime_id,
&worker_id,
&expected_worker_revision,
&reason,
)
.map_err(|error| ToolError::ExecutionFailed(error.to_string()))?
}
WorkerOperation::Spawn => {
let input = parse::<WorkerSpawnInput>(input_json, "WorkerSpawn")?;
let ticket_assignment = input
.ticket_id
.map(|ticket_id| {
let ticket_id = authority_id(&ticket_id, "ticket_id")?;
let call_id = non_empty(ctx.call_id.clone(), "tool call_id")?;
Ok::<_, ToolError>(WorkerSpawnTicketAssignmentRequest {
operation_id: format!("worker-spawn:{ticket_id}:{call_id}"),
ticket_id,
})
})
.transpose()?;
let request = WorkerSpawnRequest {
runtime_id: authority_id(&input.runtime_id, "runtime_id")?,
display_name: input
.display_name
.filter(|value| !value.trim().is_empty())
.unwrap_or_else(|| "Workspace Worker".to_string()),
profile: non_empty(input.profile, "profile")?,
ticket_assignment,
initial_submit: input.initial_submit,
working_directory: WorkerWorkingDirectorySelection {
working_directory_id: authority_id(
&input.working_directory_id,
"working_directory_id",
)?,
relative_cwd: input
.relative_cwd
.map(|value| validate_relative_cwd(&value))
.transpose()?,
},
operation => {
let request = match operation {
WorkerOperation::List => {
parse::<WorkerListInput>(input_json, "WorkerList")?;
WorkspaceRequest::get(format!("/api/w/{}/workers", self.workspace_id))
}
WorkerOperation::Spawn => {
let input = parse::<WorkerSpawnInput>(input_json, "WorkerSpawn")?;
let ticket_assignment = input
.ticket_id
.map(|ticket_id| {
let ticket_id = authority_id(&ticket_id, "ticket_id")?;
let call_id = non_empty(ctx.call_id.clone(), "tool call_id")?;
Ok::<_, ToolError>(WorkerSpawnTicketAssignmentRequest {
operation_id: format!("worker-spawn:{ticket_id}:{call_id}"),
ticket_id,
})
})
.transpose()?;
let request = WorkerSpawnRequest {
runtime_id: authority_id(&input.runtime_id, "runtime_id")?,
display_name: input
.display_name
.filter(|value| !value.trim().is_empty())
.unwrap_or_else(|| "Workspace Worker".to_string()),
profile: non_empty(input.profile, "profile")?,
ticket_assignment,
initial_submit: input.initial_submit,
working_directory: WorkerWorkingDirectorySelection {
working_directory_id: authority_id(
&input.working_directory_id,
"working_directory_id",
)?,
relative_cwd: input
.relative_cwd
.map(|value| validate_relative_cwd(&value))
.transpose()?,
},
};
WorkspaceRequest::json(
WorkspaceRequestMethod::Post,
format!("/api/w/{}/workers", self.workspace_id),
serde_json::to_string(&request)
.map_err(|error| ToolError::ExecutionFailed(error.to_string()))?,
)
}
WorkerOperation::Stop => {
let input = parse::<WorkerStopInput>(input_json, "WorkerStop")?;
let runtime_id = authority_id(&input.runtime_id, "runtime_id")?;
let worker_id = authority_id(&input.worker_id, "worker_id")?;
WorkspaceRequest::json(
WorkspaceRequestMethod::Post,
format!(
"/api/w/{}/runtimes/{runtime_id}/workers/{worker_id}/stop",
self.workspace_id
),
serde_json::json!({ "reason": input.reason }).to_string(),
)
}
WorkerOperation::Restore => {
let input = parse::<WorkerTargetInput>(input_json, "WorkerRestore")?;
let runtime_id = authority_id(&input.runtime_id, "runtime_id")?;
let worker_id = authority_id(&input.worker_id, "worker_id")?;
WorkspaceRequest::json(
WorkspaceRequestMethod::Post,
format!(
"/api/w/{}/runtimes/{runtime_id}/workers/{worker_id}/restore",
self.workspace_id
),
"{}",
)
}
WorkerOperation::Remove => unreachable!("handled above"),
};
WorkspaceRequest::json(
WorkspaceRequestMethod::Post,
format!("/api/w/{}/workers", self.workspace_id),
serde_json::to_string(&request)
.map_err(|error| ToolError::ExecutionFailed(error.to_string()))?,
)
}
WorkerOperation::Stop => {
let input = parse::<WorkerStopInput>(input_json, "WorkerStop")?;
let runtime_id = authority_id(&input.runtime_id, "runtime_id")?;
let worker_id = authority_id(&input.worker_id, "worker_id")?;
WorkspaceRequest::json(
WorkspaceRequestMethod::Post,
format!(
"/api/w/{}/runtimes/{runtime_id}/workers/{worker_id}/stop",
self.workspace_id
),
serde_json::json!({ "reason": input.reason }).to_string(),
)
}
WorkerOperation::Restore => {
let input = parse::<WorkerTargetInput>(input_json, "WorkerRestore")?;
let runtime_id = authority_id(&input.runtime_id, "runtime_id")?;
let worker_id = authority_id(&input.worker_id, "worker_id")?;
WorkspaceRequest::json(
WorkspaceRequestMethod::Post,
format!(
"/api/w/{}/runtimes/{runtime_id}/workers/{worker_id}/restore",
self.workspace_id
),
"{}",
)
self.client
.execute(request)
.map_err(|error| ToolError::ExecutionFailed(error.to_string()))?
}
};
let response = self
.client
.execute(request)
.map_err(|error| ToolError::ExecutionFailed(error.to_string()))?;
if !response.is_success() {
return Err(ToolError::ExecutionFailed(format!(
"Workspace Worker operation returned HTTP {}: {}",
@@ -356,6 +406,7 @@ mod tests {
#[derive(Debug, Default)]
struct RecordingWorkspaceClient {
requests: Mutex<Vec<WorkspaceRequest>>,
removals: Mutex<Vec<(String, String, String, String)>>,
}
impl WorkspaceClient for RecordingWorkspaceClient {
@@ -381,6 +432,25 @@ mod tests {
body: "{}".to_string(),
})
}
fn execute_worker_remove(
&self,
target_runtime_id: &str,
target_worker_id: &str,
expected_worker_revision: &str,
reason: &str,
) -> Result<WorkspaceResponse, WorkspaceClientError> {
self.removals.lock().unwrap().push((
target_runtime_id.to_string(),
target_worker_id.to_string(),
expected_worker_revision.to_string(),
reason.to_string(),
));
Ok(WorkspaceResponse {
status: 200,
body: r#"{"removed":true}"#.to_string(),
})
}
}
#[tokio::test]
@@ -433,7 +503,13 @@ mod tests {
fn worker_tool_family_is_distinct_from_sub_worker_tools() {
assert_eq!(
WorkerOperation::ALL.map(WorkerOperation::tool_name),
["WorkerList", "WorkerSpawn", "WorkerStop", "WorkerRestore"]
[
"WorkerList",
"WorkerSpawn",
"WorkerStop",
"WorkerRestore",
"WorkerRemove",
]
);
}
@@ -481,6 +557,92 @@ mod tests {
assert!(value.get("initial_text").is_none());
}
#[tokio::test]
async fn worker_remove_forwards_only_target_revision_and_bounded_reason() {
let client = Arc::new(RecordingWorkspaceClient::default());
let tool = WorkspaceWorkerTool {
operation: WorkerOperation::Remove,
client: client.clone(),
workspace_id: "workspace%2Ftest".to_string(),
};
tool.execute(
&serde_json::json!({
"runtime_id": "runtime-1",
"worker_id": "worker-7",
"expected_worker_revision": "2026-08-11T20:00:00Z",
"reason": " retire completed Worker "
})
.to_string(),
ToolExecutionContext::new("call-remove", "batch-remove", 0),
)
.await
.unwrap();
assert_eq!(
client.removals.lock().unwrap().as_slice(),
[(
"runtime-1".to_string(),
"worker-7".to_string(),
"2026-08-11T20:00:00Z".to_string(),
"retire completed Worker".to_string(),
)]
);
let schema = serde_json::to_value(schemars::schema_for!(WorkerRemoveInput))
.unwrap()
.to_string();
for field in [
"runtime_id",
"worker_id",
"expected_worker_revision",
"reason",
] {
assert!(schema.contains(field));
}
for forbidden in ["proof", "actor", "workspace_id", "policy", "plan", "stage"] {
assert!(!schema.contains(forbidden), "schema leaked {forbidden}");
}
}
#[tokio::test]
async fn worker_remove_rejects_empty_oversized_and_unknown_authority_input() {
let client = Arc::new(RecordingWorkspaceClient::default());
let tool = WorkspaceWorkerTool {
operation: WorkerOperation::Remove,
client: client.clone(),
workspace_id: "workspace%2Ftest".to_string(),
};
for reason in [" ".to_string(), "x".repeat(513)] {
let _error = tool
.execute(
&serde_json::json!({
"runtime_id": "runtime-1",
"worker_id": "worker-7",
"expected_worker_revision": "revision-1",
"reason": reason,
})
.to_string(),
ToolExecutionContext::new("call-invalid", "batch-remove", 0),
)
.await
.unwrap_err();
}
let _error = tool
.execute(
&serde_json::json!({
"runtime_id": "runtime-1",
"worker_id": "worker-7",
"expected_worker_revision": "revision-1",
"reason": "retire",
"source_proof": "caller-controlled"
})
.to_string(),
ToolExecutionContext::new("call-spoof", "batch-remove", 0),
)
.await
.unwrap_err();
assert!(client.removals.lock().unwrap().is_empty());
}
#[test]
fn worker_inputs_reject_paths_and_parent_traversal() {
assert!(authority_id("https://runtime.example", "runtime_id").is_err());
+1 -3
View File
@@ -346,11 +346,9 @@ mod tests {
use llm_engine::tool::ToolDefinition;
fn test_client() -> Arc<dyn WorkspaceClient> {
Arc::new(crate::worker::RuntimeWorkspaceHttpClient::new(
Arc::new(crate::worker::TestWorkspaceHttpClient::new(
"workspace",
"http://backend",
"test-runtime",
"test-worker",
))
}
@@ -0,0 +1,299 @@
use crate::feature::ToolDefinition;
use crate::worker::{WorkspaceClient, WorkspaceRequest, WorkspaceRequestMethod};
use async_trait::async_trait;
use llm_engine::tool::{Tool, ToolError, ToolExecutionContext, ToolMeta, ToolOutput};
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",
"MergeRequestAddRevision",
"MergeRequestComplete",
];
pub const MERGE_REQUEST_REVIEW_TOOL_NAME: &str = "MergeRequestReviewSubmit";
#[derive(Clone, Copy)]
enum Kind {
Show,
Readiness,
Open,
AddRevision,
Complete,
Review,
}
#[derive(Clone)]
struct MergeRequestTool {
client: Arc<dyn WorkspaceClient>,
kind: Kind,
}
#[derive(Debug, Deserialize, JsonSchema)]
struct ShowInput {
ticket: String,
}
#[derive(Debug, Deserialize, JsonSchema)]
struct OpenInput {
ticket: String,
repository_id: String,
revision_id: String,
base_commit: String,
head_commit: String,
head_tree: 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,
head_tree: String,
diff_digest: String,
#[serde(default)]
changed_paths: Vec<String>,
#[serde(default)]
summary: String,
}
#[derive(Debug, Deserialize, JsonSchema)]
struct CompleteInput {
ticket: String,
operation_id: String,
expected_revision_id: String,
}
#[derive(Debug, Deserialize, JsonSchema)]
struct ReviewInput {
decision: ReviewDecisionInput,
#[serde(default)]
body: String,
#[serde(default)]
findings: Vec<ReviewFindingInput>,
}
#[derive(Debug, Deserialize, JsonSchema)]
#[serde(rename_all = "snake_case")]
enum ReviewDecisionInput {
Approve,
RequestChanges,
}
#[derive(Debug, Deserialize, JsonSchema)]
struct ReviewFindingInput {
severity: String,
#[serde(default)]
code: Option<String>,
#[serde(default)]
path: Option<String>,
#[serde(default)]
line: Option<u64>,
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(|| {
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 => {
let v: ShowInput = parse(input)?;
nonempty(&v.ticket)?;
(
WorkspaceRequestMethod::Get,
format!(
"/api/w/{workspace_id}/tickets/{}/merge-request/readiness",
v.ticket
),
None,
)
}
Kind::Open => {
let v: OpenInput = parse(input)?;
nonempty(&v.ticket)?;
(
WorkspaceRequestMethod::Post,
format!("/api/w/{workspace_id}/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,"head_tree":v.head_tree,"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,"head_tree":v.head_tree,"diff_digest":v.diff_digest,"changed_paths":v.changed_paths,"summary":v.summary}),
),
)
}
Kind::Complete => {
let v: CompleteInput = parse(input)?;
nonempty(&v.ticket)?;
(
WorkspaceRequestMethod::Post,
format!(
"/api/w/{workspace_id}/tickets/{}/merge-request/complete",
v.ticket
),
Some(
json!({"operation_id":v.operation_id,"expected_revision_id":v.expected_revision_id}),
),
)
}
Kind::Review => {
let v: ReviewInput = parse(input)?;
let context = self.client.reviewer_attempt_context().ok_or_else(|| {
ToolError::ExecutionFailed(
"MergeRequestReviewSubmit is available only to an attested Reviewer child"
.into(),
)
})?;
(
WorkspaceRequestMethod::Post,
format!(
"/api/w/{workspace_id}/tickets/{}/merge-request/reviews",
context.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<_>>() }),
),
)
}
};
let request = match body {
Some(body) => WorkspaceRequest::json(method, path, body.to_string()),
None => WorkspaceRequest::get(path),
};
let response = self
.client
.execute(request)
.map_err(|e| ToolError::ExecutionFailed(e.to_string()))?;
if !response.is_success() {
return Err(ToolError::ExecutionFailed(format!(
"Merge Request API returned HTTP {}: {}",
response.status, response.body
)));
}
Ok(ToolOutput {
summary: self.kind.name().to_string(),
content: Some(response.body),
attachments: Vec::new(),
})
}
}
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 nonempty(value: &str) -> Result<(), ToolError> {
if value.trim().is_empty() {
Err(ToolError::InvalidArgument(
"ticket must not be empty".into(),
))
} else {
Ok(())
}
}
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)
})
}
pub fn common_tools(client: 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),
]
}
pub fn reviewer_tools(client: Arc<dyn WorkspaceClient>) -> Vec<ToolDefinition> {
if client.reviewer_attempt_context().is_some() {
vec![
definition(client.clone(), Kind::Show),
definition(client, Kind::Review),
]
} else {
Vec::new()
}
}
pub fn description(name: &str) -> Option<&'static str> {
match name {
"MergeRequestShow" => Some(
"Read the authoritative Merge Request, immutable current revision, and structured review status.",
),
"MergeRequestReadinessCheck" => {
Some("Check derived merge readiness for the current immutable revision.")
}
"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.")
}
"MergeRequestComplete" => {
Some("CAS-complete an approved revision with operation-id replay and crash fencing.")
}
"MergeRequestReviewSubmit" => Some(
"Submit the attested direct-child Reviewer result bound to its immutable revision.",
),
_ => None,
}
}
@@ -628,12 +628,7 @@ mod tests {
#[test]
fn workspace_http_objective_tools_include_objective_crud_tools() {
let names = tool_names(workspace_http_objective_tools(Arc::new(
crate::worker::RuntimeWorkspaceHttpClient::new(
"workspace",
"http://backend",
"test-runtime",
"test-worker",
),
crate::worker::TestWorkspaceHttpClient::new("workspace", "http://backend"),
)));
assert_eq!(
+35 -40
View File
@@ -14,12 +14,13 @@ use ticket::{
NewTicketRelation, OrchestrationPlanKind, OrchestrationPlanRecord, Result as TicketResult,
Ticket, TicketBackend, TicketBackendOperation, TicketBackendOperationResult,
TicketDoctorReport, TicketError, TicketIdOrSlug, TicketIntakeSummary, TicketListQuery,
TicketRef, TicketRelation, TicketRelationKind, TicketRelationView, TicketReview,
TicketStateChange, TicketSummary,
TicketRef, TicketRelation, TicketRelationKind, TicketRelationView, TicketStateChange,
TicketSummary,
config::{DEFAULT_TICKET_BACKEND_RELATIVE_PATH, TicketConfig},
tool::{TICKET_TOOL_NAMES, TicketToolBackend, ticket_tool_description, ticket_tools},
};
use super::merge_request;
use crate::feature::{
FeatureDescriptor, FeatureDiagnostic, FeatureInstallContext, FeatureInstallError,
FeatureInstructionContribution, FeatureInstructionDeclaration, FeatureInstructionId,
@@ -100,7 +101,7 @@ impl TicketFeatureAccess {
pub const fn review() -> Self {
Self {
authoring: false,
thread: true,
thread: false,
intake: false,
orchestration_control: false,
}
@@ -141,7 +142,7 @@ const AUTHORING_TOOL_NAMES: &[&str] = &[
"TicketRelationRecord",
];
const THREAD_TOOL_NAMES: &[&str] = &["TicketComment", "TicketReview"];
const THREAD_TOOL_NAMES: &[&str] = &["TicketComment"];
const INTAKE_TOOL_NAMES: &[&str] = &["TicketIntakeReady"];
@@ -152,7 +153,6 @@ const WORKSPACE_AUTHORING_TOOL_NAMES: &[&str] = &[
"TicketList",
"TicketShow",
"TicketComment",
"TicketReview",
"TicketQueue",
"TicketClose",
"TicketDependencyCheck",
@@ -167,7 +167,6 @@ const ORCHESTRATION_CONTROL_TOOL_NAMES: &[&str] = &[
"TicketList",
"TicketShow",
"TicketComment",
"TicketReview",
"TicketWorkflowState",
"TicketClose",
"TicketDependencyCheck",
@@ -340,6 +339,22 @@ 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_attempt_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
}
@@ -373,6 +388,17 @@ 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() {
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(())
}
}
@@ -611,14 +637,6 @@ impl WorkspaceHttpTicketBackend {
format!("{base}/{}/workflow/queue", Self::ticket_path(&id)),
None,
),
TicketBackendOperation::Review { id, review } => Self::request_unit(
client,
WorkspaceRequestMethod::Post,
format!("{base}/{}/workflow/review", Self::ticket_path(&id)),
Some(serde_json::to_value(review).map_err(|error| {
TicketError::Conflict(format!("serialize Ticket review: {error}"))
})?),
),
TicketBackendOperation::Close { id, resolution } => Self::request_unit(
client,
WorkspaceRequestMethod::Post,
@@ -844,15 +862,6 @@ impl TicketBackend for WorkspaceHttpTicketBackend {
}
}
fn review(&self, id: TicketIdOrSlug, review: TicketReview) -> TicketResult<()> {
match self.invoke(TicketBackendOperation::Review { id, review })? {
TicketBackendOperationResult::Unit => Ok(()),
other => Err(TicketError::Conflict(format!(
"unexpected ticket backend response: {other:?}"
))),
}
}
fn close(&self, id: TicketIdOrSlug, resolution: MarkdownText) -> TicketResult<()> {
match self.invoke(TicketBackendOperation::Close { id, resolution })? {
TicketBackendOperationResult::Unit => Ok(()),
@@ -1075,7 +1084,6 @@ mod tests {
.map(|tool| tool.name.as_str())
.collect::<Vec<_>>();
assert!(work_report_tools.contains(&"TicketComment"));
assert!(work_report_tools.contains(&"TicketReview"));
assert!(!work_report_tools.contains(&"TicketWorkflowState"));
let review = ticket_tools_feature_with_access(temp.path(), TicketFeatureAccess::review());
@@ -1085,7 +1093,6 @@ mod tests {
.iter()
.map(|tool| tool.name.as_str())
.collect::<Vec<_>>();
assert!(review_tools.contains(&"TicketReview"));
assert!(!review_tools.contains(&"TicketWorkflowState"));
}
@@ -1369,12 +1376,7 @@ provider = "github"
#[tokio::test(flavor = "multi_thread")]
async fn workspace_http_backend_invoke_is_safe_inside_async_context() {
let backend = WorkspaceHttpTicketBackend::new(Arc::new(
crate::worker::RuntimeWorkspaceHttpClient::new(
"workspace-a",
"not-a-url",
"test-runtime",
"test-worker",
),
crate::worker::TestWorkspaceHttpClient::new("workspace-a", "not-a-url"),
));
let error = backend
@@ -1405,11 +1407,9 @@ provider = "github"
.write_all(b"HTTP/1.1 204 No Content\r\nContent-Length: 0\r\n\r\n")
.unwrap();
});
let client = Arc::new(crate::worker::RuntimeWorkspaceHttpClient::new(
let client = Arc::new(crate::worker::TestWorkspaceHttpClient::new(
"workspace-a",
format!("http://{address}"),
"test-runtime",
"worker-a",
));
let backend = WorkspaceHttpTicketBackend::new(client);
@@ -1450,12 +1450,7 @@ provider = "github"
});
let backend = WorkspaceHttpTicketBackend::new(Arc::new(
crate::worker::RuntimeWorkspaceHttpClient::new(
"workspace-a",
base_url,
"test-runtime",
"test-worker",
),
crate::worker::TestWorkspaceHttpClient::new("workspace-a", base_url),
));
let created = backend.create(NewTicket::new("HTTP ticket")).unwrap();
+4
View File
@@ -259,6 +259,10 @@ pub(crate) struct InternalWorkerSessionHandle {
}
impl InternalWorkerSessionHandle {
pub(crate) fn session_id_string(&self) -> String {
self.session_id.to_string()
}
pub(crate) fn status(&self) -> InternalWorkerSessionStatus {
InternalWorkerSessionStatus::decode(self.status.load(std::sync::atomic::Ordering::Acquire))
}
+4 -4
View File
@@ -40,9 +40,9 @@ pub use runtime::dir::RuntimeDir;
pub use segment_log_sink::SegmentLogSink;
pub use shared_state::WorkerSharedState;
pub use worker::{
LocalWorkingDirectory, RuntimeWorkspaceHttpClient, WORKER_INPUT_SUBMISSION_EXTENSION_DOMAIN,
Worker, WorkerError, WorkerFilesystemAuthority, WorkerRunResult, WorkerWorkspaceContext,
WorkspaceClient, WorkspaceClientError, WorkspaceId, WorkspaceIdError, WorkspaceRequest,
WorkspaceRequestMethod, WorkspaceResponse, apply_worker_manifest, marker_workspace_client,
LocalWorkingDirectory, WORKER_INPUT_SUBMISSION_EXTENSION_DOMAIN, Worker, WorkerError,
WorkerFilesystemAuthority, WorkerRunResult, WorkerWorkspaceContext, WorkspaceClient,
WorkspaceClientError, WorkspaceId, WorkspaceIdError, WorkspaceRequest, WorkspaceRequestMethod,
WorkspaceResponse, apply_worker_manifest, marker_workspace_client,
unavailable_workspace_client,
};
+15
View File
@@ -746,6 +746,21 @@ compact_system = "PREFIX\n{% include \"$yoi/internal/compact_system\" %}"
assert!(rendered.contains("bypass user/Ticket authorization"));
}
#[test]
fn orchestrator_role_prompt_fences_worker_remove_authority() {
let source = include_str!("../../../../resources/prompts/role/orchestrator.md");
assert!(source.contains("Use `WorkerRemove` only for a terminal or authoritatively reassigned non-internal Coder"));
assert!(source.contains("exact current `updated_at` value"));
assert!(source.contains("must have no current Ticket assignment"));
assert!(source.contains("pending notification, Reviewer handoff, legal hold, or pin"));
assert!(source.contains("After removal, reread the Worker catalog and attachment state"));
assert!(source.contains("attachment-close, and attachment-release conflicts"));
assert!(source.contains("preserves the Workdir materialization"));
assert!(!source.contains("source proof"));
assert!(!source.contains("provider handle"));
assert!(!source.contains("retention plan"));
}
#[test]
fn sub_worker_spawn_tool_description_renders_profile_block() {
let cat = PromptCatalog::builtins_only().unwrap();
+47 -2
View File
@@ -42,6 +42,8 @@ pub struct SpawnedWorkerRecord {
pub struct RuntimeDir {
path: PathBuf,
write_legacy_snapshots: bool,
preserve_on_drop: bool,
socket_file_name: &'static str,
}
impl RuntimeDir {
@@ -56,6 +58,8 @@ impl RuntimeDir {
Ok(Self {
path,
write_legacy_snapshots: true,
preserve_on_drop: false,
socket_file_name: "sock",
})
}
@@ -69,6 +73,36 @@ impl RuntimeDir {
Ok(Self {
path,
write_legacy_snapshots: false,
preserve_on_drop: false,
socket_file_name: "sock",
})
}
/// Create an exact, persistent generation-scoped Worker run directory.
/// Existing directories are rejected so stale artifacts cannot be reused.
pub async fn create_worker_run(path: &Path) -> Result<Self, io::Error> {
let parent = path
.parent()
.ok_or_else(|| io::Error::other("run path has no parent"))?;
fs::create_dir_all(parent).await?;
fs::create_dir(path).await?;
fs::create_dir(path.join("artifacts")).await?;
fs::create_dir(path.join("spawned")).await?;
for log in ["worker.out.log", "worker.err.log"] {
let file = fs::OpenOptions::new()
.write(true)
.create_new(true)
.open(path.join(log))
.await?;
file.sync_all().await?;
}
std::fs::File::open(path)?.sync_all()?;
std::fs::File::open(parent)?.sync_all()?;
Ok(Self {
path: path.to_path_buf(),
write_legacy_snapshots: false,
preserve_on_drop: true,
socket_file_name: "worker.sock",
})
}
@@ -116,13 +150,24 @@ impl RuntimeDir {
/// that only know the worker name (e.g. the TUI's attach flow)
/// predict the same path via [`manifest::paths::worker_socket_path`].
pub fn socket_path(&self) -> PathBuf {
self.path.join("sock")
self.path.join(self.socket_file_name)
}
pub async fn close_socket(&self) -> Result<(), io::Error> {
match fs::remove_file(self.socket_path()).await {
Ok(()) => Ok(()),
Err(error) if error.kind() == io::ErrorKind::NotFound => Ok(()),
Err(error) => Err(error),
}
}
}
impl Drop for RuntimeDir {
fn drop(&mut self) {
let _ = std::fs::remove_dir_all(&self.path);
let _ = std::fs::remove_file(self.socket_path());
if !self.preserve_on_drop {
let _ = std::fs::remove_dir_all(&self.path);
}
}
}
@@ -120,15 +120,15 @@ pub fn adopt_allocation(
/// The Worker's in-memory `segment_id` can change underneath the
/// allocation in two normal places:
///
/// - `Worker::compact` mints a fresh session and swaps it in.
/// - `session_store::ensure_head_or_fork` auto-forks when another
/// - `Worker::compact` mints a fresh Segment in the same Session.
/// - `session_store::ensure_head_or_fork` auto-forks within that Session when another
/// writer has advanced the store head behind our back.
///
/// Both paths must call this so subsequent [`lookup_segment`] queries
/// find the live session id, not the old one. Without this update a
/// find the live Segment id, not the old one. Without this update a
/// concurrent `restore_from_manifest(new_id)` would see "no live
/// writer" and proceed to register a competing allocation on the
/// session this Worker just moved into.
/// Segment lineage this Worker just moved into.
///
/// The lock is opened once and the allocation is rewritten inside the
/// guard, so the segment_id collision check is atomic with the
+3 -8
View File
@@ -212,8 +212,8 @@ mod tests {
break;
}
}
assert_eq!(runtime_header.as_deref(), Some("runtime-test"));
assert_eq!(worker_header.as_deref(), Some("test-worker"));
assert_eq!(runtime_header, None);
assert_eq!(worker_header, None);
assert_eq!(authorization, None);
let body = serde_json::json!({
"authority": "workspace-backend-skills-v0",
@@ -236,12 +236,7 @@ mod tests {
.unwrap();
});
let client = crate::worker::RuntimeWorkspaceHttpClient::new(
"ws-1",
format!("http://{addr}"),
"runtime-test",
"test-worker",
);
let client = crate::worker::TestWorkspaceHttpClient::new("ws-1", format!("http://{addr}"));
let catalog = (&client as &dyn WorkspaceClient).list_skills().unwrap();
assert_eq!(catalog.entries[0].name, "triage-errors");
assert_eq!(catalog.entries[0].provenance.id, "workspace:triage-errors");
+169 -2
View File
@@ -27,7 +27,10 @@ use crate::internal_worker::{
};
use crate::prompt::catalog::PromptCatalog;
use crate::spawn::registry::SpawnedWorkerRegistry;
use crate::worker::{Worker, WorkerFilesystemAuthority};
use crate::worker::{
ReviewerAttemptContext, ReviewerChildWorkspaceClient, Worker, WorkerFilesystemAuthority,
WorkspaceRequest, WorkspaceRequestMethod,
};
use protocol::Method;
#[derive(Debug, Deserialize, schemars::JsonSchema)]
@@ -55,6 +58,16 @@ 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.
#[serde(default)]
review: Option<ReviewerHandoffInput>,
}
#[derive(Debug, Deserialize, schemars::JsonSchema)]
struct ReviewerHandoffInput {
ticket_id: String,
revision_id: String,
}
#[derive(Debug, Deserialize, schemars::JsonSchema)]
@@ -320,6 +333,32 @@ impl SubWorkerSpawnTool {
}
}
fn validate_reviewer_handoff(input: &SubWorkerSpawnInput) -> Result<(), ToolError> {
let Some(review) = &input.review else {
return Ok(());
};
if review.ticket_id.trim().is_empty() || review.revision_id.trim().is_empty() {
return Err(ToolError::InvalidArgument(
"reviewer handoff requires non-empty ticket_id and revision_id".to_string(),
));
}
if input.profile.as_deref() != Some("builtin:reviewer") {
return Err(ToolError::InvalidArgument(
"reviewer handoff requires the explicit effective profile builtin:reviewer".to_string(),
));
}
if input
.scope
.iter()
.any(|rule| matches!(rule.permission, PermissionInput::Write))
{
return Err(ToolError::InvalidArgument(
"Merge Request Reviewer SubWorkers must have read-only delegated scope".to_string(),
));
}
Ok(())
}
#[async_trait]
impl Tool for SubWorkerSpawnTool {
async fn execute(
@@ -340,6 +379,7 @@ impl Tool for SubWorkerSpawnTool {
input.name
)));
}
validate_reviewer_handoff(&input)?;
let name_reservation = self
.registry
.reserve_internal_name(input.name.clone())
@@ -378,6 +418,48 @@ impl Tool for SubWorkerSpawnTool {
.map_err(|error| {
ToolError::ExecutionFailed(format!("resolve child manifest: {error}"))
})?;
let reviewer_attempt = 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(),
uuid::Uuid::now_v7().simple()
),
)
});
let child_workspace_context =
if let Some((ticket_id, revision_id, _, capability_token)) = &reviewer_attempt {
let workspace_id =
self.workspace_context
.workspace_id()
.cloned()
.ok_or_else(|| {
ToolError::InvalidArgument(
"reviewer handoff requires Workspace identity".to_string(),
)
})?;
let parent_client = self.workspace_context.client_handle();
if !parent_client.is_available() {
return Err(ToolError::InvalidArgument(
"reviewer handoff requires Workspace API authority".to_string(),
));
}
let child_client: Arc<dyn crate::worker::WorkspaceClient> =
Arc::new(ReviewerChildWorkspaceClient::new(
parent_client.clone(),
ReviewerAttemptContext {
ticket_id: ticket_id.clone(),
revision_id: revision_id.clone(),
},
capability_token.clone(),
));
crate::worker::WorkerWorkspaceContext::with_client(Some(workspace_id), child_client)
} else {
self.workspace_context.clone()
};
let store = EphemeralSessionStore::default();
let filesystem_authority =
WorkerFilesystemAuthority::local(self.workspace_root.clone(), child_cwd.clone());
@@ -385,7 +467,7 @@ impl Tool for SubWorkerSpawnTool {
child_manifest,
store.clone(),
self.prompt_loader.clone(),
self.workspace_context.clone(),
child_workspace_context,
filesystem_authority,
self.internal_client_override
.as_ref()
@@ -465,6 +547,66 @@ impl Tool for SubWorkerSpawnTool {
}
};
if let Some((ticket_id, revision_id, attempt_id, capability_token)) = &reviewer_attempt {
let workspace_id = self.workspace_context.workspace_id().ok_or_else(|| {
ToolError::ExecutionFailed("reviewer attempt lost Workspace identity".to_string())
})?;
let child_session_id = session.session_id_string();
let child_registration = WorkspaceRequest::json(
WorkspaceRequestMethod::Post,
format!(
"/api/w/{}/internal/reviewer-child-sessions",
workspace_id.as_str()
),
serde_json::json!({"child_session_id": child_session_id}).to_string(),
);
let child_response = self
.workspace_context
.client()
.execute(child_registration)
.map_err(|error| {
ToolError::ExecutionFailed(format!(
"register Runtime-owned Reviewer child session: {error}"
))
})?;
if !child_response.is_success() {
let _ = session.stop().await;
return Err(ToolError::ExecutionFailed(format!(
"register Runtime-owned Reviewer child session failed with status {}: {}",
child_response.status, child_response.body
)));
}
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",
workspace_id.as_str(),
ticket_id
),
body.to_string(),
);
let response = self
.workspace_context
.client()
.execute(request)
.map_err(|error| {
ToolError::ExecutionFailed(format!("register reviewer attempt: {error}"))
})?;
if !response.is_success() {
let _ = session.stop().await;
return Err(ToolError::ExecutionFailed(format!(
"register reviewer attempt failed with status {}: {}",
response.status, response.body
)));
}
}
let record = crate::spawn::registry::InternalSpawnedWorkerRecord::new(
input.name.clone(),
scope_allow,
@@ -899,6 +1041,31 @@ mod tests {
WorkspaceClient, WorkspaceClientError, WorkspaceRequest, WorkspaceResponse,
};
#[test]
fn reviewer_handoff_requires_explicit_builtin_profile_and_read_only_scope() {
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"}
}))
.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"}
}))
.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"}
}))
.unwrap();
assert!(validate_reviewer_handoff(&writable).is_err());
}
fn abs_rule(path: &Path, permission: Permission) -> ScopeRule {
ScopeRule {
target: path.to_path_buf(),
+159 -114
View File
@@ -223,55 +223,135 @@ pub trait WorkspaceClient: std::fmt::Debug + Send + Sync {
fn is_available(&self) -> bool;
fn execute(&self, request: WorkspaceRequest)
-> Result<WorkspaceResponse, WorkspaceClientError>;
}
/// HTTP forwarding client created by Runtime for one concrete Worker execution.
///
/// The upstream endpoint and source headers are private implementation details;
/// model-visible tools can only submit [`WorkspaceRequest`] values through the
/// [`WorkspaceClient`] trait.
pub struct RuntimeWorkspaceHttpClient {
workspace_id: String,
base_url: String,
runtime_id: String,
worker_id: String,
}
/// Executes the destructive WorkerRemove operation through Runtime-owned source proof.
/// Target identity is operation data; source identity and permission are never caller inputs.
fn execute_worker_remove(
&self,
_target_runtime_id: &str,
_target_worker_id: &str,
_expected_worker_revision: &str,
_reason: &str,
) -> Result<WorkspaceResponse, WorkspaceClientError> {
Err(WorkspaceClientError::Unavailable(
"Runtime-owned WorkerRemove forwarding is unavailable".to_string(),
))
}
impl std::fmt::Debug for RuntimeWorkspaceHttpClient {
fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
formatter
.debug_struct("RuntimeWorkspaceHttpClient")
.field("workspace_id", &self.workspace_id)
.field("base_url", &self.base_url)
.field("runtime_id", &self.runtime_id)
.field("worker_id", &self.worker_id)
.finish()
/// Trusted review-attempt 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> {
None
}
}
impl RuntimeWorkspaceHttpClient {
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct ReviewerAttemptContext {
pub ticket_id: String,
pub revision_id: String,
}
#[derive(Debug)]
pub struct ReviewerChildWorkspaceClient {
inner: Arc<dyn WorkspaceClient>,
context: ReviewerAttemptContext,
capability_token: String,
}
impl ReviewerChildWorkspaceClient {
pub fn new(
workspace_id: impl Into<String>,
base_url: impl Into<String>,
runtime_id: impl Into<String>,
worker_id: impl Into<String>,
inner: Arc<dyn WorkspaceClient>,
context: ReviewerAttemptContext,
capability_token: String,
) -> Self {
Self {
workspace_id: workspace_id.into(),
base_url: base_url.into().trim_end_matches('/').to_string(),
runtime_id: runtime_id.into(),
worker_id: worker_id.into(),
inner,
context,
capability_token,
}
}
}
impl WorkspaceClient for RuntimeWorkspaceHttpClient {
impl WorkspaceClient for ReviewerChildWorkspaceClient {
fn workspace_id(&self) -> Option<&str> {
self.inner.workspace_id()
}
fn kind(&self) -> &str {
"runtime-reviewer-child"
}
fn is_available(&self) -> bool {
self.inner.is_available()
}
fn reviewer_attempt_context(&self) -> Option<&ReviewerAttemptContext> {
Some(&self.context)
}
fn execute(
&self,
mut request: WorkspaceRequest,
) -> Result<WorkspaceResponse, WorkspaceClientError> {
let expected_path = format!(
"/api/w/{}/tickets/{}/merge-request/reviews",
self.workspace_id().unwrap_or_default(),
self.context.ticket_id
);
if request.method == WorkspaceRequestMethod::Post && request.path == expected_path {
let body = request.body.take().ok_or_else(|| {
WorkspaceClientError::Request("review submission requires a JSON body".to_string())
})?;
let mut value: serde_json::Value = serde_json::from_str(&body)
.map_err(|error| WorkspaceClientError::Request(error.to_string()))?;
let object = value.as_object_mut().ok_or_else(|| {
WorkspaceClientError::Request(
"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()),
);
request.body = Some(
serde_json::to_string(&value)
.map_err(|error| WorkspaceClientError::Request(error.to_string()))?,
);
} else if request.method != WorkspaceRequestMethod::Get {
return Err(WorkspaceClientError::Unavailable(
"Reviewer child Workspace authority is read-only except for its one attested Merge Request review submission".to_string(),
));
}
self.inner.execute(request)
}
}
#[cfg(test)]
#[derive(Clone, Debug)]
pub(crate) struct TestWorkspaceHttpClient {
workspace_id: String,
base_url: String,
}
#[cfg(test)]
impl TestWorkspaceHttpClient {
pub(crate) fn new(workspace_id: impl Into<String>, base_url: impl Into<String>) -> Self {
Self {
workspace_id: workspace_id.into(),
base_url: base_url.into().trim_end_matches('/').to_string(),
}
}
}
#[cfg(test)]
impl WorkspaceClient for TestWorkspaceHttpClient {
fn workspace_id(&self) -> Option<&str> {
Some(&self.workspace_id)
}
fn kind(&self) -> &str {
"runtime-http-proxy"
"test-http"
}
fn is_available(&self) -> bool {
@@ -283,32 +363,28 @@ impl WorkspaceClient for RuntimeWorkspaceHttpClient {
request: WorkspaceRequest,
) -> Result<WorkspaceResponse, WorkspaceClientError> {
let base_url = self.base_url.clone();
let runtime_id = self.runtime_id.clone();
let worker_id = self.worker_id.clone();
if tokio::runtime::Handle::try_current().is_ok() {
std::thread::spawn(move || {
execute_runtime_workspace_http(&base_url, &runtime_id, &worker_id, request)
})
.join()
.map_err(|_| {
WorkspaceClientError::Request("workspace request thread panicked".to_string())
})?
std::thread::spawn(move || execute_test_workspace_http(&base_url, request))
.join()
.map_err(|_| {
WorkspaceClientError::Request(
"test workspace request thread panicked".to_string(),
)
})?
} else {
execute_runtime_workspace_http(&base_url, &runtime_id, &worker_id, request)
execute_test_workspace_http(&base_url, request)
}
}
}
fn execute_runtime_workspace_http(
#[cfg(test)]
fn execute_test_workspace_http(
base_url: &str,
runtime_id: &str,
worker_id: &str,
request: WorkspaceRequest,
) -> Result<WorkspaceResponse, WorkspaceClientError> {
if !request.path.starts_with('/') || request.path.starts_with("//") {
return Err(WorkspaceClientError::InvalidPath(request.path));
}
let url = format!("{base_url}{}", request.path);
let method = match request.method {
WorkspaceRequestMethod::Get => reqwest::Method::GET,
WorkspaceRequestMethod::Post => reqwest::Method::POST,
@@ -317,16 +393,11 @@ fn execute_runtime_workspace_http(
WorkspaceRequestMethod::Delete => reqwest::Method::DELETE,
};
let client = reqwest::blocking::Client::new();
let mut request_builder = client
.request(method, url)
.header("x-yoi-runtime-id", runtime_id)
.header("x-yoi-worker-id", worker_id);
let mut builder = client.request(method, format!("{base_url}{}", request.path));
if let Some(body) = request.body {
request_builder = request_builder
.header(reqwest::header::CONTENT_TYPE, "application/json")
.body(body);
builder = builder.body(body);
}
let response = request_builder
let response = builder
.send()
.map_err(|error| WorkspaceClientError::Request(error.to_string()))?;
let status = response.status().as_u16();
@@ -365,6 +436,36 @@ impl WorkspaceClient for MarkerWorkspaceClient {
}
}
#[cfg(test)]
mod reviewer_client_tests {
use super::*;
#[test]
fn reviewer_child_client_denies_non_review_workspace_mutations() {
let inner: Arc<dyn WorkspaceClient> = Arc::new(MarkerWorkspaceClient {
workspace_id: Some("ws".to_string()),
kind: "marker".to_string(),
available: true,
reason: "forwarded".to_string(),
});
let client = ReviewerChildWorkspaceClient::new(
inner,
ReviewerAttemptContext {
ticket_id: "T1".into(),
revision_id: "V1".into(),
},
"secret".into(),
);
let request = WorkspaceRequest::json(
WorkspaceRequestMethod::Post,
"/api/w/ws/tickets/T1/comments",
"{}".to_string(),
);
let error = client.execute(request).unwrap_err();
assert!(error.to_string().contains("read-only"));
}
}
pub fn unavailable_workspace_client(
workspace_id: Option<&WorkspaceId>,
reason: impl Into<String>,
@@ -2949,13 +3050,13 @@ impl<C: LlmClient, St: Store> Worker<C, St> {
/// Compact the current session by summarising history via a
/// disposable Engine, then replacing history with
/// `[summary, ...recent_turns]` and creating a new session.
/// `[summary, ...recent_turns]` in a new Segment of the same Session.
///
/// The summary Engine uses:
/// - `compaction.model` from the manifest if configured, or
/// - a clone of the main LlmClient via `clone_boxed()`.
///
/// Returns the new session ID.
/// Returns the new Segment ID. The Worker keeps its Session ID.
pub async fn compact(&mut self, retained_tokens: u64) -> Result<SegmentId, WorkerError> {
use crate::compact::worker::{
CompactWorkerContext, CompactWorkerInterceptor, add_reference_tool,
@@ -6844,11 +6945,9 @@ mod build_summary_prompt_tests {
});
WorkerWorkspaceContext::with_client(
Some(WorkspaceId::new("test-memory").unwrap()),
Arc::new(RuntimeWorkspaceHttpClient::new(
Arc::new(TestWorkspaceHttpClient::new(
"test-memory",
format!("http://{addr}"),
"test-runtime",
"test-worker",
)),
)
}
@@ -6981,11 +7080,9 @@ mod build_summary_prompt_tests {
store,
WorkerWorkspaceContext::with_client(
Some(WorkspaceId::new("ws-skill").unwrap()),
Arc::new(RuntimeWorkspaceHttpClient::new(
Arc::new(TestWorkspaceHttpClient::new(
"ws-skill",
format!("http://{addr}"),
"test-runtime",
"test-worker",
)),
),
authority,
@@ -7027,58 +7124,6 @@ mod build_summary_prompt_tests {
}));
}
#[test]
fn runtime_workspace_client_sends_runtime_worker_identity_without_bearer() {
use std::io::{BufRead, BufReader, Write};
use std::net::TcpListener;
let listener = TcpListener::bind("127.0.0.1:0").unwrap();
let address = listener.local_addr().unwrap();
let server = std::thread::spawn(move || {
let (mut stream, _) = listener.accept().unwrap();
let mut reader = BufReader::new(stream.try_clone().unwrap());
let mut first_line = String::new();
reader.read_line(&mut first_line).unwrap();
assert!(first_line.contains("/api/w/workspace-a/tickets/search"));
let mut runtime_id = String::new();
let mut worker_id = String::new();
let mut authorization = String::new();
loop {
let mut line = String::new();
reader.read_line(&mut line).unwrap();
if let Some(value) = line.strip_prefix("x-yoi-runtime-id: ") {
runtime_id = value.trim().to_string();
}
if let Some(value) = line.strip_prefix("x-yoi-worker-id: ") {
worker_id = value.trim().to_string();
}
if let Some(value) = line.strip_prefix("authorization: ") {
authorization = value.trim().to_string();
}
if line == "\r\n" || line.is_empty() {
break;
}
}
assert_eq!(runtime_id, "runtime-a");
assert_eq!(worker_id, "worker-a");
assert!(authorization.is_empty());
stream
.write_all(b"HTTP/1.1 200 OK\r\nContent-Length: 2\r\n\r\n{}")
.unwrap();
});
let client = RuntimeWorkspaceHttpClient::new(
"workspace-a",
format!("http://{address}"),
"runtime-a",
"worker-a",
);
let response = client
.execute(WorkspaceRequest::get("/api/w/workspace-a/tickets/search"))
.unwrap();
assert_eq!(response.status, 200);
server.join().unwrap();
}
#[tokio::test]
async fn cancelled_internal_extract_does_not_commit_pointer_or_completed_audit() {
let dir = tempfile::tempdir().unwrap();
+7 -3
View File
@@ -421,7 +421,11 @@ async fn pre_run_compact_success_broadcasts_start_and_done() {
// Drain run events so only compact events remain in `rx`.
let _ = drain(&mut rx);
let session_before = worker.session_id();
let segment_before = worker.segment_id();
worker.try_pre_run_compact().await;
assert_eq!(worker.session_id(), session_before);
assert_ne!(worker.segment_id(), segment_before);
let events = drain(&mut rx);
let kinds: Vec<&str> = events
@@ -442,7 +446,7 @@ async fn pre_run_compact_success_broadcasts_start_and_done() {
"unexpected CompactFailed in {kinds:?}"
);
// CompactDone carries the new session id.
// CompactDone carries the new Segment ID; the Session ID is unchanged.
let new_id_in_event = events.iter().find_map(|e| match e {
Event::CompactDone { new_segment_id } => Some(*new_segment_id),
_ => None,
@@ -583,11 +587,11 @@ async fn compact_resets_extract_pointer_so_extract_can_fire_again() {
);
// Compact runs. Without the fix the in-memory pointer would still
// reference the old session's history_len.
// reference the old Segment's history_len.
worker.try_pre_run_compact().await;
assert!(
worker.extract_pointer().is_none(),
"extract_pointer must be reset to None after compact (matches cold-restore on the new session)"
"extract_pointer must be reset to None after compact (matches cold-restore on the new Segment)"
);
}