pod: split ticket feature access levels
This commit is contained in:
parent
43f273de03
commit
3d662bc40e
|
|
@ -8,4 +8,6 @@ pub mod task;
|
||||||
pub mod ticket;
|
pub mod ticket;
|
||||||
|
|
||||||
pub use task::{TaskFeature, task_tools_feature};
|
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,
|
||||||
|
};
|
||||||
|
|
|
||||||
|
|
@ -9,8 +9,7 @@ use std::path::{Path, PathBuf};
|
||||||
use ticket::{
|
use ticket::{
|
||||||
LocalTicketBackend,
|
LocalTicketBackend,
|
||||||
config::{DEFAULT_TICKET_BACKEND_RELATIVE_PATH, TicketConfig},
|
config::{DEFAULT_TICKET_BACKEND_RELATIVE_PATH, TicketConfig},
|
||||||
tool::TICKET_TOOL_NAMES,
|
tool::{TICKET_READ_ONLY_TOOL_NAMES, TICKET_TOOL_NAMES, ticket_tools},
|
||||||
tool::ticket_tools,
|
|
||||||
};
|
};
|
||||||
|
|
||||||
use crate::feature::{
|
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.";
|
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.";
|
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)]
|
#[derive(Clone, Debug)]
|
||||||
pub struct TicketFeature {
|
pub struct TicketFeature {
|
||||||
backend_root: PathBuf,
|
backend_root: PathBuf,
|
||||||
config_error: Option<String>,
|
config_error: Option<String>,
|
||||||
|
access: TicketFeatureAccess,
|
||||||
}
|
}
|
||||||
|
|
||||||
impl TicketFeature {
|
impl TicketFeature {
|
||||||
pub fn new(backend_root: impl Into<PathBuf>) -> Self {
|
pub fn new(backend_root: impl Into<PathBuf>) -> Self {
|
||||||
|
Self::new_with_access(backend_root, TicketFeatureAccess::Lifecycle)
|
||||||
|
}
|
||||||
|
|
||||||
|
pub fn new_with_access(backend_root: impl Into<PathBuf>, access: TicketFeatureAccess) -> Self {
|
||||||
Self {
|
Self {
|
||||||
backend_root: backend_root.into(),
|
backend_root: backend_root.into(),
|
||||||
config_error: None,
|
config_error: None,
|
||||||
|
access,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
pub fn for_workspace(workspace: impl AsRef<Path>) -> Self {
|
pub fn for_workspace(workspace: impl AsRef<Path>) -> Self {
|
||||||
|
Self::for_workspace_with_access(workspace, TicketFeatureAccess::Lifecycle)
|
||||||
|
}
|
||||||
|
|
||||||
|
pub fn for_workspace_with_access(
|
||||||
|
workspace: impl AsRef<Path>,
|
||||||
|
access: TicketFeatureAccess,
|
||||||
|
) -> Self {
|
||||||
let workspace = workspace.as_ref();
|
let workspace = workspace.as_ref();
|
||||||
match TicketConfig::load_workspace(workspace) {
|
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 {
|
Err(error) => Self {
|
||||||
backend_root: workspace.join(DEFAULT_TICKET_BACKEND_RELATIVE_PATH),
|
backend_root: workspace.join(DEFAULT_TICKET_BACKEND_RELATIVE_PATH),
|
||||||
config_error: Some(error.to_string()),
|
config_error: Some(error.to_string()),
|
||||||
|
access,
|
||||||
},
|
},
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
@ -53,6 +83,10 @@ impl TicketFeature {
|
||||||
&self.backend_root
|
&self.backend_root
|
||||||
}
|
}
|
||||||
|
|
||||||
|
pub fn access(&self) -> TicketFeatureAccess {
|
||||||
|
self.access
|
||||||
|
}
|
||||||
|
|
||||||
fn authority(&self) -> HostAuthority {
|
fn authority(&self) -> HostAuthority {
|
||||||
HostAuthority::TicketBackend {
|
HostAuthority::TicketBackend {
|
||||||
root: self.backend_root.display().to_string(),
|
root: self.backend_root.display().to_string(),
|
||||||
|
|
@ -87,8 +121,8 @@ impl FeatureModule for TicketFeature {
|
||||||
self.authority(),
|
self.authority(),
|
||||||
AUTHORITY_REASON,
|
AUTHORITY_REASON,
|
||||||
));
|
));
|
||||||
for name in TICKET_TOOL_NAMES {
|
for name in self.access.tool_names() {
|
||||||
descriptor = descriptor.with_tool(ToolDeclaration::new(name, tool_description(name)));
|
descriptor = descriptor.with_tool(ToolDeclaration::new(*name, tool_description(name)));
|
||||||
}
|
}
|
||||||
descriptor
|
descriptor
|
||||||
}
|
}
|
||||||
|
|
@ -116,10 +150,14 @@ impl FeatureModule for TicketFeature {
|
||||||
};
|
};
|
||||||
let authority = self.authority();
|
let authority = self.authority();
|
||||||
let backend = LocalTicketBackend::new(usable_root);
|
let backend = LocalTicketBackend::new(usable_root);
|
||||||
|
let allowed_tool_names = self.access.tool_names();
|
||||||
let mut tools = context.tools();
|
let mut tools = context.tools();
|
||||||
for definition in ticket_tools(backend) {
|
for definition in ticket_tools(backend) {
|
||||||
let (meta, _) = definition();
|
let (meta, _) = definition();
|
||||||
let name = meta.name.clone();
|
let name = meta.name.clone();
|
||||||
|
if !allowed_tool_names.contains(&name.as_str()) {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
tools.register(
|
tools.register(
|
||||||
ToolContribution::new(name, definition)
|
ToolContribution::new(name, definition)
|
||||||
.with_required_host_authorities(vec![authority.clone()]),
|
.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."
|
"Append a comment/plan/decision/implementation_report event to a Ticket."
|
||||||
}
|
}
|
||||||
"TicketReview" => "Append an approve/request_changes review 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.",
|
"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.",
|
"TicketClose" => "Close a Ticket with a resolution through the typed local Ticket backend.",
|
||||||
"TicketDoctor" => "Run typed local Ticket backend consistency checks.",
|
"TicketDoctor" => "Run typed local Ticket backend consistency checks.",
|
||||||
|
|
@ -151,6 +195,13 @@ pub fn ticket_tools_feature(workspace: impl AsRef<Path>) -> TicketFeature {
|
||||||
TicketFeature::for_workspace(workspace)
|
TicketFeature::for_workspace(workspace)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
pub fn ticket_tools_feature_with_access(
|
||||||
|
workspace: impl AsRef<Path>,
|
||||||
|
access: TicketFeatureAccess,
|
||||||
|
) -> TicketFeature {
|
||||||
|
TicketFeature::for_workspace_with_access(workspace, access)
|
||||||
|
}
|
||||||
|
|
||||||
#[cfg(test)]
|
#[cfg(test)]
|
||||||
mod tests {
|
mod tests {
|
||||||
use super::*;
|
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::<Vec<_>>(),
|
||||||
|
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::<Vec<_>>();
|
||||||
|
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]
|
#[test]
|
||||||
fn installs_ticket_tools_when_default_root_is_usable() {
|
fn installs_ticket_tools_when_default_root_is_usable() {
|
||||||
let temp = TempDir::new().unwrap();
|
let temp = TempDir::new().unwrap();
|
||||||
|
|
|
||||||
|
|
@ -42,6 +42,18 @@ pub const TICKET_TOOL_NAMES: [&str; 10] = [
|
||||||
"TicketDoctor",
|
"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. \
|
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 \
|
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.";
|
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`.";
|
for `workflow_state`, and transitions workflow_state to `ready`.";
|
||||||
const WORKFLOW_STATE_DESCRIPTION: &str = "Transition Ticket `workflow_state` through the typed \
|
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 \
|
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 \
|
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 \
|
Ticket backend. Use `TicketClose` for closing because closed Tickets require a resolution accepted \
|
||||||
by `yoi ticket doctor`.";
|
by `yoi ticket doctor`.";
|
||||||
|
|
@ -989,6 +1003,50 @@ mod tests {
|
||||||
.expect("tool exists")
|
.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]
|
#[tokio::test]
|
||||||
async fn ticket_tools_create_list_show_and_doctor() {
|
async fn ticket_tools_create_list_show_and_doctor() {
|
||||||
let temp = TempDir::new().unwrap();
|
let temp = TempDir::new().unwrap();
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue
Block a user