merge: role launch config validation

This commit is contained in:
Keisuke Hirata 2026-06-07 12:42:12 +09:00
commit 3064c984b5
No known key found for this signature in database
2 changed files with 312 additions and 21 deletions

View File

@ -8,10 +8,11 @@ use std::io;
use std::path::PathBuf; use std::path::PathBuf;
use std::time::Duration; use std::time::Duration;
use manifest::{ProfileDiscovery, ProfileResolveOptions, ProfileResolver, ProfileSelector};
use protocol::{ErrorCode, Event, InvokeKind, Method, Segment}; use protocol::{ErrorCode, Event, InvokeKind, Method, Segment};
use thiserror::Error; use thiserror::Error;
pub use ticket::config::TicketRole; pub use ticket::config::TicketRole;
use ticket::config::{TicketConfig, TicketConfigError}; use ticket::config::{TicketConfig, TicketConfigError, TicketRoleLaunchConfigError};
use crate::{PodClient, PodRuntimeCommand, SpawnConfig, SpawnError, SpawnReady, spawn_pod}; use crate::{PodClient, PodRuntimeCommand, SpawnConfig, SpawnError, SpawnReady, spawn_pod};
@ -199,6 +200,16 @@ impl TicketRoleLaunchOptions {
pub enum TicketRoleLaunchError { pub enum TicketRoleLaunchError {
#[error(transparent)] #[error(transparent)]
Config(#[from] TicketConfigError), Config(#[from] TicketConfigError),
#[error(transparent)]
LaunchConfig(#[from] TicketRoleLaunchConfigError),
#[error(
"Ticket role `{role}` profile selector `{selector}` is not resolvable before launch: {message}. Configure `[roles.{role}].profile` with an executable concrete profile selector such as `builtin:default` or a project/user profile"
)]
ProfileResolution {
role: TicketRole,
selector: String,
message: String,
},
#[error("Ticket role Pod name must not be empty")] #[error("Ticket role Pod name must not be empty")]
EmptyPodName, EmptyPodName,
#[error( #[error(
@ -239,7 +250,7 @@ pub fn plan_ticket_role_launch_with_config(
context: TicketRoleLaunchContext, context: TicketRoleLaunchContext,
config: &TicketConfig, config: &TicketConfig,
) -> Result<TicketRoleLaunchPlan, TicketRoleLaunchError> { ) -> Result<TicketRoleLaunchPlan, TicketRoleLaunchError> {
let role_config = config.role(context.role); let role_config = config.role_launch_config(context.role)?;
let profile = role_config.profile.as_str().to_string(); let profile = role_config.profile.as_str().to_string();
let workflow = role_config.workflow.as_str().to_string(); let workflow = role_config.workflow.as_str().to_string();
let launch_prompt_ref = role_config let launch_prompt_ref = role_config
@ -251,6 +262,7 @@ pub fn plan_ticket_role_launch_with_config(
Some(name) => name.to_string(), Some(name) => name.to_string(),
None => default_pod_name(context.role, context.ticket.as_ref()), None => default_pod_name(context.role, context.ticket.as_ref()),
}; };
validate_ticket_role_profile(context.role, &profile, &context.workspace_root, &pod_name)?;
let prompt = build_launch_prompt(&context, &profile, &workflow, launch_prompt_ref.as_deref()); let prompt = build_launch_prompt(&context, &profile, &workflow, launch_prompt_ref.as_deref());
Ok(TicketRoleLaunchPlan { Ok(TicketRoleLaunchPlan {
@ -269,6 +281,35 @@ pub fn plan_ticket_role_launch_with_config(
}) })
} }
fn validate_ticket_role_profile(
role: TicketRole,
profile: &str,
workspace_root: &std::path::Path,
pod_name: &str,
) -> Result<(), TicketRoleLaunchError> {
let selector = ProfileSelector::parse_cli(profile);
let registry = ProfileDiscovery::for_cwd(workspace_root)
.discover()
.map_err(|source| TicketRoleLaunchError::ProfileResolution {
role,
selector: profile.to_string(),
message: source.to_string(),
})?;
ProfileResolver::new()
.with_workspace_base(workspace_root)
.resolve_from_registry(
&selector,
&registry,
ProfileResolveOptions::with_pod_name(pod_name),
)
.map(|_| ())
.map_err(|source| TicketRoleLaunchError::ProfileResolution {
role,
selector: profile.to_string(),
message: source.to_string(),
})
}
/// Spawn the Pod, connect to its socket, send the first `Method::Run` input, /// Spawn the Pod, connect to its socket, send the first `Method::Run` input,
/// and wait for bounded acceptance evidence from the Pod event stream. /// and wait for bounded acceptance evidence from the Pod event stream.
pub async fn launch_ticket_role_pod<F>( pub async fn launch_ticket_role_pod<F>(
@ -597,6 +638,16 @@ mod tests {
std::fs::write(dir.join("ticket.config.toml"), content).unwrap(); std::fs::write(dir.join("ticket.config.toml"), content).unwrap();
} }
fn write_builtin_role_config(workspace: &std::path::Path, roles: &[TicketRole]) {
let mut config = String::new();
for role in roles {
config.push_str(&format!(
"\n[roles.{role}]\nprofile = \"builtin:default\"\n"
));
}
write_config(workspace, &config);
}
fn text_segment(plan: &TicketRoleLaunchPlan) -> &str { fn text_segment(plan: &TicketRoleLaunchPlan) -> &str {
match &plan.run_segments[1] { match &plan.run_segments[1] {
Segment::Text { content } => content, Segment::Text { content } => content,
@ -751,26 +802,103 @@ mod tests {
} }
#[test] #[test]
fn default_config_role_launch_plan_uses_defaults() { fn default_config_role_launch_plan_requires_explicit_role_config() {
let temp = TempDir::new().unwrap(); let temp = TempDir::new().unwrap();
let mut context = TicketRoleLaunchContext::new(temp.path(), TicketRole::Coder); let mut context = TicketRoleLaunchContext::new(temp.path(), TicketRole::Coder);
context.ticket = Some(TicketRef::slug("Ticket Role Pod Launcher")); context.ticket = Some(TicketRef::slug("Ticket Role Pod Launcher"));
let err = plan_ticket_role_launch(context).unwrap_err();
assert!(
err.to_string()
.contains("Ticket role `coder` is not launch-configured")
);
assert!(err.to_string().contains("[roles.coder]"));
}
#[test]
fn backend_only_config_is_not_sufficient_for_role_launch_plan() {
let temp = TempDir::new().unwrap();
write_config(
temp.path(),
r#"
[backend]
provider = "builtin:yoi_local"
root = ".yoi/tickets"
"#,
);
let context = TicketRoleLaunchContext::new(temp.path(), TicketRole::Intake);
let err = plan_ticket_role_launch(context).unwrap_err();
assert!(
err.to_string()
.contains("Ticket role `intake` is not launch-configured")
);
}
#[test]
fn explicit_inherit_profile_fails_before_launch_planning() {
let temp = TempDir::new().unwrap();
write_config(
temp.path(),
r#"
[roles.intake]
profile = "inherit"
"#,
);
let context = TicketRoleLaunchContext::new(temp.path(), TicketRole::Intake);
let err = plan_ticket_role_launch(context).unwrap_err();
assert!(err.to_string().contains("profile = \"inherit\""));
assert!(err.to_string().contains("top-level Ticket role launch"));
}
#[test]
fn unresolvable_profile_selector_fails_before_spawn() {
let temp = TempDir::new().unwrap();
write_config(
temp.path(),
r#"
[roles.intake]
profile = "project:no-such-ticket-role-profile"
"#,
);
let context = TicketRoleLaunchContext::new(temp.path(), TicketRole::Intake);
let err = plan_ticket_role_launch(context).unwrap_err();
assert!(
err.to_string().contains(
"profile selector `project:no-such-ticket-role-profile` is not resolvable"
)
);
assert!(err.to_string().contains("[roles.intake].profile"));
}
#[test]
fn full_concrete_role_config_allows_launch_planning() {
let temp = TempDir::new().unwrap();
write_builtin_role_config(temp.path(), &[TicketRole::Intake]);
let context = TicketRoleLaunchContext::new(temp.path(), TicketRole::Intake);
let plan = plan_ticket_role_launch(context).unwrap(); let plan = plan_ticket_role_launch(context).unwrap();
assert_eq!(plan.role, TicketRole::Coder); assert_eq!(plan.role, TicketRole::Intake);
assert_eq!(plan.pod_name, "ticket-coder-ticket-role-pod-launcher"); assert_eq!(plan.profile, "builtin:default");
assert_eq!(plan.profile, "inherit"); }
assert_eq!(plan.workflow, "multi-agent-workflow");
assert_eq!(plan.launch_prompt_ref, None); #[test]
assert!(matches!( fn spawn_config_still_rejects_inherit_profile_defensively() {
&plan.run_segments[0], let temp = TempDir::new().unwrap();
Segment::WorkflowInvoke { slug } if slug == "multi-agent-workflow" let mut plan = test_launch_plan(temp.path());
)); plan.profile = "inherit".to_string();
assert!(text_segment(&plan).contains("Profile selector: inherit"));
let err = plan let err = plan
.spawn_config(PodRuntimeCommand::for_executable("/bin/yoi")) .spawn_config(PodRuntimeCommand::for_executable("/bin/yoi"))
.unwrap_err(); .unwrap_err();
assert!(matches!( assert!(matches!(
err, err,
TicketRoleLaunchError::UnsupportedInheritProfile TicketRoleLaunchError::UnsupportedInheritProfile
@ -785,7 +913,7 @@ mod tests {
temp.path(), temp.path(),
r#" r#"
[roles.reviewer] [roles.reviewer]
profile = "project:reviewer" profile = "builtin:default"
launch_prompt = "$workspace/ticket/reviewer/launch" launch_prompt = "$workspace/ticket/reviewer/launch"
workflow = "ticket-review-workflow" workflow = "ticket-review-workflow"
"#, "#,
@ -802,7 +930,7 @@ workflow = "ticket-review-workflow"
let text = text_segment(&plan); let text = text_segment(&plan);
assert_eq!(plan.pod_name, "reviewer-fixed"); assert_eq!(plan.pod_name, "reviewer-fixed");
assert_eq!(plan.profile, "project:reviewer"); assert_eq!(plan.profile, "builtin:default");
assert_eq!(plan.workflow, "ticket-review-workflow"); assert_eq!(plan.workflow, "ticket-review-workflow");
assert_eq!( assert_eq!(
plan.launch_prompt_ref.as_deref(), plan.launch_prompt_ref.as_deref(),
@ -816,19 +944,27 @@ workflow = "ticket-review-workflow"
"Configured launch_prompt ref (unresolved): $workspace/ticket/reviewer/launch" "Configured launch_prompt ref (unresolved): $workspace/ticket/reviewer/launch"
)); ));
assert!(text.contains("Workflow: ticket-review-workflow")); assert!(text.contains("Workflow: ticket-review-workflow"));
assert!(text.contains("Profile selector: project:reviewer")); assert!(text.contains("Profile selector: builtin:default"));
assert!(!text.contains("system_instruction")); assert!(!text.contains("system_instruction"));
let spawn = plan let spawn = plan
.spawn_config(PodRuntimeCommand::for_executable("/bin/yoi")) .spawn_config(PodRuntimeCommand::for_executable("/bin/yoi"))
.unwrap(); .unwrap();
assert_eq!(spawn.pod_name, "reviewer-fixed"); assert_eq!(spawn.pod_name, "reviewer-fixed");
assert_eq!(spawn.profile.as_deref(), Some("project:reviewer")); assert_eq!(spawn.profile.as_deref(), Some("builtin:default"));
assert_eq!(spawn.cwd, temp.path()); assert_eq!(spawn.cwd, temp.path());
} }
#[test] #[test]
fn generated_prompt_covers_intake_orchestrator_and_reviewer_context() { fn generated_prompt_covers_intake_orchestrator_and_reviewer_context() {
let temp = TempDir::new().unwrap(); let temp = TempDir::new().unwrap();
write_builtin_role_config(
temp.path(),
&[
TicketRole::Intake,
TicketRole::Orchestrator,
TicketRole::Reviewer,
],
);
let mut intake = TicketRoleLaunchContext::new(temp.path(), TicketRole::Intake); let mut intake = TicketRoleLaunchContext::new(temp.path(), TicketRole::Intake);
intake.user_instruction = Some("Clarify and materialize this request as a Ticket.".into()); intake.user_instruction = Some("Clarify and materialize this request as a Ticket.".into());
@ -882,6 +1018,7 @@ workflow = "ticket-review-workflow"
#[test] #[test]
fn caller_provided_pod_name_is_used_exactly() { fn caller_provided_pod_name_is_used_exactly() {
let temp = TempDir::new().unwrap(); let temp = TempDir::new().unwrap();
write_builtin_role_config(temp.path(), &[TicketRole::Intake]);
let mut context = TicketRoleLaunchContext::new(temp.path(), TicketRole::Intake); let mut context = TicketRoleLaunchContext::new(temp.path(), TicketRole::Intake);
context.pod_name = Some("custom-intake-pod".into()); context.pod_name = Some("custom-intake-pod".into());

View File

@ -5,7 +5,7 @@
//! launch prompts, and workflows so this crate remains independent from `pod` //! launch prompts, and workflows so this crate remains independent from `pod`
//! and `manifest` runtime resolution. //! and `manifest` runtime resolution.
use std::collections::BTreeMap; use std::collections::{BTreeMap, BTreeSet};
use std::fmt; use std::fmt;
use std::fs; use std::fs;
use std::path::{Path, PathBuf}; use std::path::{Path, PathBuf};
@ -86,6 +86,13 @@ impl TicketConfig {
self.roles.get(role) self.roles.get(role)
} }
pub fn role_launch_config(
&self,
role: TicketRole,
) -> Result<&TicketRoleConfig, TicketRoleLaunchConfigError> {
self.roles.launch_config(role)
}
pub fn profile_for(&self, role: TicketRole) -> &ProfileSelectorRef { pub fn profile_for(&self, role: TicketRole) -> &ProfileSelectorRef {
&self.role(role).profile &self.role(role).profile
} }
@ -200,6 +207,8 @@ impl fmt::Display for TicketRole {
#[derive(Debug, Clone, PartialEq, Eq)] #[derive(Debug, Clone, PartialEq, Eq)]
pub struct TicketRoleProfiles { pub struct TicketRoleProfiles {
inner: BTreeMap<TicketRole, TicketRoleConfig>, inner: BTreeMap<TicketRole, TicketRoleConfig>,
configured_roles: BTreeSet<TicketRole>,
profile_configured_roles: BTreeSet<TicketRole>,
} }
impl TicketRoleProfiles { impl TicketRoleProfiles {
@ -209,6 +218,31 @@ impl TicketRoleProfiles {
.expect("TicketRoleProfiles always contains all fixed roles") .expect("TicketRoleProfiles always contains all fixed roles")
} }
pub fn role_is_configured(&self, role: TicketRole) -> bool {
self.configured_roles.contains(&role)
}
pub fn profile_is_configured(&self, role: TicketRole) -> bool {
self.profile_configured_roles.contains(&role)
}
pub fn launch_config(
&self,
role: TicketRole,
) -> Result<&TicketRoleConfig, TicketRoleLaunchConfigError> {
if !self.role_is_configured(role) {
return Err(TicketRoleLaunchConfigError::MissingRoleTable { role });
}
if !self.profile_is_configured(role) {
return Err(TicketRoleLaunchConfigError::MissingProfile { role });
}
let config = self.get(role);
if config.profile.as_str() == "inherit" {
return Err(TicketRoleLaunchConfigError::InheritProfile { role });
}
Ok(config)
}
pub fn iter(&self) -> impl Iterator<Item = (TicketRole, &TicketRoleConfig)> { pub fn iter(&self) -> impl Iterator<Item = (TicketRole, &TicketRoleConfig)> {
TicketRole::ALL TicketRole::ALL
.into_iter() .into_iter()
@ -222,10 +256,30 @@ impl Default for TicketRoleProfiles {
.into_iter() .into_iter()
.map(|role| (role, TicketRoleConfig::default_for_role(role))) .map(|role| (role, TicketRoleConfig::default_for_role(role)))
.collect(); .collect();
Self { inner } Self {
inner,
configured_roles: BTreeSet::new(),
profile_configured_roles: BTreeSet::new(),
}
} }
} }
#[derive(Debug, Clone, PartialEq, Eq, Error)]
pub enum TicketRoleLaunchConfigError {
#[error(
"Ticket role `{role}` is not launch-configured; add `[roles.{role}]` with `profile = \"builtin:default\"` or another executable concrete profile selector"
)]
MissingRoleTable { role: TicketRole },
#[error(
"Ticket role `{role}` has no launch profile; set `[roles.{role}].profile` to `builtin:default` or another executable concrete profile selector"
)]
MissingProfile { role: TicketRole },
#[error(
"Ticket role `{role}` uses `profile = \"inherit\"`; top-level Ticket role launch requires an explicit executable profile selector such as `builtin:default` or a project/user profile"
)]
InheritProfile { role: TicketRole },
}
#[derive(Debug, Clone, PartialEq, Eq)] #[derive(Debug, Clone, PartialEq, Eq)]
pub struct TicketRoleConfig { pub struct TicketRoleConfig {
pub profile: ProfileSelectorRef, pub profile: ProfileSelectorRef,
@ -407,7 +461,12 @@ impl RawTicketConfig {
path: path.to_path_buf(), path: path.to_path_buf(),
message: format!("unknown Ticket role `{name}`"), message: format!("unknown Ticket role `{name}`"),
})?; })?;
let profile_configured = raw_role.profile.is_some();
roles.inner.insert(role, raw_role.resolve(role)); roles.inner.insert(role, raw_role.resolve(role));
roles.configured_roles.insert(role);
if profile_configured {
roles.profile_configured_roles.insert(role);
}
} }
Ok(TicketConfig { Ok(TicketConfig {
backend: self.backend.resolve(workspace_root).map_err(|message| { backend: self.backend.resolve(workspace_root).map_err(|message| {
@ -467,7 +526,8 @@ impl RawBackendConfig {
#[derive(Debug, Deserialize)] #[derive(Debug, Deserialize)]
#[serde(deny_unknown_fields)] #[serde(deny_unknown_fields)]
struct RawTicketRoleConfig { struct RawTicketRoleConfig {
profile: ProfileSelectorRef, #[serde(default)]
profile: Option<ProfileSelectorRef>,
#[serde(default)] #[serde(default)]
launch_prompt: Option<PromptRef>, launch_prompt: Option<PromptRef>,
#[serde(default)] #[serde(default)]
@ -477,7 +537,7 @@ struct RawTicketRoleConfig {
impl RawTicketRoleConfig { impl RawTicketRoleConfig {
fn resolve(self, role: TicketRole) -> TicketRoleConfig { fn resolve(self, role: TicketRole) -> TicketRoleConfig {
TicketRoleConfig { TicketRoleConfig {
profile: self.profile, profile: self.profile.unwrap_or_else(ProfileSelectorRef::inherit),
launch_prompt: self.launch_prompt, launch_prompt: self.launch_prompt,
workflow: self workflow: self
.workflow .workflow
@ -605,6 +665,100 @@ profile = "project:coder"
assert_eq!(config.profile_for(TicketRole::Reviewer).as_str(), "inherit"); assert_eq!(config.profile_for(TicketRole::Reviewer).as_str(), "inherit");
} }
#[test]
fn backend_only_config_is_not_role_launch_ready() {
let temp = TempDir::new().unwrap();
write_config(
temp.path(),
r#"
[backend]
provider = "builtin:yoi_local"
root = ".yoi/tickets"
"#,
);
let config = TicketConfig::load_workspace(temp.path()).unwrap();
assert_eq!(config.backend.root, temp.path().join(".yoi/tickets"));
assert_eq!(
config.role_launch_config(TicketRole::Intake).unwrap_err(),
TicketRoleLaunchConfigError::MissingRoleTable {
role: TicketRole::Intake
}
);
}
#[test]
fn partial_role_config_only_marks_configured_roles_launch_ready() {
let temp = TempDir::new().unwrap();
write_config(
temp.path(),
r#"
[roles.intake]
profile = "builtin:default"
"#,
);
let config = TicketConfig::load_workspace(temp.path()).unwrap();
assert_eq!(
config
.role_launch_config(TicketRole::Intake)
.unwrap()
.profile
.as_str(),
"builtin:default"
);
assert_eq!(
config
.role_launch_config(TicketRole::Orchestrator)
.unwrap_err(),
TicketRoleLaunchConfigError::MissingRoleTable {
role: TicketRole::Orchestrator
}
);
}
#[test]
fn role_table_without_profile_is_not_role_launch_ready() {
let temp = TempDir::new().unwrap();
write_config(
temp.path(),
r#"
[roles.orchestrator]
workflow = "ticket-orchestrator-routing"
"#,
);
let config = TicketConfig::load_workspace(temp.path()).unwrap();
assert_eq!(
config
.role_launch_config(TicketRole::Orchestrator)
.unwrap_err(),
TicketRoleLaunchConfigError::MissingProfile {
role: TicketRole::Orchestrator
}
);
}
#[test]
fn inherit_profile_is_not_role_launch_ready() {
let temp = TempDir::new().unwrap();
write_config(
temp.path(),
r#"
[roles.intake]
profile = "inherit"
"#,
);
let config = TicketConfig::load_workspace(temp.path()).unwrap();
assert_eq!(
config.role_launch_config(TicketRole::Intake).unwrap_err(),
TicketRoleLaunchConfigError::InheritProfile {
role: TicketRole::Intake
}
);
}
#[test] #[test]
fn unknown_roles_are_rejected() { fn unknown_roles_are_rejected() {
let temp = TempDir::new().unwrap(); let temp = TempDir::new().unwrap();