fix: explain safe WorkdirDelete recovery in tool output
This commit is contained in:
@@ -734,8 +734,14 @@ impl WorkspaceHttpWorkdirBackend {
|
|||||||
))?;
|
))?;
|
||||||
workdir_output(
|
workdir_output(
|
||||||
format!(
|
format!(
|
||||||
"Workdir {workdir_id} removal disposition: {:?}",
|
"Workdir {workdir_id} removal disposition: {:?}. {}{}",
|
||||||
response.disposition
|
response.disposition,
|
||||||
|
removal_guidance(response.failure_category.as_deref()),
|
||||||
|
if response.retryable {
|
||||||
|
" After resolving the cause, repeat this WorkdirDelete with the same id and reason; current safety conditions will be rechecked."
|
||||||
|
} else {
|
||||||
|
""
|
||||||
|
}
|
||||||
),
|
),
|
||||||
&response,
|
&response,
|
||||||
)
|
)
|
||||||
@@ -920,6 +926,48 @@ fn decode_error(error: serde_json::Error) -> ToolError {
|
|||||||
ToolError::ExecutionFailed(format!("decode Workspace Workdir response: {error}"))
|
ToolError::ExecutionFailed(format!("decode Workspace Workdir response: {error}"))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn removal_guidance(category: Option<&str>) -> &'static str {
|
||||||
|
match category {
|
||||||
|
None => "",
|
||||||
|
Some("mount_present") => {
|
||||||
|
"A mount remains inside the Workdir. Confirm its owner and usage, and have that owner safely release it. Do not recursively delete mounted data or implicitly unmount external or unknown resources."
|
||||||
|
}
|
||||||
|
Some("mount_check_unavailable") => "Restore Runtime mount inspection before deletion.",
|
||||||
|
Some("changes_present" | "dirty_or_unknown") => {
|
||||||
|
"Current changes are protected; preserve or resolve them before deletion."
|
||||||
|
}
|
||||||
|
Some("changes_unknown") => {
|
||||||
|
"Restore checkout inspection or preserve remaining content so current changes can be verified."
|
||||||
|
}
|
||||||
|
Some(
|
||||||
|
"ownership_unknown"
|
||||||
|
| "provider_identity_changed"
|
||||||
|
| "authority_changed"
|
||||||
|
| "authority_invalid",
|
||||||
|
) => {
|
||||||
|
"Confirm target identity and ownership with its owner; never remove a replacement or unknown resource."
|
||||||
|
}
|
||||||
|
Some("blocked_by_live_authority") => {
|
||||||
|
"Release current attachments, reservations and holds through their owning authority."
|
||||||
|
}
|
||||||
|
Some("permission_denied" | "resource_busy" | "storage_unavailable") => {
|
||||||
|
"Have the resource owner resolve access, usage or storage safely; do not force-clean or change shared resource permissions."
|
||||||
|
}
|
||||||
|
Some("runtime_unavailable" | "provider_unavailable") => {
|
||||||
|
"Restore the Runtime connection; the registry remains until deletion is confirmed."
|
||||||
|
}
|
||||||
|
Some("provider_observation_unknown" | "provider_cleanup_outcome_unknown") => {
|
||||||
|
"Deletion is not confirmed. Re-observe with normal removal after restoring provider access; the registry is retained."
|
||||||
|
}
|
||||||
|
Some("unsupported_target") => {
|
||||||
|
"Ask the owner for the provider's supported cleanup operation."
|
||||||
|
}
|
||||||
|
Some(_) => {
|
||||||
|
"Inspect correlated Runtime cleanup diagnostics with the owning operator and resolve the current cause."
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
fn workdir_output<T: Serialize>(summary: String, value: &T) -> Result<ToolOutput, ToolError> {
|
fn workdir_output<T: Serialize>(summary: String, value: &T) -> Result<ToolOutput, ToolError> {
|
||||||
Ok(ToolOutput {
|
Ok(ToolOutput {
|
||||||
summary,
|
summary,
|
||||||
@@ -1218,6 +1266,45 @@ mod tests {
|
|||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn ordinary_workdir_delete_reports_safe_cause_and_exact_retry_guidance() {
|
||||||
|
let client = Arc::new(RecordingWorkspaceClient::new(vec![response(json!({
|
||||||
|
"working_directory_id": "wd-retry",
|
||||||
|
"disposition": "attention_required",
|
||||||
|
"retryable": true,
|
||||||
|
"failure_category": "mount_present"
|
||||||
|
}))]));
|
||||||
|
let backend = WorkspaceHttpWorkdirBackend::new(client.clone());
|
||||||
|
let output = backend
|
||||||
|
.delete(WorkdirDeleteInput {
|
||||||
|
working_directory_id: "wd-retry".into(),
|
||||||
|
reason: "remove unused Workdir".into(),
|
||||||
|
})
|
||||||
|
.unwrap();
|
||||||
|
assert!(output.summary.contains("Confirm its owner"));
|
||||||
|
assert!(
|
||||||
|
output
|
||||||
|
.summary
|
||||||
|
.contains("Do not recursively delete mounted data")
|
||||||
|
);
|
||||||
|
assert!(output.summary.contains("same id and reason"));
|
||||||
|
assert!(output.summary.len() <= 600);
|
||||||
|
let content: serde_json::Value =
|
||||||
|
serde_json::from_str(output.content.as_deref().unwrap()).unwrap();
|
||||||
|
assert_eq!(content["failure_category"], "mount_present");
|
||||||
|
assert_eq!(content["retryable"], true);
|
||||||
|
let requests = client.requests();
|
||||||
|
assert_eq!(requests[0].method, WorkspaceRequestMethod::Delete);
|
||||||
|
assert_eq!(
|
||||||
|
serde_json::from_str::<serde_json::Value>(requests[0].body.as_deref().unwrap())
|
||||||
|
.unwrap()["reason"],
|
||||||
|
"remove unused Workdir"
|
||||||
|
);
|
||||||
|
let unknown = removal_guidance(Some("secret /private/provider/path"));
|
||||||
|
assert!(!unknown.contains("/private"));
|
||||||
|
assert!(!unknown.contains("secret"));
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn workspace_request_failure_is_not_reported_as_session_unavailable() {
|
fn workspace_request_failure_is_not_reported_as_session_unavailable() {
|
||||||
let error = workspace_workdir_error(WorkspaceClientError::Request(
|
let error = workspace_workdir_error(WorkspaceClientError::Request(
|
||||||
|
|||||||
@@ -0,0 +1,34 @@
|
|||||||
|
# SubWorker / Workdir execution ergonomics during T-730
|
||||||
|
|
||||||
|
Observed during Workdir removal implementation in a dedicated clean checkout:
|
||||||
|
|
||||||
|
- Children using `profile: builtin:default` were given explicit writable scope
|
||||||
|
and command permission, but their available tools/context did not support the
|
||||||
|
requested implementation. They performed read-only investigation and reported
|
||||||
|
the missing capabilities. Recreating coding children with `profile: inherit`
|
||||||
|
exposed the needed implementation/Ticket tools. No rejected capability was
|
||||||
|
bypassed.
|
||||||
|
- Concurrent Workdir reads sometimes returned `filesystem provider busy`.
|
||||||
|
This is transient provider contention, not an authorization rejection. A
|
||||||
|
child initially treated it as a stop condition; a subsequent parent turn
|
||||||
|
clarified the difference and sequential retries proceeded normally.
|
||||||
|
- After a user interruption, repository changes and parent tasks remained, but
|
||||||
|
`WorkerList` no longer listed the previously active children. Their work was
|
||||||
|
not treated as approved or complete: the parent inspected concrete diffs,
|
||||||
|
reran Web/browser validation and delegated a fresh bounded Runtime follow-up.
|
||||||
|
|
||||||
|
Potential improvements:
|
||||||
|
|
||||||
|
1. Show effective child tool capabilities together with filesystem/command
|
||||||
|
grants before spawn, so a writable scope is not mistaken for an available
|
||||||
|
implementation tool surface.
|
||||||
|
2. Distinguish retryable provider contention from capability rejection in typed
|
||||||
|
error metadata and recommended recovery text.
|
||||||
|
3. Make interruption disposition and retained child-session observation grants
|
||||||
|
explicit. Preserved files are useful evidence, but they do not replace a
|
||||||
|
committed final child report or verification of tests.
|
||||||
|
|
||||||
|
These observations concern development-tool ergonomics, not product removal
|
||||||
|
requirements or permission to operate on live Workdirs/mounts. The T-730 work
|
||||||
|
uses temporary fixtures and ordinary Git publication; no live cleanup, unmount,
|
||||||
|
registry edits or runtime deployment were performed.
|
||||||
Reference in New Issue
Block a user