fix: separate Ticket keys from assignment identity

2026-08-29 22:52:03 +09:00
parent 40ac83e632
commit acb3c6d68b
6 changed files with 200 additions and 55 deletions
@@ -209,9 +209,7 @@ impl WorkspaceHttpObjectiveBackend {
            .and_then(serde_json::Value::as_str)
            .filter(|key| is_canonical_resource_key(key, "T-"))
            .map(ToOwned::to_owned)
-            .ok_or_else(|| {
-                ToolError::ExecutionFailed("required T- human key is unavailable".to_string())
-            })
+            .ok_or_else(|| ToolError::ExecutionFailed("required T- key is unavailable".to_string()))
    }

    fn objective_url(&self, id: &str) -> String {
@@ -295,7 +293,7 @@ fn is_canonical_resource_key(resource_key: &str, prefix: &str) -> bool {
 fn objective_output(summary: String, response: ObjectiveDetail) -> Result<ToolOutput, ToolError> {
    if !is_canonical_resource_key(&response.resource_key, "O-") {
        return Err(ToolError::ExecutionFailed(
-            "required O- human key is unavailable".to_string(),
+            "required O- key is unavailable".to_string(),
        ));
    }
    let projected = serde_json::json!({
@@ -712,7 +710,7 @@ mod tests {
    }

    #[tokio::test(flavor = "multi_thread")]
-    async fn objective_show_summary_uses_projected_human_key() {
+    async fn objective_show_summary_uses_projected_key() {
        let listener = TcpListener::bind("127.0.0.1:0").unwrap();
        let base_url = format!("http://{}", listener.local_addr().unwrap());
        let server = thread::spawn(move || {
@@ -764,7 +762,7 @@ mod tests {
    }

    #[tokio::test(flavor = "multi_thread")]
-    async fn objective_link_summaries_resolve_internal_ticket_ids_to_human_keys() {
+    async fn objective_link_summaries_resolve_internal_ticket_ids_to_keys() {
        let listener = TcpListener::bind("127.0.0.1:0").unwrap();
        let base_url = format!("http://{}", listener.local_addr().unwrap());
        let server = thread::spawn(move || {
@@ -833,7 +831,7 @@ mod tests {
    }

    #[test]
-    fn objective_output_rejects_noncanonical_human_keys() {
+    fn objective_output_rejects_noncanonical_keys() {
        let response = ObjectiveDetail {
            resource_key: "O-internal".to_string(),
            title: "Objective".to_string(),
@@ -228,7 +228,7 @@ pub(super) fn project_ticket_query(value: Value) -> Result<ModelTicketQueryRespo
 fn project_ticket_query_item(value: &Value) -> Result<ModelTicketQueryItem, String> {
    let item = object(value, "Ticket query item")?;
    Ok(ModelTicketQueryItem {
-        ticket: human_ref(item, "resource_key", "T-")?,
+        ticket: resource_ref(item, "resource_key", "T-")?,
        title: string_field(item, "title")?,
        state: string_field(item, "state")?,
        readiness: optional_string(item, "readiness")?,
@@ -245,7 +245,7 @@ fn project_ticket_query_item(value: &Value) -> Result<ModelTicketQueryItem, Stri
            .transpose()?,
        linked_objectives: string_array(item, "linked_objective_keys")?
            .into_iter()
-            .map(|key| validate_human_ref(key, "O-"))
+            .map(|key| validate_resource_ref(key, "O-"))
            .collect::<Result<Vec<_>, _>>()?,
        relation_count: usize_field(item, "relation_count")?,
        blocker_count: usize_field(item, "blocker_count")?,
@@ -273,7 +273,7 @@ pub(super) fn project_ticket_detail(value: Value) -> Result<ModelTicketDetail, S
        .collect::<Result<Vec<_>, _>>()?;

    Ok(ModelTicketDetail {
-        ticket: human_ref(root, "resource_key", "T-")?,
+        ticket: resource_ref(root, "resource_key", "T-")?,
        title: string_field(root, "title")?,
        body: string_field(root, "body")?,
        state: string_field(root, "state")?,
@@ -332,10 +332,10 @@ fn project_objective_query_item(value: &Value) -> Result<ModelObjectiveQueryItem
    let item = object(value, "Objective query item")?;
    let linked_tickets = string_array(item, "linked_ticket_keys")?
        .into_iter()
-        .map(|key| validate_human_ref(key, "T-"))
+        .map(|key| validate_resource_ref(key, "T-"))
        .collect::<Result<Vec<_>, _>>()?;
    Ok(ModelObjectiveQueryItem {
-        objective: human_ref(item, "resource_key", "O-")?,
+        objective: resource_ref(item, "resource_key", "O-")?,
        title: string_field(item, "title")?,
        summary: optional_string(item, "snippet")?,
        state: string_field(item, "state")?,
@@ -349,7 +349,7 @@ fn project_objective_query_item(value: &Value) -> Result<ModelObjectiveQueryItem
 pub(super) fn project_objective_detail(value: Value) -> Result<ModelObjectiveDetail, String> {
    let root = object(&value, "Objective detail response")?;
    Ok(ModelObjectiveDetail {
-        objective: human_ref(root, "resource_key", "O-")?,
+        objective: resource_ref(root, "resource_key", "O-")?,
        title: string_field(root, "title")?,
        body: string_field(root, "body")?,
        state: string_field(root, "state")?,
@@ -373,7 +373,7 @@ pub(super) fn project_objective_detail(value: Value) -> Result<ModelObjectiveDet
 fn project_worker(value: &Value) -> Result<ModelWorkerSummary, String> {
    let worker = object(value, "Worker summary")?;
    Ok(ModelWorkerSummary {
-        worker: human_ref(worker, "worker_resource_key", "W-")?,
+        worker: resource_ref(worker, "worker_resource_key", "W-")?,
    })
 }

@@ -439,7 +439,7 @@ fn project_relation(
        None => optional_string(relation, "at")?,
    };
    Ok(ModelRelation {
-        ticket: human_ref(relation, ticket_key, "T-")?,
+        ticket: resource_ref(relation, ticket_key, "T-")?,
        kind,
        note,
        created_at,
@@ -449,7 +449,7 @@ fn project_relation(
 fn project_blocker(value: &Value) -> Result<ModelBlocker, String> {
    let blocker = object(value, "Ticket blocker")?;
    Ok(ModelBlocker {
-        ticket: human_ref(blocker, "blocking_resource_key", "T-")?,
+        ticket: resource_ref(blocker, "blocking_resource_key", "T-")?,
        kind: string_field(blocker, "relation_kind")?,
        state: optional_string(blocker, "blocking_state")?,
        resolved: bool_field(blocker, "resolved")?,
@@ -466,7 +466,7 @@ fn project_notice(value: &Value) -> Result<ModelNotice, String> {
 fn project_objective_summary(value: &Value) -> Result<ModelObjectiveSummary, String> {
    let summary = object(value, "Objective summary")?;
    Ok(ModelObjectiveSummary {
-        objective: human_ref(summary, "resource_key", "O-")?,
+        objective: resource_ref(summary, "resource_key", "O-")?,
        title: string_field(summary, "title")?,
        state: string_field(summary, "state")?,
    })
@@ -475,7 +475,7 @@ fn project_objective_summary(value: &Value) -> Result<ModelObjectiveSummary, Str
 fn project_ticket_summary(value: &Value) -> Result<ModelTicketSummary, String> {
    let summary = object(value, "Ticket summary")?;
    Ok(ModelTicketSummary {
-        ticket: human_ref(summary, "resource_key", "T-")?,
+        ticket: resource_ref(summary, "resource_key", "T-")?,
        title: string_field(summary, "title")?,
        state: string_field(summary, "state")?,
    })
@@ -491,9 +491,7 @@ fn project_assignment(
    let principal = match kind.as_str() {
        "worker" => current_coder
            .map(|coder| coder.worker.clone())
-            .ok_or_else(|| {
-                "Worker assignment is missing a Workspace human key projection".to_string()
-            })?,
+            .ok_or_else(|| "Worker assignment is missing a Workspace key projection".to_string())?,
        "workspace_agent" => format!("workspace-agent:{}", string_field(principal, "agent_key")?),
        "user" => "user".to_string(),
        other => format!("source:{other}"),
@@ -645,23 +643,23 @@ fn string_array(object: &Map<String, Value>, key: &str) -> Result<Vec<String>, S
        .collect()
 }

-fn human_ref(object: &Map<String, Value>, key: &str, prefix: &str) -> Result<String, String> {
+fn resource_ref(object: &Map<String, Value>, key: &str, prefix: &str) -> Result<String, String> {
    let value = object
        .get(key)
        .and_then(Value::as_str)
        .map(ToOwned::to_owned)
-        .ok_or_else(|| format!("required {prefix} human key is unavailable"))?;
-    validate_human_ref(value, prefix)
+        .ok_or_else(|| format!("required {prefix} key is unavailable"))?;
+    validate_resource_ref(value, prefix)
 }

-fn validate_human_ref(value: String, prefix: &str) -> Result<String, String> {
+fn validate_resource_ref(value: String, prefix: &str) -> Result<String, String> {
    let valid = value.strip_prefix(prefix).is_some_and(|sequence| {
        !sequence.is_empty() && sequence.bytes().all(|byte| byte.is_ascii_digit())
    });
    if valid {
        Ok(value)
    } else {
-        Err(format!("required {prefix} human key is unavailable"))
+        Err(format!("required {prefix} key is unavailable"))
    }
 }

@@ -671,7 +669,7 @@ mod tests {
    use serde_json::json;

    #[test]
-    fn objective_projection_exposes_only_human_resource_references() {
+    fn objective_projection_exposes_only_resource_references() {
        let projected = project_objective_detail(json!({
            "id": "00001M10HW6BV",
            "resource_key": "O-543",
@@ -779,9 +777,9 @@ mod tests {
    }

    #[test]
-    fn human_resource_projection_rejects_noncanonical_keys() {
+    fn resource_projection_rejects_noncanonical_keys() {
        for (key, prefix) in [("T-key", "T-"), ("O-", "O-"), ("W-1x", "W-")] {
-            assert!(validate_human_ref(key.to_string(), prefix).is_err());
+            assert!(validate_resource_ref(key.to_string(), prefix).is_err());
        }
    }

@@ -306,19 +306,32 @@ struct BackendTicketService {
    backend: TicketToolBackend,
 }

+struct WorkspaceTicketService {
+    backend: WorkspaceHttpTicketBackend,
+}
+
+fn ticket_handoff_from_record(ticket: Ticket) -> Result<TicketHandoff, TicketError> {
+    let resource_key = ticket
+        .meta
+        .resource_key
+        .filter(|key| is_canonical_ticket_resource_key(key))
+        .ok_or_else(|| TicketError::Conflict("ticket resource key is unavailable".into()))?;
+    Ok(TicketHandoff {
+        id: ticket.meta.id,
+        resource_key,
+        workflow_state: ticket.meta.workflow_state,
+    })
+}
+
 impl TicketService for BackendTicketService {
    fn ticket_handoff(&self, ticket_ref: &str) -> Result<TicketHandoff, TicketError> {
-        let ticket = self.backend.show(ticket_ref.into())?;
-        let resource_key = ticket
-            .meta
-            .resource_key
-            .filter(|key| is_canonical_ticket_resource_key(key))
-            .ok_or_else(|| TicketError::Conflict("ticket resource key is unavailable".into()))?;
-        Ok(TicketHandoff {
-            id: ticket.meta.id,
-            resource_key,
-            workflow_state: ticket.meta.workflow_state,
-        })
+        ticket_handoff_from_record(self.backend.show(ticket_ref.into())?)
+    }
+}
+
+impl TicketService for WorkspaceTicketService {
+    fn ticket_handoff(&self, ticket_ref: &str) -> Result<TicketHandoff, TicketError> {
+        ticket_handoff_from_record(self.backend.show_unprojected(ticket_ref)?)
    }
 }

@@ -640,9 +653,14 @@ impl FeatureModule for TicketFeature {
        let Some(backend) = self.tool_backend(context) else {
            return Ok(());
        };
-        let ticket_service: Arc<dyn TicketService> = Arc::new(BackendTicketService {
-            backend: backend.clone(),
-        });
+        let ticket_service: Arc<dyn TicketService> = match &self.backend {
+            TicketFeatureBackend::WorkspaceClient(client) => Arc::new(WorkspaceTicketService {
+                backend: WorkspaceHttpTicketBackend::new(client.clone()),
+            }),
+            TicketFeatureBackend::Local { .. } => Arc::new(BackendTicketService {
+                backend: backend.clone(),
+            }),
+        };
        context.services().provide(
            ServiceDeclaration::new(
                ServiceId::builtin(TICKET_SERVICE_ID),
@@ -714,6 +732,26 @@ impl WorkspaceHttpTicketBackend {
        Self::invoke_client(client, workspace_id, operation)
    }

+    fn show_unprojected(&self, ticket_ref: &str) -> TicketResult<Ticket> {
+        let client = self.client.clone();
+        let workspace_id = self.client.workspace_id().unwrap_or_default().to_string();
+        let ticket_path = Self::ticket_path(&TicketIdOrSlug::from(ticket_ref));
+        let request = move || {
+            Self::request_unprojected(
+                client,
+                WorkspaceRequestMethod::Get,
+                format!("/api/w/{workspace_id}/tickets/{ticket_path}/record"),
+                None,
+            )
+        };
+        if tokio::runtime::Handle::try_current().is_ok() {
+            return std::thread::spawn(request).join().map_err(|_| {
+                TicketError::Conflict("ticket REST request thread panicked".to_string())
+            })?;
+        }
+        request()
+    }
+
    fn ticket_path(id: &TicketIdOrSlug) -> String {
        let value = match id {
            TicketIdOrSlug::Id(value)
@@ -738,6 +776,29 @@ impl WorkspaceHttpTicketBackend {
        endpoint: String,
        body: Option<serde_json::Value>,
    ) -> TicketResult<T> {
+        let mut value = Self::request_value(client, method, endpoint, body)?;
+        Self::canonicalize_ticket_references(&mut value);
+        serde_json::from_value(value)
+            .map_err(|error| TicketError::Conflict(format!("decode ticket REST response: {error}")))
+    }
+
+    fn request_unprojected<T: serde::de::DeserializeOwned>(
+        client: Arc<dyn WorkspaceClient>,
+        method: WorkspaceRequestMethod,
+        endpoint: String,
+        body: Option<serde_json::Value>,
+    ) -> TicketResult<T> {
+        let value = Self::request_value(client, method, endpoint, body)?;
+        serde_json::from_value(value)
+            .map_err(|error| TicketError::Conflict(format!("decode ticket REST response: {error}")))
+    }
+
+    fn request_value(
+        client: Arc<dyn WorkspaceClient>,
+        method: WorkspaceRequestMethod,
+        endpoint: String,
+        body: Option<serde_json::Value>,
+    ) -> TicketResult<Value> {
        let request = match body {
            Some(body) => WorkspaceRequest::json(method, endpoint, body.to_string()),
            None if method == WorkspaceRequestMethod::Get => WorkspaceRequest::get(endpoint),
@@ -756,11 +817,7 @@ impl WorkspaceHttpTicketBackend {
                response.status
            )));
        }
-        let mut value: Value = serde_json::from_str(&response.body).map_err(|error| {
-            TicketError::Conflict(format!("decode ticket REST response: {error}"))
-        })?;
-        Self::canonicalize_ticket_references(&mut value);
-        serde_json::from_value(value)
+        serde_json::from_str(&response.body)
            .map_err(|error| TicketError::Conflict(format!("decode ticket REST response: {error}")))
    }

@@ -810,9 +867,7 @@ impl WorkspaceHttpTicketBackend {
            .and_then(Value::as_str)
            .filter(|key| is_canonical_ticket_resource_key(key))
            .map(ToOwned::to_owned)
-            .ok_or_else(|| {
-                TicketError::Conflict("required Ticket human key is unavailable".to_string())
-            })
+            .ok_or_else(|| TicketError::Conflict("required Ticket key is unavailable".to_string()))
    }

    fn request_unit(
@@ -889,7 +944,7 @@ impl WorkspaceHttpTicketBackend {
                    .is_some_and(is_canonical_ticket_resource_key)
                {
                    return Err(TicketError::Conflict(
-                        "required Ticket human key is unavailable".to_string(),
+                        "required Ticket key is unavailable".to_string(),
                    ));
                }
                Ok(TicketBackendOperationResult::Ticket(ticket))
@@ -1861,7 +1916,7 @@ provider = "github"
    }

    #[test]
-    fn workspace_http_backend_records_relation_with_authoritative_human_keys() {
+    fn workspace_http_backend_records_relation_with_authoritative_keys() {
        let listener = TcpListener::bind("127.0.0.1:0").unwrap();
        let addr = listener.local_addr().unwrap();
        let server = thread::spawn(move || {
@@ -2000,6 +2055,46 @@ provider = "github"
        assert_eq!(removed.target, "T-2");
    }

+    #[test]
+    fn workspace_ticket_service_preserves_internal_identity_for_handoff() {
+        let temp = TempDir::new().unwrap();
+        let local = LocalTicketBackend::new(temp.path().join("tickets"));
+        let created = local.create(NewTicket::new("Ticket handoff")).unwrap();
+        let mut ticket = local.show(TicketIdOrSlug::Id(created.id.clone())).unwrap();
+        ticket.meta.resource_key = Some("T-548".to_string());
+        ticket.meta.workflow_state = TicketWorkflowState::Queued;
+
+        let listener = TcpListener::bind("127.0.0.1:0").unwrap();
+        let base_url = format!("http://{}", listener.local_addr().unwrap());
+        let response_body = serde_json::to_string(&ticket).unwrap();
+        let server = thread::spawn(move || {
+            let (mut stream, _) = listener.accept().unwrap();
+            let mut buffer = [0_u8; 8192];
+            let len = stream.read(&mut buffer).unwrap();
+            let request = String::from_utf8_lossy(&buffer[..len]);
+            assert!(request.starts_with("GET /api/w/workspace-a/tickets/T-548/record HTTP/1.1"));
+            write!(
+                stream,
+                "HTTP/1.1 200 OK\r\nContent-Type: application/json\r\nContent-Length: {}\r\n\r\n{}",
+                response_body.len(),
+                response_body
+            )
+            .unwrap();
+        });
+
+        let service = WorkspaceTicketService {
+            backend: WorkspaceHttpTicketBackend::new(Arc::new(
+                crate::worker::TestWorkspaceHttpClient::new("workspace-a", base_url),
+            )),
+        };
+        let handoff = service.ticket_handoff("T-548").unwrap();
+
+        server.join().unwrap();
+        assert_eq!(handoff.id, created.id);
+        assert_eq!(handoff.resource_key, "T-548");
+        assert_eq!(handoff.workflow_state, TicketWorkflowState::Queued);
+    }
+
    #[test]
    fn ticket_handoff_accepts_only_canonical_ticket_resource_keys() {
        assert!(is_canonical_ticket_resource_key("T-482"));