From 3d662bc40edfa64b815f372022020f2823399958 Mon Sep 17 00:00:00 2001 From: Hare Date: Sun, 7 Jun 2026 12:56:13 +0900 Subject: [PATCH] pod: split ticket feature access levels --- crates/pod/src/feature/builtin.rs | 4 +- crates/pod/src/feature/builtin/ticket.rs | 150 ++++++++++++++++++++++- crates/ticket/src/tool.rs | 60 ++++++++- 3 files changed, 207 insertions(+), 7 deletions(-) diff --git a/crates/pod/src/feature/builtin.rs b/crates/pod/src/feature/builtin.rs index 35997006..ae0054b1 100644 --- a/crates/pod/src/feature/builtin.rs +++ b/crates/pod/src/feature/builtin.rs @@ -8,4 +8,6 @@ pub mod task; pub mod ticket; pub use task::{TaskFeature, task_tools_feature}; -pub use ticket::{TicketFeature, ticket_tools_feature}; +pub use ticket::{ + TicketFeature, TicketFeatureAccess, ticket_tools_feature, ticket_tools_feature_with_access, +}; diff --git a/crates/pod/src/feature/builtin/ticket.rs b/crates/pod/src/feature/builtin/ticket.rs index 3658dfc3..7c09f758 100644 --- a/crates/pod/src/feature/builtin/ticket.rs +++ b/crates/pod/src/feature/builtin/ticket.rs @@ -9,8 +9,7 @@ use std::path::{Path, PathBuf}; use ticket::{ LocalTicketBackend, config::{DEFAULT_TICKET_BACKEND_RELATIVE_PATH, TicketConfig}, - tool::TICKET_TOOL_NAMES, - tool::ticket_tools, + tool::{TICKET_READ_ONLY_TOOL_NAMES, TICKET_TOOL_NAMES, ticket_tools}, }; use crate::feature::{ @@ -24,27 +23,58 @@ const FEATURE_DESCRIPTION: &str = "Typed local Ticket work-item operations over The tools operate through the ticket crate backend and do not grant generic filesystem write scope."; const AUTHORITY_REASON: &str = "Use a configured local Ticket backend root for typed work-item operations without generic filesystem write authority."; +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum TicketFeatureAccess { + /// Status/diagnostic access for views such as Companion that must not mutate Tickets. + ReadOnly, + /// Full Ticket lifecycle access, including the read-only tools and all mutating Ticket tools. + Lifecycle, +} + +impl TicketFeatureAccess { + pub fn tool_names(self) -> &'static [&'static str] { + match self { + Self::ReadOnly => &TICKET_READ_ONLY_TOOL_NAMES, + Self::Lifecycle => &TICKET_TOOL_NAMES, + } + } +} + #[derive(Clone, Debug)] pub struct TicketFeature { backend_root: PathBuf, config_error: Option, + access: TicketFeatureAccess, } impl TicketFeature { pub fn new(backend_root: impl Into) -> Self { + Self::new_with_access(backend_root, TicketFeatureAccess::Lifecycle) + } + + pub fn new_with_access(backend_root: impl Into, access: TicketFeatureAccess) -> Self { Self { backend_root: backend_root.into(), config_error: None, + access, } } pub fn for_workspace(workspace: impl AsRef) -> Self { + Self::for_workspace_with_access(workspace, TicketFeatureAccess::Lifecycle) + } + + pub fn for_workspace_with_access( + workspace: impl AsRef, + access: TicketFeatureAccess, + ) -> Self { let workspace = workspace.as_ref(); match TicketConfig::load_workspace(workspace) { - Ok(config) => Self::new(config.backend_root().to_path_buf()), + Ok(config) => Self::new_with_access(config.backend_root().to_path_buf(), access), Err(error) => Self { backend_root: workspace.join(DEFAULT_TICKET_BACKEND_RELATIVE_PATH), config_error: Some(error.to_string()), + access, }, } } @@ -53,6 +83,10 @@ impl TicketFeature { &self.backend_root } + pub fn access(&self) -> TicketFeatureAccess { + self.access + } + fn authority(&self) -> HostAuthority { HostAuthority::TicketBackend { root: self.backend_root.display().to_string(), @@ -87,8 +121,8 @@ impl FeatureModule for TicketFeature { self.authority(), AUTHORITY_REASON, )); - for name in TICKET_TOOL_NAMES { - descriptor = descriptor.with_tool(ToolDeclaration::new(name, tool_description(name))); + for name in self.access.tool_names() { + descriptor = descriptor.with_tool(ToolDeclaration::new(*name, tool_description(name))); } descriptor } @@ -116,10 +150,14 @@ impl FeatureModule for TicketFeature { }; let authority = self.authority(); let backend = LocalTicketBackend::new(usable_root); + let allowed_tool_names = self.access.tool_names(); let mut tools = context.tools(); for definition in ticket_tools(backend) { let (meta, _) = definition(); let name = meta.name.clone(); + if !allowed_tool_names.contains(&name.as_str()) { + continue; + } tools.register( ToolContribution::new(name, definition) .with_required_host_authorities(vec![authority.clone()]), @@ -140,6 +178,12 @@ fn tool_description(name: &str) -> &'static str { "Append a comment/plan/decision/implementation_report event to a Ticket." } "TicketReview" => "Append an approve/request_changes review event to a Ticket.", + "TicketIntakeReady" => { + "Mark an intake Ticket ready and append the typed intake summary/state transition events." + } + "TicketWorkflowState" => { + "Transition Ticket workflow_state; queued -> inprogress is the accepted implementation start, so implementation side effects should happen only after that transition is accepted and recorded." + } "TicketStatus" => "Move a Ticket between open and pending; use TicketClose for closed.", "TicketClose" => "Close a Ticket with a resolution through the typed local Ticket backend.", "TicketDoctor" => "Run typed local Ticket backend consistency checks.", @@ -151,6 +195,13 @@ pub fn ticket_tools_feature(workspace: impl AsRef) -> TicketFeature { TicketFeature::for_workspace(workspace) } +pub fn ticket_tools_feature_with_access( + workspace: impl AsRef, + access: TicketFeatureAccess, +) -> TicketFeature { + TicketFeature::for_workspace_with_access(workspace, access) +} + #[cfg(test)] mod tests { use super::*; @@ -193,6 +244,95 @@ mod tests { )); } + #[test] + fn read_only_descriptor_declares_only_status_tools() { + let temp = TempDir::new().unwrap(); + let feature = ticket_tools_feature_with_access(temp.path(), TicketFeatureAccess::ReadOnly); + let descriptor = feature.descriptor(); + assert_eq!(feature.access(), TicketFeatureAccess::ReadOnly); + assert_eq!(descriptor.tools.len(), TICKET_READ_ONLY_TOOL_NAMES.len()); + assert_eq!( + descriptor + .tools + .iter() + .map(|tool| tool.name.as_str()) + .collect::>(), + TICKET_READ_ONLY_TOOL_NAMES + ); + assert_eq!(descriptor.requested_host_authorities.len(), 1); + } + + #[test] + fn read_only_installation_does_not_expose_mutating_tools() { + let temp = TempDir::new().unwrap(); + make_ticket_root(&temp.path().join(DEFAULT_TICKET_BACKEND_RELATIVE_PATH)); + let mut pending_tools = Vec::new(); + let mut hooks = HookRegistryBuilder::default(); + let report = FeatureRegistryBuilder::new() + .with_module(ticket_tools_feature_with_access( + temp.path(), + TicketFeatureAccess::ReadOnly, + )) + .install_into_pending(&mut pending_tools, &mut hooks); + + assert_eq!(pending_tools.len(), TICKET_READ_ONLY_TOOL_NAMES.len()); + assert_eq!( + report.reports[0].installed_tools, + TICKET_READ_ONLY_TOOL_NAMES + ); + let pending_names = pending_tools + .iter() + .map(|definition| definition().0.name) + .collect::>(); + assert_eq!(pending_names, TICKET_READ_ONLY_TOOL_NAMES); + for name in ticket::tool::TICKET_MUTATING_TOOL_NAMES { + assert!( + !report.reports[0] + .installed_tools + .iter() + .any(|tool| tool == name) + ); + assert!(!pending_names.iter().any(|tool| tool == name)); + } + } + + #[test] + fn lifecycle_installation_exposes_lifecycle_tools() { + let temp = TempDir::new().unwrap(); + make_ticket_root(&temp.path().join(DEFAULT_TICKET_BACKEND_RELATIVE_PATH)); + let mut pending_tools = Vec::new(); + let mut hooks = HookRegistryBuilder::default(); + let report = FeatureRegistryBuilder::new() + .with_module(ticket_tools_feature_with_access( + temp.path(), + TicketFeatureAccess::Lifecycle, + )) + .install_into_pending(&mut pending_tools, &mut hooks); + + assert_eq!(pending_tools.len(), TICKET_TOOL_NAMES.len()); + assert_eq!(report.reports[0].installed_tools, TICKET_TOOL_NAMES); + for name in ticket::tool::TICKET_MUTATING_TOOL_NAMES { + assert!( + report.reports[0] + .installed_tools + .iter() + .any(|tool| tool == name) + ); + } + assert!( + report.reports[0] + .installed_tools + .iter() + .any(|tool| tool == "TicketIntakeReady") + ); + assert!( + report.reports[0] + .installed_tools + .iter() + .any(|tool| tool == "TicketWorkflowState") + ); + } + #[test] fn installs_ticket_tools_when_default_root_is_usable() { let temp = TempDir::new().unwrap(); diff --git a/crates/ticket/src/tool.rs b/crates/ticket/src/tool.rs index ae43f264..4f170fd6 100644 --- a/crates/ticket/src/tool.rs +++ b/crates/ticket/src/tool.rs @@ -42,6 +42,18 @@ pub const TICKET_TOOL_NAMES: [&str; 10] = [ "TicketDoctor", ]; +pub const TICKET_READ_ONLY_TOOL_NAMES: [&str; 3] = ["TicketList", "TicketShow", "TicketDoctor"]; + +pub const TICKET_MUTATING_TOOL_NAMES: [&str; 7] = [ + "TicketCreate", + "TicketComment", + "TicketReview", + "TicketIntakeReady", + "TicketWorkflowState", + "TicketStatus", + "TicketClose", +]; + const CREATE_DESCRIPTION: &str = "Create a Ticket through the configured typed Ticket backend. \ Inputs mirror the Ticket `item.md` fields; `title` is required, `body` is Markdown, and the \ backend assigns the id and writes the local Ticket file layout under the configured backend root."; @@ -61,7 +73,9 @@ Ticket backend. The tool appends a bounded `intake_summary`, appends a typed `st for `workflow_state`, and transitions workflow_state to `ready`."; const WORKFLOW_STATE_DESCRIPTION: &str = "Transition Ticket `workflow_state` through the typed \ Ticket backend with a bounded `state_changed` event. This does not move local open/pending/closed \ -status; use `TicketStatus` or `TicketClose` for local status changes."; +status; use `TicketStatus` or `TicketClose` for local status changes. Treat `queued -> inprogress` \ +as the implementation acceptance step: implementation side effects should happen only after that \ +transition is accepted and recorded."; const STATUS_DESCRIPTION: &str = "Move a Ticket between non-closed local statuses through the typed \ Ticket backend. Use `TicketClose` for closing because closed Tickets require a resolution accepted \ by `yoi ticket doctor`."; @@ -989,6 +1003,50 @@ mod tests { .expect("tool exists") } + #[test] + fn ticket_tool_name_partitions_are_explicit() { + assert_eq!( + TICKET_READ_ONLY_TOOL_NAMES, + ["TicketList", "TicketShow", "TicketDoctor"] + ); + assert_eq!( + TICKET_MUTATING_TOOL_NAMES, + [ + "TicketCreate", + "TicketComment", + "TicketReview", + "TicketIntakeReady", + "TicketWorkflowState", + "TicketStatus", + "TicketClose" + ] + ); + for name in TICKET_READ_ONLY_TOOL_NAMES { + assert!(TICKET_TOOL_NAMES.contains(&name)); + assert!(!TICKET_MUTATING_TOOL_NAMES.contains(&name)); + } + for name in TICKET_MUTATING_TOOL_NAMES { + assert!(TICKET_TOOL_NAMES.contains(&name)); + assert!(!TICKET_READ_ONLY_TOOL_NAMES.contains(&name)); + } + assert_eq!( + TICKET_READ_ONLY_TOOL_NAMES.len() + TICKET_MUTATING_TOOL_NAMES.len(), + TICKET_TOOL_NAMES.len() + ); + } + + #[test] + fn workflow_state_tool_description_explains_queued_acceptance() { + let temp = TempDir::new().unwrap(); + let definition = ticket_tools(backend(&temp)) + .into_iter() + .find(|definition| definition().0.name == "TicketWorkflowState") + .expect("workflow state tool exists"); + let (meta, _) = definition(); + assert!(meta.description.contains("queued -> inprogress")); + assert!(meta.description.contains("implementation side effects")); + } + #[tokio::test] async fn ticket_tools_create_list_show_and_doctor() { let temp = TempDir::new().unwrap();