refactor: broker SubWorker Workdir tools through parent
This commit is contained in:
@@ -514,6 +514,7 @@ impl WorkerController {
|
||||
runtime_base.to_path_buf(),
|
||||
spawned_registry.clone(),
|
||||
Some(method_tx.downgrade()),
|
||||
None,
|
||||
)
|
||||
.await?;
|
||||
if let Some(session) = fs_for_view.as_ref() {
|
||||
@@ -911,6 +912,7 @@ pub(crate) async fn register_worker_tools<C, St>(
|
||||
runtime_base: PathBuf,
|
||||
spawned_registry: Arc<SpawnedWorkerRegistry>,
|
||||
parent_method_tx: Option<mpsc::WeakSender<Method>>,
|
||||
inherited_workdir_tool_broker: Option<workdir::WorkdirToolBroker>,
|
||||
) -> std::io::Result<Option<workdir::WorkdirSessionHandle>>
|
||||
where
|
||||
C: LlmClient + Clone + 'static,
|
||||
@@ -919,21 +921,26 @@ where
|
||||
// Worker-immutable snapshots taken before the mutable worker borrow
|
||||
// below so the worker borrow doesn't conflict with reads on `worker`.
|
||||
let feature_config = worker.manifest().feature.clone();
|
||||
let mut workdir_tool_broker = inherited_workdir_tool_broker;
|
||||
if feature_config.manage_workdir.enabled && worker.workdir_session().is_none() {
|
||||
let workspace_client = worker.workspace_client_handle();
|
||||
worker.bind_workdir_session(Some(workdir::delegation_capable_session(
|
||||
let broker = workdir::WorkdirToolBroker::new(
|
||||
crate::feature::builtin::manage_workdir::WorkspaceAttachedWorkdirSession::handle(
|
||||
workspace_client,
|
||||
),
|
||||
)));
|
||||
}
|
||||
if feature_config.sub_worker.enabled
|
||||
);
|
||||
worker.bind_workdir_session(Some(broker.tool_session()));
|
||||
workdir_tool_broker = Some(broker);
|
||||
} else if workdir_tool_broker.is_none()
|
||||
&& let Some(existing) = worker.workdir_session().cloned()
|
||||
&& !existing.is_delegation_capable()
|
||||
{
|
||||
worker.bind_workdir_session(Some(workdir::delegation_capable_session(existing)));
|
||||
let broker = workdir::WorkdirToolBroker::new(existing);
|
||||
worker.bind_workdir_session(Some(broker.tool_session()));
|
||||
workdir_tool_broker = Some(broker);
|
||||
}
|
||||
let worker_workdir = worker.workdir_session().cloned();
|
||||
let worker_workdir = workdir_tool_broker
|
||||
.as_ref()
|
||||
.map(workdir::WorkdirToolBroker::tool_session);
|
||||
let local_filesystem = worker.local_working_directory().cloned();
|
||||
let local_workspace_root = local_filesystem.as_ref().map(|local| local.root.clone());
|
||||
let task_feature = worker.task_feature();
|
||||
@@ -1157,7 +1164,6 @@ where
|
||||
}
|
||||
|
||||
let host_worker_observation_provider = worker.worker_observation_provider();
|
||||
let source_workdir_session = worker.workdir_session().cloned();
|
||||
{
|
||||
let workspace_client = worker.workspace_client_handle();
|
||||
let engine = worker.engine_mut();
|
||||
@@ -1199,7 +1205,7 @@ where
|
||||
runtime_base.clone(),
|
||||
bash_output_dir.clone(),
|
||||
spawner_workspace_root,
|
||||
source_workdir_session,
|
||||
workdir_tool_broker,
|
||||
spawned_registry.clone(),
|
||||
spawner_manifest,
|
||||
prompts,
|
||||
|
||||
@@ -12,7 +12,7 @@ use async_trait::async_trait;
|
||||
use serde::{Deserialize, Serialize};
|
||||
use serde_json::json;
|
||||
use workdir::http::{WorkdirSessionOperation, WorkdirSessionOperationResult};
|
||||
use workdir::workspace::{WorkspaceWorkdirSessionFence, WorkspaceWorkdirSessionOperationRequest};
|
||||
use workdir::workspace::WorkspaceWorkdirSessionOperationRequest;
|
||||
use workdir::{
|
||||
CommandHandle, CommandOutput, CommandOutputRequest, CommandRequest, CommandStatus, EditRequest,
|
||||
EditResult, GlobRequest, GlobResult, GrepRequest, GrepResult, ListRequest, ListResult,
|
||||
@@ -156,8 +156,6 @@ struct WorkspaceHttpWorkdirBackend {
|
||||
pub struct WorkspaceAttachedWorkdirSession {
|
||||
client: Arc<dyn WorkspaceClient>,
|
||||
workdir: Workdir,
|
||||
expected_session_fence: Option<String>,
|
||||
delegations: Vec<workdir::WorkdirDelegationRequest>,
|
||||
}
|
||||
|
||||
impl WorkspaceAttachedWorkdirSession {
|
||||
@@ -165,8 +163,6 @@ impl WorkspaceAttachedWorkdirSession {
|
||||
Arc::new(Self {
|
||||
client,
|
||||
workdir: Workdir::new("workspace-attachment"),
|
||||
expected_session_fence: None,
|
||||
delegations: Vec::new(),
|
||||
})
|
||||
}
|
||||
|
||||
@@ -183,16 +179,13 @@ impl WorkspaceAttachedWorkdirSession {
|
||||
"/api/w/{}/workers/self/workdir-session/operations",
|
||||
encode_path_segment(workspace_id)
|
||||
),
|
||||
serde_json::to_string(&WorkspaceWorkdirSessionOperationRequest {
|
||||
expected_session_fence: self.expected_session_fence.clone(),
|
||||
delegations: self.delegations.clone(),
|
||||
operation,
|
||||
})
|
||||
.map_err(|error| {
|
||||
WorkdirError::Transport(format!(
|
||||
"failed to encode Workspace Workdir operation: {error}"
|
||||
))
|
||||
})?,
|
||||
serde_json::to_string(&WorkspaceWorkdirSessionOperationRequest { operation }).map_err(
|
||||
|error| {
|
||||
WorkdirError::Transport(format!(
|
||||
"failed to encode Workspace Workdir operation: {error}"
|
||||
))
|
||||
},
|
||||
)?,
|
||||
);
|
||||
let response = self
|
||||
.client
|
||||
@@ -241,59 +234,6 @@ impl WorkdirSession for WorkspaceAttachedWorkdirSession {
|
||||
WorkdirSessionCapabilities::ALL
|
||||
}
|
||||
|
||||
fn transports_delegation_context(&self) -> bool {
|
||||
true
|
||||
}
|
||||
|
||||
async fn capture_delegation_source(
|
||||
&self,
|
||||
request: &workdir::WorkdirDelegationRequest,
|
||||
) -> Result<WorkdirSessionHandle, WorkdirError> {
|
||||
let expected_session_fence = if let Some(fence) = &self.expected_session_fence {
|
||||
fence.clone()
|
||||
} else {
|
||||
let workspace_id = self.client.workspace_id().ok_or_else(|| {
|
||||
WorkdirError::Unavailable("Workspace identity is unavailable".to_string())
|
||||
})?;
|
||||
let response = self
|
||||
.client
|
||||
.execute(WorkspaceRequest {
|
||||
method: WorkspaceRequestMethod::Get,
|
||||
path: format!(
|
||||
"/api/w/{}/workers/self/workdir-session/fence",
|
||||
encode_path_segment(workspace_id)
|
||||
),
|
||||
body: None,
|
||||
})
|
||||
.map_err(|error| {
|
||||
WorkdirError::Unavailable(format!(
|
||||
"failed to capture Workdir attachment fence: {error}"
|
||||
))
|
||||
})?;
|
||||
let fence: WorkspaceWorkdirSessionFence = serde_json::from_str(&response.body)
|
||||
.map_err(|error| {
|
||||
WorkdirError::Unavailable(format!(
|
||||
"invalid Workdir attachment fence response: {error}"
|
||||
))
|
||||
})?;
|
||||
fence.value
|
||||
};
|
||||
let mut delegations = self.delegations.clone();
|
||||
delegations.push(request.clone());
|
||||
let candidate = Arc::new(Self {
|
||||
client: self.client.clone(),
|
||||
workdir: self.workdir.clone(),
|
||||
expected_session_fence: Some(expected_session_fence),
|
||||
delegations,
|
||||
});
|
||||
candidate
|
||||
.stat(StatRequest {
|
||||
path: workdir::WorkdirPath::new("").expect("empty Workdir path is valid"),
|
||||
})
|
||||
.await?;
|
||||
Ok(candidate)
|
||||
}
|
||||
|
||||
async fn stat(&self, request: StatRequest) -> Result<StatResult, WorkdirError> {
|
||||
match self.operate(WorkdirSessionOperation::Stat(request))? {
|
||||
WorkdirSessionOperationResult::Stat(result) => Ok(result),
|
||||
@@ -1155,6 +1095,7 @@ mod tests {
|
||||
command: "true".to_string(),
|
||||
timeout_secs: 120,
|
||||
output_limit: 1024,
|
||||
cwd: None,
|
||||
spill_dir: Some("/worker-local/bash-output".into()),
|
||||
tool_call_id: Some("call-1".to_string()),
|
||||
})
|
||||
@@ -1178,83 +1119,6 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn delegated_attached_session_carries_captured_fence_on_operations() {
|
||||
let client = Arc::new(RecordingWorkspaceClient::new(vec![
|
||||
response(json!({"value": "attachment-fence"})),
|
||||
response(json!({
|
||||
"operation": "stat",
|
||||
"result": {"path": "", "kind": "directory", "size": 0}
|
||||
})),
|
||||
response(json!({
|
||||
"operation": "stat",
|
||||
"result": {"path": "visible.txt", "kind": "file", "size": 8}
|
||||
})),
|
||||
]));
|
||||
let parent = workdir::delegation_capable_session(WorkspaceAttachedWorkdirSession::handle(
|
||||
client.clone(),
|
||||
));
|
||||
let delegation = parent
|
||||
.delegate(workdir::WorkdirDelegationRequest {
|
||||
rules: vec![workdir::WorkdirDelegationRule {
|
||||
target: workdir::WorkdirPath::new("").unwrap(),
|
||||
permission: workdir::WorkdirDelegationPermission::Read,
|
||||
recursive: false,
|
||||
}],
|
||||
cwd: workdir::WorkdirPath::new("").unwrap(),
|
||||
})
|
||||
.await
|
||||
.unwrap();
|
||||
delegation
|
||||
.scoped_session
|
||||
.stat(StatRequest {
|
||||
path: workdir::WorkdirPath::new("visible.txt").unwrap(),
|
||||
})
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
let requests = client.requests();
|
||||
assert_eq!(requests.len(), 3);
|
||||
assert_eq!(
|
||||
requests[0].path,
|
||||
"/api/w/workspace%2Ftest/workers/self/workdir-session/fence"
|
||||
);
|
||||
let body: serde_json::Value =
|
||||
serde_json::from_str(requests[2].body.as_deref().unwrap()).unwrap();
|
||||
assert_eq!(body["expected_session_fence"], "attachment-fence");
|
||||
assert_eq!(body["operation"]["operation"], "stat");
|
||||
assert_eq!(body["delegations"][0]["rules"][0]["target"], "");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn attached_provider_rejection_happens_before_delegation_is_returned() {
|
||||
let client = Arc::new(RecordingWorkspaceClient::new(vec![
|
||||
response(json!({"value": "attachment-fence"})),
|
||||
response(json!({"error": "provider rejected delegated write target"})),
|
||||
]));
|
||||
let parent = workdir::delegation_capable_session(WorkspaceAttachedWorkdirSession::handle(
|
||||
client.clone(),
|
||||
));
|
||||
let result = parent
|
||||
.delegate(workdir::WorkdirDelegationRequest {
|
||||
rules: vec![workdir::WorkdirDelegationRule {
|
||||
target: workdir::WorkdirPath::new("linked-target").unwrap(),
|
||||
permission: workdir::WorkdirDelegationPermission::Write,
|
||||
recursive: true,
|
||||
}],
|
||||
cwd: workdir::WorkdirPath::new("linked-target").unwrap(),
|
||||
})
|
||||
.await;
|
||||
|
||||
assert!(result.is_err(), "provider rejection must fail before lease");
|
||||
let requests = client.requests();
|
||||
assert_eq!(requests.len(), 2);
|
||||
let validation: serde_json::Value =
|
||||
serde_json::from_str(requests[1].body.as_deref().unwrap()).unwrap();
|
||||
assert_eq!(validation["operation"]["operation"], "stat");
|
||||
assert_eq!(validation["delegations"].as_array().unwrap().len(), 1);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn attached_session_preserves_typed_provider_validation_error() {
|
||||
let client = Arc::new(RecordingWorkspaceClient::new(vec![error_response(
|
||||
@@ -1298,73 +1162,52 @@ mod tests {
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn nested_attached_session_preserves_full_delegation_chain() {
|
||||
async fn scoped_broker_operations_carry_no_child_context() {
|
||||
let client = Arc::new(RecordingWorkspaceClient::new(vec![
|
||||
response(json!({"value": "attachment-fence"})),
|
||||
response(json!({
|
||||
"operation": "stat",
|
||||
"result": {"path": "", "kind": "directory", "size": 0}
|
||||
"result": {"path": "visible.txt", "kind": "file", "size": 8}
|
||||
})),
|
||||
response(json!({
|
||||
"operation": "stat",
|
||||
"result": {"path": "nested", "kind": "directory", "size": 0}
|
||||
})),
|
||||
response(json!({
|
||||
"operation": "stat",
|
||||
"result": {"path": "nested/file", "kind": "file", "size": 1}
|
||||
"result": {"path": "visible.txt", "kind": "file", "size": 8}
|
||||
})),
|
||||
]));
|
||||
let parent = workdir::delegation_capable_session(WorkspaceAttachedWorkdirSession::handle(
|
||||
let broker = workdir::WorkdirToolBroker::new(WorkspaceAttachedWorkdirSession::handle(
|
||||
client.clone(),
|
||||
));
|
||||
let outer = parent
|
||||
.delegate(workdir::WorkdirDelegationRequest {
|
||||
rules: vec![workdir::WorkdirDelegationRule {
|
||||
let scoped = broker
|
||||
.scope(workdir::WorkdirToolScope {
|
||||
rules: vec![workdir::WorkdirToolScopeRule {
|
||||
target: workdir::WorkdirPath::new("").unwrap(),
|
||||
permission: workdir::WorkdirDelegationPermission::Read,
|
||||
permission: workdir::WorkdirToolScopePermission::Read,
|
||||
recursive: true,
|
||||
}],
|
||||
cwd: workdir::WorkdirPath::new("").unwrap(),
|
||||
command: false,
|
||||
})
|
||||
.await
|
||||
.unwrap();
|
||||
let nested = outer
|
||||
.scoped_session
|
||||
.delegate(workdir::WorkdirDelegationRequest {
|
||||
rules: vec![workdir::WorkdirDelegationRule {
|
||||
target: workdir::WorkdirPath::new("nested").unwrap(),
|
||||
permission: workdir::WorkdirDelegationPermission::Read,
|
||||
recursive: true,
|
||||
}],
|
||||
cwd: workdir::WorkdirPath::new("nested").unwrap(),
|
||||
})
|
||||
.await
|
||||
.unwrap();
|
||||
nested
|
||||
.scoped_session
|
||||
scoped
|
||||
.stat(StatRequest {
|
||||
path: workdir::WorkdirPath::new("file").unwrap(),
|
||||
path: workdir::WorkdirPath::new("visible.txt").unwrap(),
|
||||
})
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
let requests = client.requests();
|
||||
assert_eq!(requests.len(), 4);
|
||||
let outer_validation: serde_json::Value =
|
||||
serde_json::from_str(requests[1].body.as_deref().unwrap()).unwrap();
|
||||
let nested_validation: serde_json::Value =
|
||||
serde_json::from_str(requests[2].body.as_deref().unwrap()).unwrap();
|
||||
assert_eq!(outer_validation["delegations"].as_array().unwrap().len(), 1);
|
||||
assert_eq!(
|
||||
nested_validation["delegations"].as_array().unwrap().len(),
|
||||
2
|
||||
);
|
||||
let body: serde_json::Value =
|
||||
serde_json::from_str(requests[3].body.as_deref().unwrap()).unwrap();
|
||||
assert_eq!(body["delegations"].as_array().unwrap().len(), 2);
|
||||
assert_eq!(body["delegations"][0]["rules"][0]["target"], "");
|
||||
assert_eq!(body["delegations"][1]["rules"][0]["target"], "nested");
|
||||
assert_eq!(body["operation"]["request"]["path"], "file");
|
||||
assert_eq!(requests.len(), 2);
|
||||
for request in requests {
|
||||
assert_eq!(
|
||||
request.path,
|
||||
"/api/w/workspace%2Ftest/workers/self/workdir-session/operations"
|
||||
);
|
||||
let body: serde_json::Value =
|
||||
serde_json::from_str(request.body.as_deref().unwrap()).unwrap();
|
||||
assert!(body.get("delegations").is_none());
|
||||
assert!(body.get("child").is_none());
|
||||
assert!(body.get("expected_session_fence").is_none());
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -709,7 +709,7 @@ pub(crate) fn prepare_internal_worker_from_spec(
|
||||
}
|
||||
|
||||
Box::pin(prepare_internal_worker_session(
|
||||
worker, store, visibility, None, None,
|
||||
worker, store, visibility, None, None, None,
|
||||
))
|
||||
.await
|
||||
})
|
||||
@@ -746,13 +746,16 @@ pub(crate) async fn prepare_internal_worker_session(
|
||||
visibility: InternalWorkerVisibility,
|
||||
child_registry: Option<Arc<SpawnedWorkerRegistry>>,
|
||||
on_turn_end: Option<Arc<dyn Fn(InternalWorkerSessionStatus) + Send + Sync>>,
|
||||
command_event_broker: Option<workdir::WorkdirToolBroker>,
|
||||
) -> Result<InternalWorkerSessionHandle, InternalWorkerSessionError> {
|
||||
let (event_tx, _event_rx) = broadcast::channel(256);
|
||||
let sink = worker.sink();
|
||||
spawn_internal_log_event_bridge(sink.clone(), event_tx.clone());
|
||||
let alerter = Alerter::new(event_tx.clone());
|
||||
let in_flight = InFlightEvents::new(event_tx.clone());
|
||||
if let Some(session) = worker.workdir_session() {
|
||||
if let Some(broker) = command_event_broker.as_ref() {
|
||||
wire_workdir_command_events(&broker.tool_session(), &in_flight);
|
||||
} else if let Some(session) = worker.workdir_session() {
|
||||
wire_workdir_command_events(session, &in_flight);
|
||||
}
|
||||
let actor_in_flight = in_flight.clone();
|
||||
@@ -887,6 +890,7 @@ pub(crate) async fn spawn_prepared_internal_worker_session(
|
||||
InternalWorkerVisibility::ServicePrivate,
|
||||
None,
|
||||
on_turn_end,
|
||||
None,
|
||||
)
|
||||
.await?;
|
||||
handle.send(input).await?;
|
||||
|
||||
@@ -25,7 +25,7 @@ use session_store::{
|
||||
};
|
||||
use tokio::sync::broadcast;
|
||||
use tracing::warn;
|
||||
use workdir::WorkdirDelegation;
|
||||
use workdir::WorkdirScopeLease;
|
||||
|
||||
use crate::internal_worker::{InternalWorkerSessionHandle, InternalWorkerVisibility};
|
||||
use crate::runtime::dir::{RuntimeDir, SpawnedWorkerRecord};
|
||||
@@ -68,7 +68,7 @@ pub(crate) struct SubWorkerStopSummary {
|
||||
pub(crate) struct InternalSpawnedWorkerRecord {
|
||||
pub worker_name: String,
|
||||
pub scope_delegated: Vec<ScopeRule>,
|
||||
pub workdir_delegation: Arc<WorkdirDelegation>,
|
||||
pub workdir_tool_scope: Arc<WorkdirScopeLease>,
|
||||
#[cfg(test)]
|
||||
pub installed_tools: Arc<[String]>,
|
||||
pub session: InternalWorkerSessionHandle,
|
||||
@@ -86,7 +86,7 @@ impl InternalSpawnedWorkerRecord {
|
||||
pub(crate) fn new(
|
||||
worker_name: String,
|
||||
scope_delegated: Vec<ScopeRule>,
|
||||
workdir_delegation: WorkdirDelegation,
|
||||
workdir_tool_scope: WorkdirScopeLease,
|
||||
#[cfg(test)] installed_tools: Vec<String>,
|
||||
session: InternalWorkerSessionHandle,
|
||||
change_tracker: Option<tools::Tracker>,
|
||||
@@ -94,7 +94,7 @@ impl InternalSpawnedWorkerRecord {
|
||||
Self {
|
||||
worker_name,
|
||||
scope_delegated,
|
||||
workdir_delegation: Arc::new(workdir_delegation),
|
||||
workdir_tool_scope: Arc::new(workdir_tool_scope),
|
||||
#[cfg(test)]
|
||||
installed_tools: installed_tools.into(),
|
||||
session,
|
||||
@@ -690,7 +690,7 @@ impl SpawnedWorkerRegistry {
|
||||
if !record.claim_scope_reclaim() {
|
||||
return Ok(false);
|
||||
}
|
||||
record.workdir_delegation.release();
|
||||
record.workdir_tool_scope.release();
|
||||
let result = if let Some(parent_scope) = &self.parent_scope {
|
||||
parent_scope
|
||||
.update(|current| current.with_removed_deny_rules(delegated_write_rules(record)))
|
||||
@@ -966,7 +966,7 @@ mod tests {
|
||||
deny: Vec::new(),
|
||||
})
|
||||
.unwrap();
|
||||
let source = workdir::delegation_capable_session(Arc::new(
|
||||
let source = workdir::WorkdirToolBroker::new(Arc::new(
|
||||
workdir::LocalWorkdirSession::materialized_bound(
|
||||
workdir::Workdir::new("registry-test"),
|
||||
root.clone(),
|
||||
@@ -976,13 +976,14 @@ mod tests {
|
||||
),
|
||||
));
|
||||
let delegation = source
|
||||
.delegate(workdir::WorkdirDelegationRequest {
|
||||
rules: vec![workdir::WorkdirDelegationRule {
|
||||
.scope(workdir::WorkdirToolScope {
|
||||
rules: vec![workdir::WorkdirToolScopeRule {
|
||||
target: workdir::WorkdirPath::new("").unwrap(),
|
||||
permission: workdir::WorkdirDelegationPermission::Read,
|
||||
permission: workdir::WorkdirToolScopePermission::Read,
|
||||
recursive: true,
|
||||
}],
|
||||
cwd: workdir::WorkdirPath::new("").unwrap(),
|
||||
command: false,
|
||||
})
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
+62
-160
@@ -22,8 +22,7 @@ use manifest::{
|
||||
use serde::Deserialize;
|
||||
use tokio::sync::mpsc;
|
||||
use workdir::{
|
||||
WorkdirDelegationPermission, WorkdirDelegationRequest, WorkdirDelegationRule, WorkdirPath,
|
||||
WorkdirSessionHandle,
|
||||
WorkdirToolBroker, WorkdirToolScope, WorkdirToolScopePermission, WorkdirToolScopeRule,
|
||||
};
|
||||
|
||||
use crate::PromptCatalogSource;
|
||||
@@ -64,6 +63,9 @@ 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>,
|
||||
/// Explicitly grant command execution through the parent-owned Workdir tool broker.
|
||||
#[serde(default)]
|
||||
command: bool,
|
||||
/// 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)]
|
||||
@@ -267,8 +269,8 @@ pub struct SubWorkerSpawnTool {
|
||||
workspace_root: PathBuf,
|
||||
/// Directory the spawned SubWorker's tools should use when the LLM did not
|
||||
/// override it. Defaults to the spawner's cwd.
|
||||
/// Active provider-backed Workdir session from which child leases are captured.
|
||||
source_workdir_session: Option<WorkdirSessionHandle>,
|
||||
/// Parent-owned broker for scoped Workdir tool execution.
|
||||
workdir_tool_broker: Option<WorkdirToolBroker>,
|
||||
/// Parent-owned in-memory registry shared by the five SubWorker tools.
|
||||
registry: Arc<SpawnedWorkerRegistry>,
|
||||
/// Spawner's resolved Manifest. `profile = "inherit"` derives the
|
||||
@@ -295,7 +297,7 @@ impl SubWorkerSpawnTool {
|
||||
runtime_base: PathBuf,
|
||||
bash_output_dir: PathBuf,
|
||||
workspace_root: PathBuf,
|
||||
source_workdir_session: Option<WorkdirSessionHandle>,
|
||||
workdir_tool_broker: Option<WorkdirToolBroker>,
|
||||
registry: Arc<SpawnedWorkerRegistry>,
|
||||
spawner_manifest: WorkerManifest,
|
||||
prompt_loader: PromptCatalogSource,
|
||||
@@ -308,7 +310,7 @@ impl SubWorkerSpawnTool {
|
||||
runtime_base,
|
||||
bash_output_dir,
|
||||
workspace_root,
|
||||
source_workdir_session,
|
||||
workdir_tool_broker,
|
||||
registry,
|
||||
spawner_manifest,
|
||||
prompt_loader,
|
||||
@@ -341,6 +343,11 @@ fn validate_reviewer_handoff(input: &SubWorkerSpawnInput) -> Result<(), ToolErro
|
||||
"Merge Request Reviewer SubWorkers must include writable delegated scope".to_string(),
|
||||
));
|
||||
}
|
||||
if !input.command {
|
||||
return Err(ToolError::InvalidArgument(
|
||||
"Merge Request Reviewer SubWorkers require an explicit command grant".to_string(),
|
||||
));
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -370,7 +377,7 @@ impl Tool for SubWorkerSpawnTool {
|
||||
.reserve_internal_name(input.name.clone())
|
||||
.map_err(|error| ToolError::InvalidArgument(error.to_string()))?;
|
||||
|
||||
let mut workdir_rules = parse_workdir_scope(&input.scope)?;
|
||||
let workdir_rules = parse_workdir_scope(&input.scope)?;
|
||||
let child_bash_output_dir = self.bash_output_dir.join("sub-workers").join(&input.name);
|
||||
tokio::fs::create_dir_all(&child_bash_output_dir)
|
||||
.await
|
||||
@@ -380,28 +387,15 @@ impl Tool for SubWorkerSpawnTool {
|
||||
child_bash_output_dir.display()
|
||||
))
|
||||
})?;
|
||||
let source_workdir_session =
|
||||
require_active_workdir_session(self.source_workdir_session.as_ref())?;
|
||||
let transports_delegation_context = source_workdir_session.transports_delegation_context();
|
||||
// Provider-transported sessions resolve every delegation rule in the
|
||||
// receiving Workdir namespace. The Bash spill directory instead belongs
|
||||
// to this Worker host, so forwarding it would widen the request with a
|
||||
// foreign absolute path and fail the provider's existing scope check.
|
||||
if !transports_delegation_context {
|
||||
workdir_rules.push(WorkdirDelegationRule {
|
||||
target: WorkdirPath::new_scoped(child_bash_output_dir.to_string_lossy())
|
||||
.map_err(|error| ToolError::ExecutionFailed(error.to_string()))?,
|
||||
permission: WorkdirDelegationPermission::Read,
|
||||
recursive: true,
|
||||
});
|
||||
}
|
||||
let delegation_request = workdir_delegation_request(input.cwd.as_deref(), workdir_rules)?;
|
||||
let workdir_delegation = source_workdir_session
|
||||
.delegate(delegation_request)
|
||||
let workdir_tool_broker = require_workdir_tool_broker(self.workdir_tool_broker.as_ref())?;
|
||||
let tool_scope = workdir_tool_scope(input.cwd.as_deref(), workdir_rules, input.command)?;
|
||||
let workdir_scope = workdir_tool_broker
|
||||
.scope(tool_scope)
|
||||
.await
|
||||
.map_err(|error| {
|
||||
ToolError::InvalidArgument(format!("delegate Workdir session: {error}"))
|
||||
ToolError::InvalidArgument(format!("scope parent-owned Workdir tools: {error}"))
|
||||
})?;
|
||||
let child_workdir_tool_broker = workdir_scope.broker();
|
||||
|
||||
let spawn_selector =
|
||||
parse_spawn_profile_selector(input.profile.as_deref()).map_err(|msg| {
|
||||
@@ -490,7 +484,6 @@ impl Tool for SubWorkerSpawnTool {
|
||||
)
|
||||
.await
|
||||
.map_err(|error| ToolError::ExecutionFailed(format!("build Internal Worker: {error}")))?;
|
||||
child.bind_workdir_session(Some(workdir_delegation.scoped_session.clone()));
|
||||
child
|
||||
.add_scope_rules([ScopeRule {
|
||||
target: child_bash_output_dir.clone(),
|
||||
@@ -510,6 +503,7 @@ impl Tool for SubWorkerSpawnTool {
|
||||
self.runtime_base.clone(),
|
||||
child_registry.clone(),
|
||||
None,
|
||||
Some(child_workdir_tool_broker.clone()),
|
||||
)
|
||||
.await
|
||||
.map_err(|error| {
|
||||
@@ -552,6 +546,7 @@ impl Tool for SubWorkerSpawnTool {
|
||||
);
|
||||
parent_notifications.notify(message, true);
|
||||
})),
|
||||
Some(child_workdir_tool_broker.clone()),
|
||||
)
|
||||
.await;
|
||||
let session = session_result.map_err(|error| {
|
||||
@@ -621,7 +616,7 @@ impl Tool for SubWorkerSpawnTool {
|
||||
let record = crate::spawn::registry::InternalSpawnedWorkerRecord::new(
|
||||
input.name.clone(),
|
||||
scope_allow,
|
||||
workdir_delegation,
|
||||
workdir_scope,
|
||||
#[cfg(test)]
|
||||
installed_tools,
|
||||
session.clone(),
|
||||
@@ -674,18 +669,18 @@ fn logical_workdir_path(value: &str, field: &str) -> Result<FsPath, ToolError> {
|
||||
})
|
||||
}
|
||||
|
||||
fn parse_workdir_scope(rules: &[ScopeRuleInput]) -> Result<Vec<WorkdirDelegationRule>, ToolError> {
|
||||
fn parse_workdir_scope(rules: &[ScopeRuleInput]) -> Result<Vec<WorkdirToolScopeRule>, ToolError> {
|
||||
if rules.is_empty() {
|
||||
return Err(ToolError::InvalidArgument("scope must not be empty".into()));
|
||||
}
|
||||
rules
|
||||
.iter()
|
||||
.map(|rule| {
|
||||
Ok(WorkdirDelegationRule {
|
||||
Ok(WorkdirToolScopeRule {
|
||||
target: logical_workdir_path(&rule.target, "scope.target")?,
|
||||
permission: match rule.permission {
|
||||
PermissionInput::Read => WorkdirDelegationPermission::Read,
|
||||
PermissionInput::Write => WorkdirDelegationPermission::Write,
|
||||
PermissionInput::Read => WorkdirToolScopePermission::Read,
|
||||
PermissionInput::Write => WorkdirToolScopePermission::Write,
|
||||
},
|
||||
recursive: rule.recursive,
|
||||
})
|
||||
@@ -693,22 +688,24 @@ fn parse_workdir_scope(rules: &[ScopeRuleInput]) -> Result<Vec<WorkdirDelegation
|
||||
.collect()
|
||||
}
|
||||
|
||||
fn workdir_delegation_request(
|
||||
fn workdir_tool_scope(
|
||||
cwd: Option<&str>,
|
||||
rules: Vec<WorkdirDelegationRule>,
|
||||
) -> Result<WorkdirDelegationRequest, ToolError> {
|
||||
Ok(WorkdirDelegationRequest {
|
||||
rules: Vec<WorkdirToolScopeRule>,
|
||||
command: bool,
|
||||
) -> Result<WorkdirToolScope, ToolError> {
|
||||
Ok(WorkdirToolScope {
|
||||
rules,
|
||||
cwd: logical_workdir_path(cwd.unwrap_or("."), "cwd")?,
|
||||
command,
|
||||
})
|
||||
}
|
||||
|
||||
fn require_active_workdir_session(
|
||||
session: Option<&WorkdirSessionHandle>,
|
||||
) -> Result<&WorkdirSessionHandle, ToolError> {
|
||||
session.ok_or_else(|| {
|
||||
fn require_workdir_tool_broker(
|
||||
broker: Option<&WorkdirToolBroker>,
|
||||
) -> Result<&WorkdirToolBroker, ToolError> {
|
||||
broker.ok_or_else(|| {
|
||||
ToolError::InvalidArgument(
|
||||
"SubWorkerSpawn requires an active Workdir session; attach a Workdir before delegating filesystem access"
|
||||
"SubWorkerSpawn requires parent-owned Workdir tools; attach a Workdir before granting filesystem access"
|
||||
.to_string(),
|
||||
)
|
||||
})
|
||||
@@ -946,7 +943,7 @@ pub(crate) fn sub_worker_spawn_tool(
|
||||
runtime_base: PathBuf,
|
||||
bash_output_dir: PathBuf,
|
||||
workspace_root: PathBuf,
|
||||
source_workdir_session: Option<WorkdirSessionHandle>,
|
||||
workdir_tool_broker: Option<WorkdirToolBroker>,
|
||||
registry: Arc<SpawnedWorkerRegistry>,
|
||||
spawner_manifest: WorkerManifest,
|
||||
prompts: Arc<ArcSwap<PromptCatalog>>,
|
||||
@@ -958,7 +955,7 @@ pub(crate) fn sub_worker_spawn_tool(
|
||||
runtime_base,
|
||||
bash_output_dir,
|
||||
workspace_root,
|
||||
source_workdir_session,
|
||||
workdir_tool_broker,
|
||||
registry,
|
||||
spawner_manifest,
|
||||
prompts,
|
||||
@@ -972,7 +969,7 @@ fn sub_worker_spawn_tool_impl(
|
||||
runtime_base: PathBuf,
|
||||
bash_output_dir: PathBuf,
|
||||
workspace_root: PathBuf,
|
||||
source_workdir_session: Option<WorkdirSessionHandle>,
|
||||
workdir_tool_broker: Option<WorkdirToolBroker>,
|
||||
registry: Arc<SpawnedWorkerRegistry>,
|
||||
spawner_manifest: WorkerManifest,
|
||||
prompts: Arc<ArcSwap<PromptCatalog>>,
|
||||
@@ -1004,7 +1001,7 @@ fn sub_worker_spawn_tool_impl(
|
||||
runtime_base.clone(),
|
||||
bash_output_dir.clone(),
|
||||
workspace_root.clone(),
|
||||
source_workdir_session.clone(),
|
||||
workdir_tool_broker.clone(),
|
||||
registry.clone(),
|
||||
spawner_manifest.clone(),
|
||||
prompts.load_full().source(),
|
||||
@@ -1037,12 +1034,12 @@ mod tests {
|
||||
};
|
||||
|
||||
#[test]
|
||||
fn missing_active_workdir_session_fails_deterministically() {
|
||||
let error = require_active_workdir_session(None).unwrap_err();
|
||||
fn missing_parent_workdir_tool_broker_fails_deterministically() {
|
||||
let error = require_workdir_tool_broker(None).unwrap_err();
|
||||
assert!(matches!(
|
||||
error,
|
||||
ToolError::InvalidArgument(message)
|
||||
if message.contains("requires an active Workdir session")
|
||||
if message.contains("requires parent-owned Workdir tools")
|
||||
));
|
||||
}
|
||||
|
||||
@@ -1079,6 +1076,7 @@ mod tests {
|
||||
let valid: SubWorkerSpawnInput = serde_json::from_value(serde_json::json!({
|
||||
"name":"reviewer","task":"review","profile":"builtin:reviewer",
|
||||
"scope":[{"target":"work","permission":"write"}],
|
||||
"command":true,
|
||||
"review":{"ticket_id":"T1"}
|
||||
}))
|
||||
.unwrap();
|
||||
@@ -1173,7 +1171,7 @@ enabled = false
|
||||
let fail_requests = Arc::new(AtomicBool::new(false));
|
||||
let prompt_loader = PromptCatalogSource::builtins_only();
|
||||
let (parent_method_tx, mut parent_method_rx) = mpsc::channel(8);
|
||||
let source_workdir_session = workdir::delegation_capable_session(Arc::new(
|
||||
let workdir_tool_broker = workdir::WorkdirToolBroker::new(Arc::new(
|
||||
workdir::LocalWorkdirSession::materialized_bound(
|
||||
workdir::Workdir::new("test-workdir"),
|
||||
workspace_root.clone(),
|
||||
@@ -1189,7 +1187,7 @@ enabled = false
|
||||
runtime.path().to_path_buf(),
|
||||
bash_output_dir.clone(),
|
||||
workspace_root.clone(),
|
||||
Some(source_workdir_session),
|
||||
Some(workdir_tool_broker),
|
||||
registry.clone(),
|
||||
manifest.clone(),
|
||||
prompt_loader,
|
||||
@@ -1212,7 +1210,8 @@ enabled = false
|
||||
"target": ".",
|
||||
"permission": "write",
|
||||
"recursive": true
|
||||
}]
|
||||
}],
|
||||
"command": true
|
||||
});
|
||||
|
||||
assert!(spawner_scope.snapshot().is_writable(&workspace_root));
|
||||
@@ -1247,15 +1246,6 @@ enabled = false
|
||||
let record = registry
|
||||
.get_internal("reviewer-child")
|
||||
.expect("Internal reviewer registry record");
|
||||
let child_bash_output_dir = bash_output_dir.join("sub-workers").join("reviewer-child");
|
||||
record
|
||||
.workdir_delegation
|
||||
.scoped_session
|
||||
.stat(workdir::StatRequest {
|
||||
path: WorkdirPath::new_scoped(child_bash_output_dir.to_string_lossy()).unwrap(),
|
||||
})
|
||||
.await
|
||||
.expect("local child retains read scope for its Bash output directory");
|
||||
for required in ["Read", "Write", "Edit", "Glob", "Grep", "Bash"] {
|
||||
assert!(
|
||||
record.installed_tools.iter().any(|name| name == required),
|
||||
@@ -1371,7 +1361,7 @@ enabled = false
|
||||
"Stopped terminal child must release its delegated Workdir session"
|
||||
);
|
||||
assert!(
|
||||
!record.workdir_delegation.is_active(),
|
||||
!record.workdir_tool_scope.is_active(),
|
||||
"stopped child must revoke cloned scoped sessions"
|
||||
);
|
||||
assert!(registry.get_internal("reviewer-child").is_some());
|
||||
@@ -1426,7 +1416,7 @@ enabled = false
|
||||
Arc::new(AvailableWorkspaceClient),
|
||||
);
|
||||
let remote_client = Arc::new(StrictRemoteWorkdirWorkspaceClient::default());
|
||||
let source_workdir_session = workdir::delegation_capable_session(
|
||||
let workdir_tool_broker = workdir::WorkdirToolBroker::new(
|
||||
WorkspaceAttachedWorkdirSession::handle(remote_client.clone()),
|
||||
);
|
||||
let calls = Arc::new(AtomicUsize::new(0));
|
||||
@@ -1438,7 +1428,7 @@ enabled = false
|
||||
runtime.path().to_path_buf(),
|
||||
bash_output_dir.clone(),
|
||||
workspace_root.clone(),
|
||||
Some(source_workdir_session),
|
||||
Some(workdir_tool_broker),
|
||||
registry.clone(),
|
||||
manifest,
|
||||
PromptCatalogSource::builtins_only(),
|
||||
@@ -1478,51 +1468,12 @@ enabled = false
|
||||
record.session.wait_until_idle().await,
|
||||
crate::internal_worker::InternalWorkerSessionStatus::Idle
|
||||
);
|
||||
assert!(record.installed_tools.iter().any(|tool| tool == "Write"));
|
||||
assert!(!record.installed_tools.iter().any(|tool| tool == "Bash"));
|
||||
assert_eq!(calls.load(Ordering::SeqCst), 1);
|
||||
assert_eq!(
|
||||
remote_client
|
||||
.foreign_scope_rejections
|
||||
.load(Ordering::SeqCst),
|
||||
0
|
||||
);
|
||||
let child_bash_output_dir = bash_output_dir.join("sub-workers").join("remote-child");
|
||||
assert!(child_bash_output_dir.is_dir());
|
||||
for required in ["Read", "Write", "Edit", "Glob", "Grep", "Bash"] {
|
||||
assert!(
|
||||
record.installed_tools.iter().any(|name| name == required),
|
||||
"remote write-scoped child is missing {required}: {:?}",
|
||||
record.installed_tools
|
||||
);
|
||||
}
|
||||
|
||||
let remote_requests = remote_client.requests();
|
||||
let operate_requests = remote_requests
|
||||
.iter()
|
||||
.filter(|request| request.body.is_some())
|
||||
.collect::<Vec<_>>();
|
||||
assert_eq!(
|
||||
operate_requests.len(),
|
||||
1,
|
||||
"remote requests: {remote_requests:?}"
|
||||
);
|
||||
let operation_body: serde_json::Value = serde_json::from_str(
|
||||
operate_requests[0]
|
||||
.body
|
||||
.as_deref()
|
||||
.expect("remote operation body"),
|
||||
)
|
||||
.unwrap();
|
||||
let rules = operation_body["delegations"][0]["rules"]
|
||||
.as_array()
|
||||
.expect("delegation rules");
|
||||
assert_eq!(rules.len(), 1, "remote operation body: {operation_body}");
|
||||
assert_eq!(rules[0]["target"], "");
|
||||
assert!(
|
||||
!operation_body.to_string().contains(
|
||||
child_bash_output_dir
|
||||
.to_str()
|
||||
.expect("UTF-8 test output directory")
|
||||
)
|
||||
remote_client.requests().is_empty(),
|
||||
"spawning a child must not open or delegate a provider Workdir session"
|
||||
);
|
||||
}
|
||||
|
||||
@@ -1534,6 +1485,7 @@ enabled = false
|
||||
.and_then(serde_json::Value::as_object)
|
||||
.expect("schema properties");
|
||||
assert!(properties.contains_key("cwd"), "schema: {schema}");
|
||||
assert!(properties.contains_key("command"), "schema: {schema}");
|
||||
let required = schema
|
||||
.get("required")
|
||||
.and_then(serde_json::Value::as_array)
|
||||
@@ -1663,7 +1615,6 @@ enabled = false
|
||||
#[derive(Debug, Default)]
|
||||
struct StrictRemoteWorkdirWorkspaceClient {
|
||||
requests: Mutex<Vec<WorkspaceRequest>>,
|
||||
foreign_scope_rejections: AtomicUsize,
|
||||
}
|
||||
|
||||
impl StrictRemoteWorkdirWorkspaceClient {
|
||||
@@ -1695,59 +1646,10 @@ enabled = false
|
||||
self.requests
|
||||
.lock()
|
||||
.expect("remote Workdir request lock")
|
||||
.push(request.clone());
|
||||
if request.path.ends_with("/fence") {
|
||||
return Ok(WorkspaceResponse {
|
||||
status: 200,
|
||||
body: serde_json::json!({ "value": "remote-fence-1" }).to_string(),
|
||||
});
|
||||
}
|
||||
|
||||
let body: serde_json::Value = serde_json::from_str(
|
||||
request
|
||||
.body
|
||||
.as_deref()
|
||||
.ok_or_else(|| WorkspaceClientError::Request("missing request body".into()))?,
|
||||
)
|
||||
.map_err(|error| WorkspaceClientError::Request(error.to_string()))?;
|
||||
let has_foreign_scope = body
|
||||
.get("delegations")
|
||||
.and_then(serde_json::Value::as_array)
|
||||
.into_iter()
|
||||
.flatten()
|
||||
.flat_map(|delegation| {
|
||||
delegation
|
||||
.get("rules")
|
||||
.and_then(serde_json::Value::as_array)
|
||||
.into_iter()
|
||||
.flatten()
|
||||
})
|
||||
.filter_map(|rule| rule.get("target").and_then(serde_json::Value::as_str))
|
||||
.any(|target| Path::new(target).is_absolute());
|
||||
if has_foreign_scope {
|
||||
self.foreign_scope_rejections.fetch_add(1, Ordering::SeqCst);
|
||||
return Ok(WorkspaceResponse {
|
||||
status: 403,
|
||||
body: serde_json::json!({
|
||||
"code": "out_of_scope",
|
||||
"message": "Worker-host path is outside the remote Workdir namespace"
|
||||
})
|
||||
.to_string(),
|
||||
});
|
||||
}
|
||||
|
||||
Ok(WorkspaceResponse {
|
||||
status: 200,
|
||||
body: serde_json::json!({
|
||||
"operation": "stat",
|
||||
"result": {
|
||||
"path": "",
|
||||
"kind": "directory",
|
||||
"size": 0
|
||||
}
|
||||
})
|
||||
.to_string(),
|
||||
})
|
||||
.push(request);
|
||||
Err(WorkspaceClientError::Request(
|
||||
"SubWorker spawn must not call the remote Workdir provider".into(),
|
||||
))
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -332,6 +332,7 @@ async fn shutdown_closes_bound_workdir_session() {
|
||||
command: "sleep 30".to_owned(),
|
||||
timeout_secs: 60,
|
||||
output_limit: 1024,
|
||||
cwd: None,
|
||||
spill_dir: None,
|
||||
tool_call_id: None,
|
||||
})
|
||||
@@ -376,6 +377,7 @@ async fn controller_projects_workdir_command_events_and_snapshot_state() {
|
||||
command: "printf ready; sleep 0.3; printf done".to_owned(),
|
||||
timeout_secs: 5,
|
||||
output_limit: 1024,
|
||||
cwd: None,
|
||||
spill_dir: None,
|
||||
tool_call_id: Some("tool-command-1".into()),
|
||||
})
|
||||
@@ -484,6 +486,7 @@ async fn controller_refreshes_command_snapshot_after_high_output_provider_lag()
|
||||
.to_owned(),
|
||||
timeout_secs: 10,
|
||||
output_limit: 1024,
|
||||
cwd: None,
|
||||
spill_dir: None,
|
||||
tool_call_id: Some("tool-high-output".into()),
|
||||
})
|
||||
@@ -560,6 +563,7 @@ async fn controller_startup_failure_closes_bound_workdir_session() {
|
||||
command: "printf unreachable".to_owned(),
|
||||
timeout_secs: 5,
|
||||
output_limit: 1024,
|
||||
cwd: None,
|
||||
spill_dir: None,
|
||||
tool_call_id: None,
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user