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

fix: simplify Ticket reporting and speed up browser lists

Browse Source 
Remove model-supplied assignment and retry identities from implementation reports; retain Host idempotency, authentication and notification evidence.

Add a SQL-only browser projection and decouple auxiliary page loads. Preserve authoritative MR evidence queries. Include regression tests and component benchmarks, documenting the remaining no-match search cost.
This commit is contained in:
Hare
2026-10-16 01:58:15 +09:00
parent 9f1cbbd385
commit b67aa2de7d
42 changed files with 3197 additions and 152 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/worker/src/feature/builtin/ticket/assignment_public.rs
+27 -19
View File
@@ -91,8 +91,9 @@ struct AnswerInput {
#[serde(deny_unknown_fields)]
struct ImplementationResultInput {
ticket: String,
#[serde(flatten)]
request: server_api::TicketImplementationResultRequest,
kind: server_api::TicketImplementationResultKind,
body: String,
merge_request: Option<server_api::MergeRequestReviewSubject>,
}
#[derive(Deserialize, JsonSchema)]
#[serde(deny_unknown_fields)]
@@ -157,7 +158,7 @@ pub(super) fn description(name: &str) -> &'static str {
"Append continuing input to the observed current waiting/inprogress assignment with assignment_id, expected_last_event_id, stable operation_key and bounded answer body. Query current tracking first. Answers continue the same row without automatic inprogress; done requires explicit new Assign/reissue instead. Backend authenticates/fences source, current assignment and event CAS, then saves input/audit/exact receipt atomically and notifies committed input. Exact retry retains full input/key; stale/cleared/replaced conflicts require rereading, and unknown outcome must not be retried as a fresh operation. No model actor/role claims, new assignment, case closure or Worker start."
}
"TicketImplementationResult" => {
"Save an explicit handoff, result or blocker for the current implementation assignment and its original Orchestrator delegation. Read ShowTicket first for assignment identities. Backend authenticates the implementation Worker and fences both assignments; this is not AssignmentAnswer or a handling update. Optional MR/source identifies retained evidence, never approval. Retry an unknown response using exactly the same operation_key and full input; changed input with the same key is rejected. Result and durable notification source commit together. No case closure, new Assign, Worker stop or integration occurs."
"Report facts as the authenticated active Workspace Worker for this Ticket: an explicit handoff, result or blocker with kind, bounded body and optional retained MR/source evidence. No current or pinned implementation assignment or Orchestrator delegation match is required; do not supply assignment identities or actor claims. The Host owns transport retry identity: retries of the same tool invocation and full input are idempotent; a new invocation is a new report. If the outcome is uncertain, reconcile saved evidence rather than blindly submitting a new invocation. This is not AssignmentAnswer, a handling update or approval. Result and durable notification source commit together. No case closure, new Assign, Worker stop or integration occurs."
}
"TicketImplementationResultQuery" => {
"Read one saved implementation result by Ticket and result_id from a result notification/receipt. Historical evidence is not a new request or approval; reread current delegation and MR before acting."
@@ -196,7 +197,11 @@ struct PublicAssignmentTool {
}
#[async_trait]
impl Tool for PublicAssignmentTool {
async fn execute(&self, input: &str, _: ToolExecutionContext) -> Result<ToolOutput, ToolError> {
async fn execute(
&self,
input: &str,
ctx: ToolExecutionContext,
) -> Result<ToolOutput, ToolError> {
self.client
.workspace_id()
.filter(|id| !id.trim().is_empty())
@@ -282,31 +287,34 @@ impl Tool for PublicAssignmentTool {
let input: ImplementationResultInput =
serde_json::from_str(input).map_err(parse)?;
identity("ticket", &input.ticket)?;
for (name, value) in [
("assignment_id", &input.request.assignment_id),
(
"orchestrator_assignment_id",
&input.request.orchestrator_assignment_id,
),
("operation_key", &input.request.operation_key),
] {
identity(name, value)?;
}
if input.request.body.trim().is_empty()
|| input.request.body.len() > 16384
|| input.request.body.contains('\0')
// Retry identity belongs to the Host, never model input. Missing trusted
// context must fail closed rather than minting a replacement identity.
identity("tool call_id", &ctx.call_id)?;
let operation_key = format!(
"ticket-implementation-result:{}:{}",
input.ticket, ctx.call_id
);
identity("operation_key", &operation_key)?;
if input.body.trim().is_empty()
|| input.body.len() > 16384
|| input.body.contains('\0')
{
return Err(ToolError::InvalidArgument(
"Result body must be nonblank, NUL-free and at most 16384 bytes".into(),
));
}
if let Some(mr) = &input.request.merge_request {
if let Some(mr) = &input.merge_request {
identity("merge_request_id", &mr.merge_request_id)?;
identity("subject_ref", &mr.subject_ref)?;
}
WorkspaceServerOperation::TicketImplementationResult {
ticket_id: input.ticket,
request: input.request,
request: server_api::TicketImplementationResultRequest {
operation_key,
kind: input.kind,
body: input.body,
merge_request: input.merge_request,
},
}
}
PublicAssignmentKind::ImplementationResultQuery => {
crates/worker/src/feature/builtin/ticket/assignment_public/tests.rs
+299 -3
View File
@@ -59,13 +59,13 @@ fn cases() -> Vec<(PublicAssignmentKind, Value, Value)> {
),
(
PublicAssignmentKind::ImplementationResult,
json!({"ticket":"T-743","assignment_id":"implementation","orchestrator_assignment_id":"delegation","operation_key":"result-1","kind":"result","body":"Research without an MR","merge_request":null}),
json!({"result_id":"result-id","ticket_id":"T-743","created_at":"now","request":{"assignment_id":"implementation","orchestrator_assignment_id":"delegation","operation_key":"result-1","kind":"result","body":"Research without an MR","merge_request":null}}),
json!({"ticket":"T-743","kind":"result","body":"Research without an MR","merge_request":null}),
json!({"result_id":"result-id","ticket_id":"T-743","created_at":"now","request":{"operation_key":"result-1","kind":"result","body":"Research without an MR","merge_request":null}}),
),
(
PublicAssignmentKind::ImplementationResultQuery,
json!({"ticket":"T-743","result_id":"result-id"}),
json!({"result_id":"result-id","ticket_id":"T-743","created_at":"now","request":{"assignment_id":"implementation","orchestrator_assignment_id":"delegation","operation_key":"result-1","kind":"result","body":"Research without an MR","merge_request":null}}),
json!({"result_id":"result-id","ticket_id":"T-743","created_at":"now","request":{"operation_key":"result-1","kind":"result","body":"Research without an MR","merge_request":null}}),
),
(
PublicAssignmentKind::Answer,
@@ -175,6 +175,203 @@ async fn assign_reissue_and_clear_forward_observed_identity_and_exact_retry_with
}
}
}
#[tokio::test]
async fn implementation_result_schema_exposes_only_report_facts_and_rejects_forged_fields() {
let (kind, input, response) = cases().remove(4);
let client = transport(200, response);
let (meta, tool) = definition(client.clone(), kind)();
let properties = meta.input_schema["properties"].as_object().unwrap();
assert_eq!(properties.len(), 4);
for field in ["ticket", "kind", "body", "merge_request"] {
assert!(properties.contains_key(field), "missing {field}");
}
assert_eq!(meta.input_schema["additionalProperties"], false);
let schema = jsonschema::validator_for(&meta.input_schema).unwrap();
let mut minimal = input.clone();
minimal.as_object_mut().unwrap().remove("merge_request");
assert!(schema.is_valid(&minimal));
assert!(serde_json::from_value::<ImplementationResultInput>(minimal).is_ok());
for field in [
"operation_key",
"assignment_id",
"orchestrator_assignment_id",
"result_id",
"call_id",
"request",
"actor",
"runtime_id",
"worker_id",
] {
let mut forged = input.clone();
forged[field] = json!("forged");
assert!(!schema.is_valid(&forged), "schema accepts {field}");
assert!(serde_json::from_value::<ImplementationResultInput>(forged.clone()).is_err());
assert!(matches!(
tool.execute(&forged.to_string(), ToolExecutionContext::direct())
.await,
Err(ToolError::InvalidArgument(_))
));
}
assert!(client.requests.lock().unwrap().is_empty());
let query = PublicAssignmentKind::ImplementationResultQuery.schema();
assert!(query["properties"].get("result_id").is_some());
assert!(
query["required"]
.as_array()
.unwrap()
.contains(&json!("result_id"))
);
}
#[tokio::test]
async fn implementation_result_transport_key_uses_only_trusted_invocation_identity() {
let (kind, input, response) = cases().remove(4);
let client = transport(200, response);
let (_, tool) = definition(client.clone(), kind)();
for ctx in [
ToolExecutionContext::new("call-a", "batch-a", 0),
ToolExecutionContext::new("call-a", "batch-b", 7),
ToolExecutionContext::new("call-b", "batch-a", 0),
] {
tool.execute(&input.to_string(), ctx).await.unwrap();
}
let requests = client.requests.lock().unwrap();
assert_eq!(requests.len(), 3);
assert_eq!(requests[0], requests[1]);
assert_ne!(requests[0], requests[2]);
for (request, call_id) in requests.iter().zip(["call-a", "call-a", "call-b"]) {
assert_eq!(request.method, WorkspaceRequestMethod::Post);
assert!(
request
.path
.ends_with("/tickets/T-743/implementation-results")
);
let body: Value = serde_json::from_str(request.body.as_deref().unwrap()).unwrap();
let mut expected = input.clone();
expected.as_object_mut().unwrap().remove("ticket");
expected["operation_key"] = json!(format!("ticket-implementation-result:T-743:{call_id}"));
assert_eq!(body, expected);
assert!(body.get("assignment_id").is_none());
assert!(body.get("orchestrator_assignment_id").is_none());
}
}
#[tokio::test]
async fn implementation_result_missing_or_invalid_context_fails_before_dispatch() {
let (kind, input, response) = cases().remove(4);
let client = transport(200, response);
let (_, tool) = definition(client.clone(), kind)();
for call_id in [
String::new(),
" \t".into(),
"call\nforged".into(),
"x".repeat(1025),
] {
let error = tool
.execute(
&input.to_string(),
ToolExecutionContext::new(call_id, "batch", 0),
)
.await
.unwrap_err();
assert!(matches!(error, ToolError::InvalidArgument(_)));
assert!(error.to_string().contains("tool call_id"));
}
// A valid call ID must also fit the bounded transport key after namespacing.
assert!(matches!(
tool.execute(
&input.to_string(),
ToolExecutionContext::new("x".repeat(1024), "batch", 0),
)
.await,
Err(ToolError::InvalidArgument(_))
));
assert!(client.requests.lock().unwrap().is_empty());
}
#[tokio::test]
async fn implementation_result_validates_report_body_ticket_kind_and_optional_mr() {
let (kind, input, response) = cases().remove(4);
let client = transport(200, response);
let (meta, tool) = definition(client.clone(), kind)();
let schema = jsonschema::validator_for(&meta.input_schema).unwrap();
let mut invalid_inputs = vec![];
for body in [
String::new(),
" \n".into(),
"x".repeat(16385),
"text\0hidden".into(),
"é".repeat(8193),
] {
let mut invalid = input.clone();
invalid["body"] = json!(body);
invalid_inputs.push(invalid);
}
for value in [String::new(), " \t".into(), "x\n".into(), "x".repeat(1025)] {
let mut invalid = input.clone();
invalid["ticket"] = json!(value);
invalid_inputs.push(invalid);
for field in ["merge_request_id", "subject_ref"] {
let mut invalid = input.clone();
invalid["merge_request"] = json!({"merge_request_id":"MR-1","subject_ref":"source"});
invalid["merge_request"][field] = json!(value);
invalid_inputs.push(invalid);
}
}
for value in [json!("done"), json!(null), json!(42)] {
let mut invalid = input.clone();
invalid["kind"] = value;
assert!(!schema.is_valid(&invalid));
invalid_inputs.push(invalid);
}
for mr in [
json!({"merge_request_id":"MR-1"}),
json!({"merge_request_id":"MR-1","subject_ref":"source","operation_key":"forged"}),
] {
let mut invalid = input.clone();
invalid["merge_request"] = mr;
assert!(!schema.is_valid(&invalid));
invalid_inputs.push(invalid);
}
for invalid in invalid_inputs {
assert!(matches!(
tool.execute(&invalid.to_string(), ToolExecutionContext::direct())
.await,
Err(ToolError::InvalidArgument(_))
));
}
assert!(client.requests.lock().unwrap().is_empty());
for (index, report_kind) in ["handoff", "result", "blocker"].into_iter().enumerate() {
let mut valid = input.clone();
valid["kind"] = json!(report_kind);
valid["body"] = json!("é".repeat(8192));
if index == 0 {
valid.as_object_mut().unwrap().remove("merge_request");
} else if index == 2 {
valid["merge_request"] = json!({"merge_request_id":"MR-1","subject_ref":"source"});
}
assert!(schema.is_valid(&valid));
tool.execute(
&valid.to_string(),
ToolExecutionContext::new(format!("call-{index}"), "batch", index),
)
.await
.unwrap();
}
assert_eq!(client.requests.lock().unwrap().len(), 3);
}
#[test]
fn implementation_result_guidance_keeps_retry_identity_host_owned() {
let description = description("TicketImplementationResult");
assert!(description.contains("authenticated active Workspace Worker"));
assert!(description.contains("Host owns transport retry identity"));
assert!(description.contains("a new invocation is a new report"));
assert!(description.contains("rather than blindly submitting"));
assert!(!description.contains("operation_key"));
assert!(!description.contains("fences both assignments"));
}
#[tokio::test]
async fn continuing_answer_forwards_same_assignment_event_cas_and_saved_receipt() {
let (kind, input, response) = cases().pop().unwrap();
@@ -511,6 +708,105 @@ async fn invalid_answer_body_and_assignment_identities_fail_before_dispatch() {
assert!(client.requests.lock().unwrap().is_empty());
}
#[tokio::test]
async fn implementation_result_wip_and_native_dispatch_preserve_trusted_call_context() {
let (kind, input, response) = cases().remove(4);
let client = transport(200, response);
let (_, direct_tool) = definition(client.clone(), kind)();
direct_tool
.execute(
&input.to_string(),
ToolExecutionContext::new("call-a", "native", 0),
)
.await
.unwrap();
let tools = native_ticket_tools(client.clone(), TicketFeatureAccess::workflow()).unwrap();
let handler = TicketItemWipHandler {
tools: operation_map(
tools
.into_iter()
.filter(|t| t.projection.surface == NativeTicketSurface::Item)
.collect(),
),
permissions: None,
ticket_reference: "T-743".into(),
observations: Arc::new(Mutex::new(TicketObservationState::default())),
};
let args: BTreeMap<_, _> = input
.as_object()
.unwrap()
.iter()
.filter(|(key, _)| key.as_str() != "ticket")
.map(|(key, value)| (key.clone(), json_to_wip(value).unwrap()))
.collect();
for call_id in ["call-a", "call-b"] {
handler
.call(
"implementation_result",
&args,
WipCallContext {
execution: ToolExecutionContext::new(call_id, "wip", 3),
security_context: "worker".into(),
},
)
.await
.unwrap_or_else(|_| panic!("WIP implementation_result rejected {call_id}"));
}
{
let requests = client.requests.lock().unwrap();
assert_eq!(requests.len(), 3);
assert_eq!(requests[0], requests[1]);
assert_ne!(requests[1], requests[2]);
let body: Value = serde_json::from_str(requests[2].body.as_deref().unwrap()).unwrap();
assert_eq!(
body["operation_key"],
"ticket-implementation-result:T-743:call-b"
);
}
assert!(matches!(
handler
.call(
"implementation_result",
&args,
WipCallContext {
execution: ToolExecutionContext::new("", "wip", 0),
security_context: "worker".into(),
},
)
.await,
Err(WipOperationError::Protocol(ProtocolError {
code: ProtocolErrorCode::InvalidArguments,
..
}))
));
for field in [
"operation_key",
"assignment_id",
"orchestrator_assignment_id",
"call_id",
] {
let mut forged = args.clone();
forged.insert(field.into(), WipValue::String("forged".into()));
assert!(matches!(
handler
.call(
"implementation_result",
&forged,
WipCallContext {
execution: ToolExecutionContext::new("call-c", "wip", 0),
security_context: "worker".into(),
},
)
.await,
Err(WipOperationError::Protocol(ProtocolError {
code: ProtocolErrorCode::InvalidArguments,
..
}))
));
}
assert_eq!(client.requests.lock().unwrap().len(), 3);
}
#[tokio::test]
async fn assignment_wip_binds_ticket_and_preserves_normal_permission_and_cas_inputs() {
for (kind, mut input, response) in cases() {