fix(worker): preserve Ticket WIP outcome semantics

This commit is contained in:
2026-10-04 02:38:55 +09:00
parent 31ee6b3c6a
commit 983c20c3f7
3 changed files with 172 additions and 41 deletions
+19 -3
View File
@@ -1545,11 +1545,15 @@ fn model_ticket_reference(
) -> Result<String, ToolError> { ) -> Result<String, ToolError> {
let ticket = backend let ticket = backend
.show(TicketIdOrSlug::Id(reference.to_string())) .show(TicketIdOrSlug::Id(reference.to_string()))
.map_err(|error| backend_error(tool_name, error))?; .map_err(|error| {
ToolError::Internal(format!(
"{tool_name} outcome unknown after mutation while resolving Ticket reference: {error}"
))
})?;
match ticket.meta.resource_key { match ticket.meta.resource_key {
Some(resource_key) if is_canonical_ticket_resource_key(&resource_key) => Ok(resource_key), Some(resource_key) if is_canonical_ticket_resource_key(&resource_key) => Ok(resource_key),
Some(_) => Err(ToolError::ExecutionFailed(format!( Some(_) => Err(ToolError::Internal(format!(
"{tool_name} failed: required Ticket key is unavailable" "{tool_name} outcome unknown after mutation: required Ticket key is unavailable"
))), ))),
None => Ok(ticket.meta.id), None => Ok(ticket.meta.id),
} }
@@ -1998,6 +2002,18 @@ mod tests {
.expect("tool exists") .expect("tool exists")
} }
#[test]
fn post_mutation_reference_failures_are_outcome_unknown() {
let temp = TempDir::new().unwrap();
let backend = TicketToolBackend::new(backend(&temp));
let error = model_ticket_reference(&backend, "missing", "TicketClose").unwrap_err();
assert!(matches!(
error,
ToolError::Internal(message)
if message.contains("outcome unknown after mutation")
));
}
#[test] #[test]
fn ticket_tool_name_partitions_are_explicit() { fn ticket_tool_name_partitions_are_explicit() {
assert_eq!( assert_eq!(
+150 -35
View File
@@ -1604,6 +1604,9 @@ impl WipOperationHandler for TicketItemWipHandler {
&response, &response,
tool.projection.mutating, tool.projection.mutating,
); );
if tool.projection.mutating {
record_affected_ticket_mutations(&self.revisions, &self.ticket_reference, &response);
}
Ok(WipOperationOutput::native( Ok(WipOperationOutput::native(
json_to_wip(&response).map_err(WipOperationError::OutcomeUnknown)?, json_to_wip(&response).map_err(WipOperationError::OutcomeUnknown)?,
)) ))
@@ -1652,9 +1655,6 @@ fn native_ticket_input(
} }
let mut input = serde_json::Map::new(); let mut input = serde_json::Map::new();
for (name, value) in arguments { for (name, value) in arguments {
if matches!(value, WipValue::Unit) {
continue;
}
input.insert( input.insert(
name.clone(), name.clone(),
wip_to_json(value).map_err(|message| { wip_to_json(value).map_err(|message| {
@@ -1702,7 +1702,7 @@ fn map_ticket_tool_error(error: ToolError) -> WipOperationError {
ProtocolErrorCode::InvalidArguments, ProtocolErrorCode::InvalidArguments,
format!("{code}: {message}"), format!("{code}: {message}"),
), ),
ToolError::ExecutionFailed(message) | ToolError::Internal(message) => { ToolError::ExecutionFailed(message) => {
if message.contains("HTTP status 401") || message.contains("HTTP status 403") { if message.contains("HTTP status 401") || message.contains("HTTP status 403") {
protocol_failure(ProtocolErrorCode::PermissionDenied, message) protocol_failure(ProtocolErrorCode::PermissionDenied, message)
} else if message.contains("HTTP status 404") { } else if message.contains("HTTP status 404") {
@@ -1716,6 +1716,7 @@ fn map_ticket_tool_error(error: ToolError) -> WipOperationError {
WipOperationError::OutcomeUnknown(message) WipOperationError::OutcomeUnknown(message)
} }
} }
ToolError::Internal(message) => WipOperationError::OutcomeUnknown(message),
} }
} }
@@ -1753,6 +1754,7 @@ fn add_ticket_paths(response: &mut Value, collection_route: &str) {
fn ticket_reference(response: &Value) -> Option<&str> { fn ticket_reference(response: &Value) -> Option<&str> {
response response
.get("ticket") .get("ticket")
.or_else(|| response.get("ticket_id"))
.or_else(|| response.get("id")) .or_else(|| response.get("id"))
.or_else(|| response.get("meta").and_then(|meta| meta.get("id"))) .or_else(|| response.get("meta").and_then(|meta| meta.get("id")))
.and_then(Value::as_str) .and_then(Value::as_str)
@@ -1774,35 +1776,49 @@ fn record_ticket_observation(
response: &Value, response: &Value,
mutation: bool, mutation: bool,
) { ) {
let canonical = ticket_reference(response) let observed_canonical = ticket_reference(response).map(ToOwned::to_owned);
.map(ToOwned::to_owned)
.or_else(|| {
bound_reference
.filter(|reference| is_canonical_ticket_resource_key(reference))
.map(ToOwned::to_owned)
});
let mut state = revisions.lock().unwrap_or_else(|error| error.into_inner()); let mut state = revisions.lock().unwrap_or_else(|error| error.into_inner());
let canonical = canonical.or_else(|| { let existing_bound_group = bound_reference.and_then(|reference| {
bound_reference.and_then(|reference| { state
state .aliases_by_canonical
.aliases_by_canonical .iter()
.iter() .find(|(_, aliases)| aliases.contains(reference))
.find(|(_, aliases)| aliases.contains(reference)) .map(|(canonical, _)| canonical.clone())
.map(|(canonical, _)| canonical.clone())
})
}); });
let Some(canonical) = canonical else { let existing_canonical_group = observed_canonical.as_deref().and_then(|reference| {
state
.aliases_by_canonical
.iter()
.find(|(_, aliases)| aliases.contains(reference))
.map(|(canonical, _)| canonical.clone())
});
let Some(group_key) = observed_canonical
.clone()
.or(existing_canonical_group.clone())
.or(existing_bound_group.clone())
.or_else(|| bound_reference.map(ToOwned::to_owned))
else {
return; return;
}; };
let aliases = state
.aliases_by_canonical let mut aliases = BTreeSet::from([group_key.clone()]);
.entry(canonical.clone()) for prior_group in [existing_bound_group, existing_canonical_group]
.or_default(); .into_iter()
aliases.insert(canonical.clone()); .flatten()
.chain(std::iter::once(group_key.clone()))
.collect::<BTreeSet<_>>()
{
if let Some(existing) = state.aliases_by_canonical.remove(&prior_group) {
aliases.extend(existing);
}
}
if let Some(canonical) = observed_canonical {
aliases.insert(canonical);
}
if let Some(reference) = bound_reference { if let Some(reference) = bound_reference {
aliases.insert(reference.to_string()); aliases.insert(reference.to_string());
} }
let aliases = aliases.clone();
let revision = if mutation { let revision = if mutation {
state.mutation_sequence = state.mutation_sequence.saturating_add(1); state.mutation_sequence = state.mutation_sequence.saturating_add(1);
format!( format!(
@@ -1818,8 +1834,40 @@ fn record_ticket_observation(
.find_map(|alias| state.revisions.get(alias).cloned()) .find_map(|alias| state.revisions.get(alias).cloned())
.unwrap_or_else(|| "observed".into()) .unwrap_or_else(|| "observed".into())
}; };
for alias in aliases { for alias in &aliases {
state.revisions.insert(alias, revision.clone()); state.revisions.insert(alias.clone(), revision.clone());
}
state.aliases_by_canonical.insert(group_key, aliases);
}
fn record_affected_ticket_mutations(
revisions: &TicketRevisions,
bound_reference: &str,
response: &Value,
) {
let mut subject_references = BTreeSet::from([bound_reference.to_string()]);
if let Some(reference) = ticket_reference(response) {
subject_references.insert(reference.to_string());
}
let mut affected = BTreeSet::new();
if let Some(target) = response
.get("target")
.and_then(Value::as_str)
.filter(|reference| is_ticket_route_reference(reference))
{
affected.insert(target.to_string());
}
if let Some(queued) = response.get("queued_tickets").and_then(Value::as_array) {
affected.extend(
queued
.iter()
.filter_map(Value::as_str)
.filter(|reference| is_ticket_route_reference(reference))
.map(ToOwned::to_owned),
);
}
for reference in affected.difference(&subject_references) {
record_ticket_observation(revisions, Some(reference), &Value::Null, true);
} }
} }
@@ -2094,7 +2142,17 @@ mod tests {
]); ]);
let input = native_ticket_input(&arguments, Some("T-42"), Some("ticket")) let input = native_ticket_input(&arguments, Some("T-42"), Some("ticket"))
.unwrap_or_else(|_| panic!("route-bound Ticket input should project")); .unwrap_or_else(|_| panic!("route-bound Ticket input should project"));
assert_eq!(input, json!({"ticket": "T-42", "body": "comment"})); assert_eq!(
input,
json!({"ticket": "T-42", "body": "comment", "reason": null})
);
let omitted = native_ticket_input(
&BTreeMap::from([("body".into(), WipValue::String("comment".into()))]),
Some("T-42"),
Some("ticket"),
)
.unwrap_or_else(|_| panic!("omitted optional Ticket input should project"));
assert_eq!(omitted, json!({"ticket": "T-42", "body": "comment"}));
let replaced = native_ticket_input( let replaced = native_ticket_input(
&BTreeMap::from([("ticket".into(), WipValue::String("T-99".into()))]), &BTreeMap::from([("ticket".into(), WipValue::String("T-99".into()))]),
Some("T-42"), Some("T-42"),
@@ -2128,18 +2186,31 @@ mod tests {
#[test] #[test]
fn ticket_alias_validators_refresh_together_after_mutation() { fn ticket_alias_validators_refresh_together_after_mutation() {
let revisions = Arc::new(Mutex::new(TicketRevisionState::default())); let revisions = Arc::new(Mutex::new(TicketRevisionState::default()));
record_ticket_observation(
&revisions,
Some("00001TICKET"),
&json!({"ticket": "T-42", "updated_at": "rev-1"}),
false,
);
let resolver = TicketItemResolver { let resolver = TicketItemResolver {
tools: HashMap::new(), tools: HashMap::new(),
permissions: None, permissions: None,
collection_route: "/features/ticket/tickets".into(), collection_route: "/features/ticket/tickets".into(),
revisions: Arc::clone(&revisions), revisions: Arc::clone(&revisions),
}; };
let unobserved_internal = resolver.resolve("00001TICKET").unwrap().object.validator;
record_ticket_observation(
&revisions,
Some("00001TICKET"),
&json!({"ticket": "00001TICKET", "ok": true}),
true,
);
assert_ne!(
unobserved_internal,
resolver.resolve("00001TICKET").unwrap().object.validator,
"a first mutation through an internal-id route must stale that route"
);
record_ticket_observation(
&revisions,
Some("00001TICKET"),
&json!({"ticket": "T-42", "item_revision": "rev-1"}),
false,
);
let canonical_v1 = resolver.resolve("T-42").unwrap().object.validator; let canonical_v1 = resolver.resolve("T-42").unwrap().object.validator;
let internal_v1 = resolver.resolve("00001TICKET").unwrap().object.validator; let internal_v1 = resolver.resolve("00001TICKET").unwrap().object.validator;
record_ticket_observation( record_ticket_observation(
@@ -2158,6 +2229,44 @@ mod tests {
); );
} }
#[test]
fn related_ticket_mutations_invalidate_target_and_queue_observations() {
let revisions = Arc::new(Mutex::new(TicketRevisionState::default()));
for ticket in ["T-42", "T-43", "T-44"] {
record_ticket_observation(
&revisions,
Some(ticket),
&json!({"ticket": ticket, "item_revision": "rev-1"}),
false,
);
}
let resolver = TicketItemResolver {
tools: HashMap::new(),
permissions: None,
collection_route: "/features/ticket/tickets".into(),
revisions: Arc::clone(&revisions),
};
let target_v1 = resolver.resolve("T-43").unwrap().object.validator;
let queued_v1 = resolver.resolve("T-44").unwrap().object.validator;
record_affected_ticket_mutations(
&revisions,
"T-42",
&json!({
"ticket": "T-42",
"target": "T-43",
"queued_tickets": ["T-42", "T-44"]
}),
);
assert_ne!(
target_v1,
resolver.resolve("T-43").unwrap().object.validator
);
assert_ne!(
queued_v1,
resolver.resolve("T-44").unwrap().object.validator
);
}
#[test] #[test]
fn native_ticket_errors_keep_deterministic_protocol_outcomes() { fn native_ticket_errors_keep_deterministic_protocol_outcomes() {
for (status, code) in [ for (status, code) in [
@@ -2177,6 +2286,12 @@ mod tests {
map_ticket_tool_error(ToolError::ExecutionFailed("transport disconnected".into())), map_ticket_tool_error(ToolError::ExecutionFailed("transport disconnected".into())),
WipOperationError::OutcomeUnknown(_) WipOperationError::OutcomeUnknown(_)
)); ));
assert!(matches!(
map_ticket_tool_error(ToolError::Internal(
"TicketClose outcome unknown after mutation: HTTP status 404".into()
)),
WipOperationError::OutcomeUnknown(_)
));
} }
#[test] #[test]
+3 -3
View File
@@ -60,9 +60,9 @@ No native operation is inferred for a Tool that the Feature did not register. En
`query` uses the authoritative Workspace bounded query and cursor contract. Results include canonical `T-*` references and native item paths without embedding unbounded thread or evidence history. `read` uses the authoritative bounded detail projection for thread events, relations, Objective references, assignments, Merge Requests and evidence. Objective and Merge Request values remain references/data; this projection neither assumes their native routes nor grants their authority. `query` uses the authoritative Workspace bounded query and cursor contract. Results include canonical `T-*` references and native item paths without embedding unbounded thread or evidence history. `read` uses the authoritative bounded detail projection for thread events, relations, Objective references, assignments, Merge Requests and evidence. Objective and Merge Request values remain references/data; this projection neither assumes their native routes nor grants their authority.
Backend 401/403 failures map to permission denial, 404 to missing objects, and deterministic 400/409/422 validation or workflow failures to invalid arguments. Cancellation and interruption remain distinct. Transport, server, or malformed post-dispatch results retain unknown outcome, and mutating operations are not retried automatically. Backend 401/403 failures map to permission denial, 404 to missing objects, and deterministic 400/409/422 validation or workflow failures to invalid arguments when the typed operation fails before any successful mutation. Cancellation and interruption remain distinct. Post-mutation canonical-reference lookups are explicitly marked as unknown-outcome stages, so even a later deterministic HTTP status cannot misreport an already-committed write as a rejection. Transport, server, malformed response, and other post-dispatch failures retain unknown outcome, and mutating operations are not retried automatically.
Successful reads observe the authoritative update marker. Successful mutations advance a local observation generation and invalidate every known canonical/internal alias for the same Ticket. A stale validator is rejected by the common WIP Host before another Backend dispatch; the caller must rediscover/reinspect and explicitly decide whether to call again. Successful reads observe the authoritative item revision. Successful mutations advance a local observation generation even when the first call uses an internal ID whose canonical alias has not yet been learned. Once a read or mutation reveals the canonical key, the canonical/internal alias groups are merged and invalidated together. Relation mutations also invalidate the reported target Ticket, while queue results invalidate every reported queued Ticket, because those objects' incoming relations or workflow states changed. A stale validator is rejected by the common WIP Host before another Backend dispatch; the caller must rediscover/reinspect and explicitly decide whether to call again.
## Preserved Ticket contracts ## Preserved Ticket contracts
@@ -77,4 +77,4 @@ The projection invokes the existing Tool and scoped Backend routes, so the follo
- exact forward relation ownership, inverse views and relation target validation; - exact forward relation ownership, inverse views and relation target validation;
- orchestration-plan validation and bounded query behavior. - orchestration-plan validation and bounded query behavior.
The ordinary JSON Schema is retained as a second validation boundary behind the WIP descriptor. Optional fields omitted by the caller stay omitted when reconstructing permission input; defaults are applied only by the same ordinary typed Tool that applies them in Tool mode. The ordinary JSON Schema is retained as a second validation boundary behind the WIP descriptor. Optional fields omitted by the caller stay omitted, while an explicitly supplied `null` stays present as JSON `null`, so permission matching sees the same input shape as normal Tool mode. Defaults are applied only by the same ordinary typed Tool that applies them in Tool mode.